Bug 1315592 - Handle JS compilation errors in the async subscript loader. r=bholley, a=jcristau
authorShu-yu Guo <shu@rfrn.org>
Thu, 15 Dec 2016 11:51:00 -0500
changeset 353070 aca9ef14e7818b7c19ba07760b2be2cda837559a
parent 353069 db736300d90f165a1636c35d5dd1cd4e369092e7
child 353071 c1af43b748f1e30cef0b721edd797bd45fcf9613
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, jcristau
bugs1315592
milestone52.0a2
Bug 1315592 - Handle JS compilation errors in the async subscript loader. r=bholley, a=jcristau
js/xpconnect/loader/mozJSSubScriptLoader.cpp
js/xpconnect/loader/mozJSSubScriptLoader.h
js/xpconnect/tests/unit/subScriptWithEarlyError.js
js/xpconnect/tests/unit/test_asyncLoadSubScriptError.js
js/xpconnect/tests/unit/xpcshell.ini
--- a/js/xpconnect/loader/mozJSSubScriptLoader.cpp
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ -86,42 +86,43 @@ mozJSSubScriptLoader::mozJSSubScriptLoad
 
 mozJSSubScriptLoader::~mozJSSubScriptLoader()
 {
     /* empty */
 }
 
 NS_IMPL_ISUPPORTS(mozJSSubScriptLoader, mozIJSSubScriptLoader)
 
-static nsresult
+static void
 ReportError(JSContext* cx, const char* msg)
 {
     RootedValue exn(cx, JS::StringValue(JS_NewStringCopyZ(cx, msg)));
     JS_SetPendingException(cx, exn);
-    return NS_OK;
 }
 
-static nsresult
+static void
 ReportError(JSContext* cx, const char* origMsg, nsIURI* uri)
 {
-    if (!uri)
-        return ReportError(cx, origMsg);
+    if (!uri) {
+        ReportError(cx, origMsg);
+        return;
+    }
 
     nsAutoCString spec;
     nsresult rv = uri->GetSpec(spec);
     if (NS_FAILED(rv))
         spec.Assign("(unknown)");
 
     nsAutoCString msg(origMsg);
     msg.Append(": ");
     msg.Append(spec);
-    return ReportError(cx, msg.get());
+    ReportError(cx, msg.get());
 }
 
-nsresult
+bool
 PrepareScript(nsIURI* uri,
               JSContext* cx,
               RootedObject& targetObj,
               const char* uriStr,
               const nsAString& charset,
               const char* buf,
               int64_t len,
               bool reuseGlobal,
@@ -138,110 +139,115 @@ PrepareScript(nsIURI* uri,
         nsresult rv =
             nsScriptLoader::ConvertToUTF16(nullptr, reinterpret_cast<const uint8_t*>(buf), len,
                                            charset, nullptr, scriptBuf, scriptLength);
 
         JS::SourceBufferHolder srcBuf(scriptBuf, scriptLength,
                                       JS::SourceBufferHolder::GiveOwnership);
 
         if (NS_FAILED(rv)) {
-            return ReportError(cx, LOAD_ERROR_BADCHARSET, uri);
+            ReportError(cx, LOAD_ERROR_BADCHARSET, uri);
+            return false;
         }
 
         if (!reuseGlobal) {
-            if (JS_IsGlobalObject(targetObj))
-                JS::Compile(cx, options, srcBuf, script);
-            else
-                JS::CompileForNonSyntacticScope(cx, options, srcBuf, script);
+            if (JS_IsGlobalObject(targetObj)) {
+                return JS::Compile(cx, options, srcBuf, script);
+            }
+            return JS::CompileForNonSyntacticScope(cx, options, srcBuf, script);
         } else {
             AutoObjectVector envChain(cx);
-            if (!JS_IsGlobalObject(targetObj) &&
-                !envChain.append(targetObj)) {
-                return NS_ERROR_OUT_OF_MEMORY;
+            if (!JS_IsGlobalObject(targetObj) && !envChain.append(targetObj)) {
+                return false;
             }
-            // XXXbz do we really not care if the compile fails???
-            JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr,
-                                srcBuf, function);
+            return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr,
+                                       srcBuf, function);
         }
     } else {
         // We only use lazy source when no special encoding is specified because
         // the lazy source loader doesn't know the encoding.
         if (!reuseGlobal) {
             options.setSourceIsLazy(true);
-            if (JS_IsGlobalObject(targetObj))
-                JS::Compile(cx, options, buf, len, script);
-            else
-                JS::CompileForNonSyntacticScope(cx, options, buf, len, script);
+            if (JS_IsGlobalObject(targetObj)) {
+                return JS::Compile(cx, options, buf, len, script);
+            }
+            return JS::CompileForNonSyntacticScope(cx, options, buf, len, script);
         } else {
             AutoObjectVector envChain(cx);
-            if (!JS_IsGlobalObject(targetObj) &&
-                !envChain.append(targetObj)) {
-                return NS_ERROR_OUT_OF_MEMORY;
+            if (!JS_IsGlobalObject(targetObj) && !envChain.append(targetObj)) {
+                return false;
             }
-            // XXXbz do we really not care if the compile fails???
-            JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr,
-                                buf, len, function);
+            return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr,
+                                       buf, len, function);
         }
     }
