Bug 1306472 - Back out bug 1268047, because the spec it tried to implement backs the web. r=smaug, a=ritu
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 13 Oct 2016 12:10:23 -0400
changeset 350682 76da38551dce411d9d2d021853f91a2b63c1df42
parent 350681 c62114e522a36a446300e8b555357934232388b4
child 350683 24addbfafd8d9c764d1c1603ec9eeb3aa6d6f9b2
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, ritu
bugs1306472, 1268047
milestone50.0
Bug 1306472 - Back out bug 1268047, because the spec it tried to implement backs the web. r=smaug, a=ritu
dom/base/nsJSUtils.cpp
dom/base/nsJSUtils.h
dom/jsurl/nsJSProtocolHandler.cpp
testing/web-platform/tests/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling.html
--- a/dom/base/nsJSUtils.cpp
+++ b/dom/base/nsJSUtils.cpp
@@ -147,16 +147,17 @@ nsJSUtils::EvaluateString(JSContext* aCx
                           JS::MutableHandle<JS::Value> aRetValue,
                           void **aOffThreadToken)
 {
   PROFILER_LABEL("nsJSUtils", "EvaluateString",
     js::ProfileEntry::Category::JS);
 
   MOZ_ASSERT_IF(aCompileOptions.versionSet,
                 aCompileOptions.version != JSVERSION_UNKNOWN);
+  MOZ_ASSERT_IF(aEvaluateOptions.coerceToString, !aCompileOptions.noScriptRval);
   MOZ_ASSERT(aCx == nsContentUtils::GetCurrentJSContext());
   MOZ_ASSERT(aSrcBuf.get());
   MOZ_ASSERT(js::GetGlobalForObjectCrossCompartment(aEvaluationGlobal) ==
              aEvaluationGlobal);
   MOZ_ASSERT_IF(aOffThreadToken, aCompileOptions.noScriptRval);
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(nsContentUtils::IsInMicroTask());
 
@@ -200,16 +201,23 @@ nsJSUtils::EvaluateString(JSContext* aCx
       if (script) {
         ok = JS_ExecuteScript(aCx, scopeChain, script);
       } else {
         ok = false;
       }
     } else if (ok) {
       ok = JS::Evaluate(aCx, scopeChain, aCompileOptions, aSrcBuf, aRetValue);
     }
+
+    if (ok && aEvaluateOptions.coerceToString && !aRetValue.isUndefined()) {
+      JS::Rooted<JS::Value> value(aCx, aRetValue);
+      JSString* str = JS::ToString(aCx, value);
+      ok = !!str;
+      aRetValue.set(ok ? JS::StringValue(str) : JS::UndefinedValue());
+    }
   }
 
   if (!ok) {
     if (JS_IsExceptionPending(aCx)) {
       rv = NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW;
     } else {
       rv = NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW_UNCATCHABLE;
     }
--- a/dom/base/nsJSUtils.h
+++ b/dom/base/nsJSUtils.h
@@ -60,21 +60,28 @@ public:
                                   JS::CompileOptions& aOptions,
                                   const nsACString& aName,
                                   uint32_t aArgCount,
                                   const char** aArgArray,
                                   const nsAString& aBody,
                                   JSObject** aFunctionObject);
 
   struct MOZ_STACK_CLASS EvaluateOptions {
+    bool coerceToString;
     JS::AutoObjectVector scopeChain;
 
     explicit EvaluateOptions(JSContext* cx)
-      : scopeChain(cx)
+      : coerceToString(false)
+      , scopeChain(cx)
     {}
+
+    EvaluateOptions& setCoerceToString(bool aCoerce) {
+      coerceToString = aCoerce;
+      return *this;
+    }
   };
 
   // aEvaluationGlobal is the global to evaluate in.  The return value
   // will then be wrapped back into the compartment aCx is in when
   // this function is called.  For all the EvaluateString overloads,
   // the JSContext must come from an AutoJSAPI that has had
   // TakeOwnershipOfErrorReporting() called on it.
   static nsresult EvaluateString(JSContext* aCx,
--- a/dom/jsurl/nsJSProtocolHandler.cpp
+++ b/dom/jsurl/nsJSProtocolHandler.cpp
@@ -263,22 +263,23 @@ nsresult nsJSThunk::EvaluateScript(nsICh
     }
 
     JS::Rooted<JS::Value> v (cx, JS::UndefinedValue());
     // Finally, we have everything needed to evaluate the expression.
     JS::CompileOptions options(cx);
     options.setFileAndLine(mURL.get(), 1)
            .setVersion(JSVERSION_DEFAULT);
     nsJSUtils::EvaluateOptions evalOptions(cx);
+    evalOptions.setCoerceToString(true);
     rv = nsJSUtils::EvaluateString(cx, NS_ConvertUTF8toUTF16(script),
                                    globalJSObject, options, evalOptions, &v);
 
-    if (NS_FAILED(rv)) {
+    if (NS_FAILED(rv) || !(v.isString() || v.isUndefined())) {
         return NS_ERROR_MALFORMED_URI;
-    } else if (!v.isString()) {
+    } else if (v.isUndefined()) {
         return NS_ERROR_DOM_RETVAL_UNDEFINED;
     } else {
         MOZ_ASSERT(rv != NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW,
                    "How did we get a non-undefined return value?");
         nsAutoJSString result;
         if (!result.init(cx, v)) {
             return NS_ERROR_OUT_OF_MEMORY;
         }
--- a/testing/web-platform/tests/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling.html
+++ b/testing/web-platform/tests/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling.html
@@ -11,22 +11,26 @@
 <iframe src="javascript:null"></iframe>
 <iframe src="javascript:true"></iframe>
 <iframe src="javascript:new String('1')"></iframe>
 <script>
   var t = async_test();
   onload = t.step_func_done(function() {
     assert_equals(frames[0].document.documentElement.textContent,
                   "1", "string return should cause navigation");
+    // The rest of the test is disabled for now, until
+    //  https://github.com/whatwg/html/issues/1895 gets sorted out
+/*
     assert_equals(frames[1].document.documentElement.textContent,
                   "", "number return should not cause navigation");
     assert_equals(frames[2].document.documentElement.textContent,
                   "", "object return should not cause navigation");
     assert_equals(frames[3].document.documentElement.textContent,
                   "", "undefined return should not cause navigation");
     assert_equals(frames[4].document.documentElement.textContent,
                   "", "null return should not cause navigation");
     assert_equals(frames[5].document.documentElement.textContent,
                   "", "null return should not cause navigation");
     assert_equals(frames[6].document.documentElement.textContent,
                   "", "String object return should not cause navigation");
+*/
   });
 </script>