Bug 1931058 - Do not coerce the javascript: return value to a string. r=smaug

Update the code to follow the spec change:
9997cd93c6

Differential Revision: https://phabricator.services.mozilla.com/D229475

browser/components/originattributes/test/browser/browser_firstPartyIsolation_js_uri.js

@@ -12,7 +12,7 @@ add_task(async function test_remote_window_open_js_uri() {
 
   Assert.ok(browser.isRemoteBrowser, "should be a remote browser");
 
-  BrowserTestUtils.startLoadingURIString(browser, `javascript:1;`);
+  BrowserTestUtils.startLoadingURIString(browser, `javascript:"1";`);
 
   await BrowserTestUtils.browserLoaded(browser);
 

dom/jsurl/nsJSProtocolHandler.cpp

@@ -179,61 +179,44 @@ static bool AllowedByCSP(nsIContentSecurityPolicy* aCSP,
   return (NS_SUCCEEDED(rv) && allowsInlineScript);
 }
 
-static bool IsPromiseValue(JSContext* aCx, JS::Handle<JS::Value> aValue) {
-  if (!aValue.isObject()) {
-    return false;
-  }
-
-  // We only care about Promise here, so CheckedUnwrapStatic is fine.
-  JS::Rooted<JSObject*> obj(aCx, js::CheckedUnwrapStatic(&aValue.toObject()));
-  if (!obj) {
-    return false;
-  }
-
-  return JS::IsPromiseObject(obj);
-}
-
-// Execute the compiled script a get the return value, coerced to a string.
+// https://html.spec.whatwg.org/#evaluate-a-javascript:-url
+// Steps 7-10.
 //
-// Copy the returned value into the mutable handle argument. In case of a
-// evaluation failure either during the execution or the conversion of the
-// result to a string, the nsresult is be set to the corresponding result
-// code and the mutable handle argument remains unchanged.
+// If the execution result is a string, |aRv| is set to success, and
+// |aRetValue| is set to the string.
 //
-// The value returned in the mutable handle argument is part of |aCx|'s
-// compartment. If the caller is in a different compartment, then the out-param
-// value should be wrapped by calling |JS_WrapValue|.
+// If the execution result is not a string, |aRv| is set to success, and
+// |aRetValue| is set to undefined.
 //
-static void ExecScriptAndCoerceToString(JSContext* aCx,
+// In case of a evaluation failure during the execution, |aRv| is set to the
+// corresponding result code and |aRetValue| remains unchanged.
+static void ExecScriptAndGetString(JSContext* aCx,
                                    JS::Handle<JSScript*> aScript,
                                    JS::MutableHandle<JS::Value> aRetValue,
                                    mozilla::ErrorResult& aRv) {
   MOZ_ASSERT(aScript);
 
+  // Step 7. Let evaluationStatus be the result of running the classic script
+  //         script.
   if (!JS_ExecuteScript(aCx, aScript, aRetValue)) {
     aRv.NoteJSContextException(aCx);
     return;
   }
 
-  if (IsPromiseValue(aCx, aRetValue)) {
-    // We're a javascript: url and we should treat Promise return values as
-    // undefined.
-    //
-    // Once bug 1477821 is fixed this code might be able to go away, or will
-    // become enshrined in the spec, depending.
-    aRetValue.setUndefined();
-  }
-
-  if (!aRetValue.isUndefined()) {
-    JSString* str = JS::ToString(aCx, aRetValue);
-    if (!str) {
-      // ToString can be a function call, so an exception can be raised while
-      // executing the function.
-      aRv.NoteJSContextException(aCx);
+  // Step 8. Let result be null.
+  // Step 9. If evaluationStatus is a normal completion, and
+  //         evaluationStatus.[[Value]] is a String, then set result to
+  //         evaluationStatus.[[Value]].
+  if (aRetValue.isString()) {
     return;
   }
-    aRetValue.set(JS::StringValue(str));
-  }
+
+  // Step 10. Otherwise, return null.
+  //
+  // NOTE: The `null` here is the return value of the entire algorithm.
+  //       This function returns `undefined` for all cases and let the caller
+  //       handle it.
+  aRetValue.setUndefined();
 }
 
 nsresult JSURLInputStream::EvaluateScript(
@@ -416,15 +399,16 @@ nsresult JSURLInputStream::EvaluateScript(
 
       if (!erv.Failed()) {
         MOZ_ASSERT(!options.noScriptRval);
-        ExecScriptAndCoerceToString(cx, compiledScript, &v, erv);
+        ExecScriptAndGetString(cx, compiledScript, &v, erv);
       }
     }
     rv = mozilla::dom::EvaluationExceptionToNSResult(erv);
   }
 
   js::AssertSameCompartment(cx, v);
+  MOZ_ASSERT(v.isString() || v.isUndefined());
 
-  if (NS_FAILED(rv) || !(v.isString() || v.isUndefined())) {
+  if (NS_FAILED(rv)) {
     return NS_ERROR_MALFORMED_URI;
   }
   if (v.isUndefined()) {

testing/web-platform/meta/url/javascript-urls.window.js.ini

@@ -1,2 +1,5 @@
 [javascript-urls.window.html]
-  expected: ERROR
+  [javascript: URL that fails to parse due to invalid host and has a U+0009 in scheme]
+    expected: FAIL
+  [javascript: URL without an opaque path]
+    expected: FAIL