-    return NS_OK;
 }
 
-nsresult
+bool
 EvalScript(JSContext* cx,
            RootedObject& target_obj,
            MutableHandleValue retval,
            nsIURI* uri,
            bool cache,
            RootedScript& script,
            RootedFunction& function)
 {
     if (function) {
         script = JS_GetFunctionScript(cx, function);
     }
 
-    bool ok = false;
     if (function) {
-        ok = JS_CallFunction(cx, target_obj, function, JS::HandleValueArray::empty(),
-                             retval);
+        if (!JS_CallFunction(cx, target_obj, function, JS::HandleValueArray::empty(), retval)) {
+            return false;
+        }
     } else {
         if (JS_IsGlobalObject(target_obj)) {
-            ok = JS_ExecuteScript(cx, script, retval);
+            if (!JS_ExecuteScript(cx, script, retval)) {
+                return false;
+            }
         } else {
             JS::AutoObjectVector envChain(cx);
-            ok = envChain.append(target_obj) &&
-                 JS_ExecuteScript(cx, envChain, script, retval);
+            if (!envChain.append(target_obj)) {
+                return false;
+            }
+            if (!JS_ExecuteScript(cx, envChain, script, retval)) {
+                return false;
+            }
         }
     }
 
-    if (ok) {
-        JSAutoCompartment rac(cx, target_obj);
-        if (!JS_WrapValue(cx, retval))
-            return NS_ERROR_UNEXPECTED;
+    JSAutoCompartment rac(cx, target_obj);
+    if (!JS_WrapValue(cx, retval)) {
+        return false;
     }
 
-    nsAutoCString cachePath;
-    JSVersion version = JS_GetVersion(cx);
-    cachePath.AppendPrintf("jssubloader/%d", version);
-    PathifyURI(uri, cachePath);
+    if (cache && !!script) {
+        nsAutoCString cachePath;
+        JSVersion version = JS_GetVersion(cx);
+        cachePath.AppendPrintf("jssubloader/%d", version);
+        PathifyURI(uri, cachePath);
 
-    nsCOMPtr<nsIScriptSecurityManager> secman =
-        do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID);
-    if (!secman)
-        return NS_OK;
+        nsCOMPtr<nsIScriptSecurityManager> secman =
+            do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID);
+        if (!secman) {
+            return false;
+        }
 
-    nsCOMPtr<nsIPrincipal> principal;
-    nsresult rv = secman->GetSystemPrincipal(getter_AddRefs(principal));
-    if (NS_FAILED(rv) || !principal)
-        return rv;
+        nsCOMPtr<nsIPrincipal> principal;
+        nsresult rv = secman->GetSystemPrincipal(getter_AddRefs(principal));
+        if (NS_FAILED(rv) || !principal) {
+            ReportError(cx, LOAD_ERROR_NOPRINCIPALS, uri);
+            return false;
+        }
 
-    if (cache && ok && !!script) {
         WriteCachedScript(StartupCache::GetSingleton(),
                           cachePath, cx, principal, script);
     }
-    return NS_OK;
+
+    return true;
 }
 
 class AsyncScriptLoader : public nsIIncrementalStreamLoaderObserver
 {
 public:
     NS_DECL_CYCLE_COLLECTING_ISUPPORTS
     NS_DECL_NSIINCREMENTALSTREAMLOADEROBSERVER
 
@@ -294,104 +300,108 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(Asy
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mTargetObj)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(AsyncScriptLoader)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(AsyncScriptLoader)
 
 class MOZ_STACK_CLASS AutoRejectPromise
 {
-  public:
-    AutoRejectPromise(JSContext* cx,
+public:
+    AutoRejectPromise(AutoEntryScript& aAutoEntryScript,
                       Promise* aPromise,
                       nsIGlobalObject* aGlobalObject)
-      : mCx(cx)
-      , mPromise(aPromise)
-      , mGlobalObject(aGlobalObject) {}
+        : mAutoEntryScript(aAutoEntryScript)
+        , mPromise(aPromise)
+        , mGlobalObject(aGlobalObject) {}
 
     ~AutoRejectPromise() {
-      if (mPromise) {
-        JS::Rooted<JS::Value> undefined(mCx, JS::UndefinedValue());
-        mPromise->MaybeReject(mCx, undefined);
-      }
+        if (mPromise) {
+            JSContext* cx = mAutoEntryScript.cx();
+            RootedValue rejectionValue(cx, JS::UndefinedValue());
+            if (mAutoEntryScript.HasException()) {
+                Unused << mAutoEntryScript.PeekException(&rejectionValue);
+            }
+            mPromise->MaybeReject(cx, rejectionValue);
+        }
     }
 
     void ResolvePromise(HandleValue aResolveValue) {
-      mPromise->MaybeResolve(aResolveValue);
-      mPromise = nullptr;
+        mPromise->MaybeResolve(aResolveValue);
+        mPromise = nullptr;
     }
 
-  private:
-    JSContext*                mCx;
-    RefPtr<Promise>         mPromise;
+private:
+    AutoEntryScript&          mAutoEntryScript;
+    RefPtr<Promise>           mPromise;
     nsCOMPtr<nsIGlobalObject> mGlobalObject;
 };
 
 NS_IMETHODIMP
 AsyncScriptLoader::OnIncrementalData(nsIIncrementalStreamLoader* aLoader,
                                      nsISupports* aContext,
                                      uint32_t aDataLength,
                                      const uint8_t* aData,
                                      uint32_t *aConsumedData)
 {
-  return NS_OK;
+    return NS_OK;
 }
 
 NS_IMETHODIMP
 AsyncScriptLoader::OnStreamComplete(nsIIncrementalStreamLoader* aLoader,
                                     nsISupports* aContext,
                                     nsresult aStatus,
                                     uint32_t aLength,
                                     const uint8_t* aBuf)
 {
     nsCOMPtr<nsIURI> uri;
     mChannel->GetURI(getter_AddRefs(uri));
 
     nsCOMPtr<nsIGlobalObject> globalObject = xpc::NativeGlobal(mTargetObj);
     AutoEntryScript aes(globalObject, "async loadSubScript");
+    AutoRejectPromise autoPromise(aes, mPromise, globalObject);
     JSContext* cx = aes.cx();
-    AutoRejectPromise autoPromise(cx, mPromise, globalObject);
 
     if (NS_FAILED(aStatus)) {
         ReportError(cx, "Unable to load script.", uri);
     }
     // Just notify that we are done with this load.
     NS_ENSURE_SUCCESS(aStatus, NS_OK);
 
     if (aLength == 0) {
-        return ReportError(cx, LOAD_ERROR_NOCONTENT, uri);
+        ReportError(cx, LOAD_ERROR_NOCONTENT, uri);
+        return NS_OK;
     }
 
     if (aLength > INT32_MAX) {
-        return ReportError(cx, LOAD_ERROR_CONTENTTOOBIG, uri);
+        ReportError(cx, LOAD_ERROR_CONTENTTOOBIG, uri);
+        return NS_OK;
     }
 
     RootedFunction function(cx);
     RootedScript script(cx);
     nsAutoCString spec;
     nsresult rv = uri->GetSpec(spec);
     NS_ENSURE_SUCCESS(rv, rv);
 
     RootedObject target_obj(cx, mTargetObj);
 
-    rv = PrepareScript(uri, cx, target_obj, spec.get(), mCharset,
+    if (!PrepareScript(uri, cx, target_obj, spec.get(), mCharset,
                        reinterpret_cast<const char*>(aBuf), aLength,
-                       mReuseGlobal, &script, &function);
-    if (NS_FAILED(rv)) {
-        return rv;
+                       mReuseGlobal, &script, &function))
+    {
+        return NS_OK;
     }
 
     JS::Rooted<JS::Value> retval(cx);
-    rv = EvalScript(cx, target_obj, &retval, uri, mCache, script, function);
-
-    if (NS_SUCCEEDED(rv)) {
+    if (EvalScript(cx, target_obj, &retval, uri, mCache, script, function)) {
         autoPromise.ResolvePromise(retval);
     }
 
-    return rv;
+    return NS_OK;
 }
 
 nsresult
 mozJSSubScriptLoader::ReadScriptAsync(nsIURI* uri, JSObject* targetObjArg,
                                       const nsAString& charset,
                                       nsIIOService* serv, bool reuseGlobal,
                                       bool cache, MutableHandleValue retval)
 {
@@ -444,17 +454,17 @@ mozJSSubScriptLoader::ReadScriptAsync(ns
     nsCOMPtr<nsIIncrementalStreamLoader> loader;
     rv = NS_NewIncrementalStreamLoader(getter_AddRefs(loader), loadObserver);
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIStreamListener> listener = loader.get();
     return channel->AsyncOpen2(listener);
 }
 
-nsresult
+bool
 mozJSSubScriptLoader::ReadScript(nsIURI* uri, JSContext* cx, JSObject* targetObjArg,
                                  const nsAString& charset, const char* uriStr,
                                  nsIIOService* serv, nsIPrincipal* principal,
                                  bool reuseGlobal, JS::MutableHandleScript script,
                                  JS::MutableHandleFunction function)
 {
     script.set(nullptr);
     function.set(nullptr);
@@ -477,41 +487,41 @@ mozJSSubScriptLoader::ReadScript(nsIURI*
                        serv);
 
     if (NS_SUCCEEDED(rv)) {
         chan->SetContentType(NS_LITERAL_CSTRING("application/javascript"));
         rv = chan->Open2(getter_AddRefs(instream));
     }
 
     if (NS_FAILED(rv)) {
-        return ReportError(cx, LOAD_ERROR_NOSTREAM, uri);
+        ReportError(cx, LOAD_ERROR_NOSTREAM, uri);
+        return false;
     }
 
     int64_t len = -1;
 
     rv = chan->GetContentLength(&len);
     if (NS_FAILED(rv) || len == -1) {
-        return ReportError(cx, LOAD_ERROR_NOCONTENT, uri);
+        ReportError(cx, LOAD_ERROR_NOCONTENT, uri);
+        return false;
     }
 
     if (len > INT32_MAX) {
-        return ReportError(cx, LOAD_ERROR_CONTENTTOOBIG, uri);
+        ReportError(cx, LOAD_ERROR_CONTENTTOOBIG, uri);
+        return false;
     }
 
     nsCString buf;
     rv = NS_ReadInputStreamToString(instream, buf, len);
-    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ENSURE_SUCCESS(rv, false);
 
-    rv = PrepareScript(uri, cx, target_obj, uriStr, charset,
-                       buf.get(), len,
-                       reuseGlobal,
-                       script, function);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    return NS_OK;
+    return PrepareScript(uri, cx, target_obj, uriStr, charset,
+                         buf.get(), len,
+                         reuseGlobal,
+                         script, function);
 }
 
 NS_IMETHODIMP
 mozJSSubScriptLoader::LoadSubScript(const nsAString& url,
                                     HandleValue target,
                                     const nsAString& charset,
                                     JSContext* cx,
                                     MutableHandleValue retval)
@@ -607,42 +617,47 @@ mozJSSubScriptLoader::DoLoadSubScriptWit
     JSAutoCompartment ac(cx, targetObj);
 
     // Suppress caching if we're compiling as content.
     StartupCache* cache = (principal == mSystemPrincipal)
                           ? StartupCache::GetSingleton()
                           : nullptr;
     nsCOMPtr<nsIIOService> serv = do_GetService(NS_IOSERVICE_CONTRACTID);
     if (!serv) {
-        return ReportError(cx, LOAD_ERROR_NOSERVICE);
+        ReportError(cx, LOAD_ERROR_NOSERVICE);
+        return NS_OK;
     }
 
     // Make sure to explicitly create the URI, since we'll need the
     // canonicalized spec.
     rv = NS_NewURI(getter_AddRefs(uri), NS_LossyConvertUTF16toASCII(url).get(), nullptr, serv);
     if (NS_FAILED(rv)) {
-        return ReportError(cx, LOAD_ERROR_NOURI);
+        ReportError(cx, LOAD_ERROR_NOURI);
+        return NS_OK;
     }
 
     rv = uri->GetSpec(uriStr);
     if (NS_FAILED(rv)) {
-        return ReportError(cx, LOAD_ERROR_NOSPEC);
+        ReportError(cx, LOAD_ERROR_NOSPEC);
+        return NS_OK;
     }
 
     rv = uri->GetScheme(scheme);
     if (NS_FAILED(rv)) {
-        return ReportError(cx, LOAD_ERROR_NOSCHEME, uri);
+        ReportError(cx, LOAD_ERROR_NOSCHEME, uri);
+        return NS_OK;
     }
 
     if (!scheme.EqualsLiteral("chrome") && !scheme.EqualsLiteral("app")) {
         // This might be a URI to a local file, though!
         nsCOMPtr<nsIURI> innerURI = NS_GetInnermostURI(uri);
         nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(innerURI);
         if (!fileURL) {
-            return ReportError(cx, LOAD_ERROR_URI_NOT_LOCAL, uri);
+            ReportError(cx, LOAD_ERROR_URI_NOT_LOCAL, uri);
+            return NS_OK;
         }
 
         // For file URIs prepend the filename with the filename of the
         // calling script, and " -> ". See bug 418356.
         nsAutoCString tmp(filename.get());
         tmp.AppendLiteral(" -> ");
         tmp.Append(uriStr);
 
@@ -666,27 +681,28 @@ mozJSSubScriptLoader::DoLoadSubScriptWit
 
     // If we are doing an async load, trigger it and bail out.
     if (!script && options.async) {
         return ReadScriptAsync(uri, targetObj, options.charset, serv,
                                reusingGlobal, !!cache, retval);
     }
 
     if (!script) {
-        rv = ReadScript(uri, cx, targetObj, options.charset,
+        if (!ReadScript(uri, cx, targetObj, options.charset,
                         static_cast<const char*>(uriStr.get()), serv,
-                        principal, reusingGlobal, &script, &function);
+                        principal, reusingGlobal, &script, &function))
+        {
+            return NS_OK;
+        }
     } else {
         cache = nullptr;
     }
 
-    if (NS_FAILED(rv) || (!script && !function))
-        return rv;
-
-    return EvalScript(cx, targetObj, retval, uri, !!cache, script, function);
+    Unused << EvalScript(cx, targetObj, retval, uri, !!cache, script, function);
+    return NS_OK;
 }
 
 /**
   * Let us compile scripts from a URI off the main thread.
   */
 
 class ScriptPrecompiler : public nsIIncrementalStreamLoaderObserver
 {
--- a/js/xpconnect/loader/mozJSSubScriptLoader.h
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.h
@@ -28,21 +28,21 @@ public:
 
     // all the interface method declarations...
     NS_DECL_ISUPPORTS
     NS_DECL_MOZIJSSUBSCRIPTLOADER
 
 private:
     virtual ~mozJSSubScriptLoader();
 
-    nsresult ReadScript(nsIURI* uri, JSContext* cx, JSObject* target_obj,
-                        const nsAString& charset, const char* uriStr,
-                        nsIIOService* serv, nsIPrincipal* principal,
-                        bool reuseGlobal, JS::MutableHandleScript script,
-                        JS::MutableHandleFunction function);
+    bool ReadScript(nsIURI* uri, JSContext* cx, JSObject* target_obj,
+                    const nsAString& charset, const char* uriStr,
+                    nsIIOService* serv, nsIPrincipal* principal,
+                    bool reuseGlobal, JS::MutableHandleScript script,
+                    JS::MutableHandleFunction function);
 
     nsresult ReadScriptAsync(nsIURI* uri, JSObject* target_obj,
                              const nsAString& charset,
                              nsIIOService* serv, bool reuseGlobal,
                              bool cache, JS::MutableHandleValue retval);
 
     nsresult DoLoadSubScriptWithOptions(const nsAString& url,
                                         LoadSubScriptOptions& options,
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/subScriptWithEarlyError.js
@@ -0,0 +1,1 @@
+var ;
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/test_asyncLoadSubScriptError.js
@@ -0,0 +1,33 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+
+var srvScope = {};
+
+function success(result) {
+  ok(false, "script should not have loaded successfully");
+  do_test_finished();
+}
+
+function error(err) {
+  ok(err instanceof SyntaxError, "loading script with early error asynchronously resulted in error");
+  do_test_finished();
+}
+
+function run_test() {
+  do_test_pending();
+
+  var file = do_get_file("subScriptWithEarlyError.js");
+  var ios = Cc["@mozilla.org/network/io-service;1"]
+              .getService(Ci.nsIIOService);
+  var uri = ios.newFileURI(file);
+  var scriptLoader = Cc["@mozilla.org/moz/jssubscript-loader;1"]
+                       .getService(Ci.mozIJSSubScriptLoader);
+  var p = scriptLoader.loadSubScriptWithOptions(uri.spec,
+                                                { target: srvScope,
+                                                  async: true });
+  p.then(success, error);
+}
--- a/js/xpconnect/tests/unit/xpcshell.ini
+++ b/js/xpconnect/tests/unit/xpcshell.ini
@@ -10,16 +10,17 @@ support-files =
   component-blob.manifest
   component-file.js
   component-file.manifest
   component_import.js
   component_import.manifest
   importer.jsm
   recursive_importA.jsm
   recursive_importB.jsm
+  subScriptWithEarlyError.js
   syntax_error.jsm
 
 [test_allowWaivers.js]
 [test_bogus_files.js]
 [test_bug408412.js]
 [test_bug451678.js]
 [test_bug604362.js]
 [test_bug677864.js]
@@ -128,8 +129,9 @@ head = head_watchdog.js
 head = head_watchdog.js
 [test_weak_keys.js]
 [test_writeToGlobalPrototype.js]
 [test_xpcwn_tamperproof.js]
 [test_xrayed_iterator.js]
 [test_xray_SavedFrame.js]
 [test_xray_SavedFrame-02.js]
 [test_resolve_dead_promise.js]
+[test_asyncLoadSubScriptError.js]