Bug 1438988: Cleanup some error handling logic. r=mccr8
authorKris Maglione <maglione.k@gmail.com>
Wed, 21 Feb 2018 11:54:17 -0500
changeset 404677 9fc0c79890cb95ea48d83cd358b19e8f90e030cc
parent 404676 182c9af3ed1acd7a4459966f73043093312c32c2
child 404678 6bbf8dc0b86e3e229f90ad38c50f0f56da3a1a11
push id100063
push usermaglione.k@gmail.com
push dateWed, 21 Feb 2018 18:07:09 +0000
treeherdermozilla-inbound@9fc0c79890cb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1438988
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1438988: Cleanup some error handling logic. r=mccr8 The error handling logic here would fail to remove a stale raw pointer from mInProgressImports if the EnsureURI() call failed. Fortunately, it's not actually possible for EnsureURI() to fail in this case, because the EnsureResolvedURI() call above already implies EnsureURI(). That said, we're better off structuring this code to ensure that we never leave a value in mInProgressImports after we exit the scope. MozReview-Commit-ID: 8mnKcHL75x
js/xpconnect/loader/mozJSComponentLoader.cpp
--- a/js/xpconnect/loader/mozJSComponentLoader.cpp
+++ b/js/xpconnect/loader/mozJSComponentLoader.cpp
@@ -1259,18 +1259,18 @@ mozJSComponentLoader::Import(JSContext* 
 
     ModuleEntry* mod;
     nsAutoPtr<ModuleEntry> newEntry;
     if (!mImports.Get(info.Key(), &mod) && !mInProgressImports.Get(info.Key(), &mod)) {
         newEntry = new ModuleEntry(RootingContext::get(aCx));
         if (!newEntry)
             return NS_ERROR_OUT_OF_MEMORY;
 
-        rv = info.EnsureResolvedURI();
-        NS_ENSURE_SUCCESS(rv, rv);
+        // Note: This implies EnsureURI().
+        MOZ_TRY(info.EnsureResolvedURI());
 
         // get the JAR if there is one
         nsCOMPtr<nsIJARURI> jarURI;
         jarURI = do_QueryInterface(info.ResolvedURI(), &rv);
         nsCOMPtr<nsIFileURL> baseFileURL;
         if (NS_SUCCEEDED(rv)) {
             nsCOMPtr<nsIURI> baseURI;
             while (jarURI) {
@@ -1292,26 +1292,28 @@ mozJSComponentLoader::Import(JSContext* 
         NS_ENSURE_SUCCESS(rv, rv);
 
         nsCString* existingPath;
         if (mLocations.Get(newEntry->resolvedURL, &existingPath) && *existingPath != info.Key()) {
             return NS_ERROR_UNEXPECTED;
         }
 
         mLocations.Put(newEntry->resolvedURL, new nsCString(info.Key()));
-        mInProgressImports.Put(info.Key(), newEntry);
 
-        rv = info.EnsureURI();
-        NS_ENSURE_SUCCESS(rv, rv);
         RootedValue exception(aCx);
-        rv = ObjectForLocation(info, sourceFile, &newEntry->obj,
-                               &newEntry->thisObjectKey,
-                               &newEntry->location, true, &exception);
+        {
+            mInProgressImports.Put(info.Key(), newEntry);
+            auto cleanup = MakeScopeExit([&] () {
+                mInProgressImports.Remove(info.Key());
+            });
 
-        mInProgressImports.Remove(info.Key());
+            rv = ObjectForLocation(info, sourceFile, &newEntry->obj,
+                                   &newEntry->thisObjectKey,
+                                   &newEntry->location, true, &exception);
+        }
 
         if (NS_FAILED(rv)) {
             if (!exception.isUndefined()) {
                 // An exception was thrown during compilation. Propagate it
                 // out to our caller so they can report it.
                 if (!JS_WrapValue(aCx, &exception))
                     return NS_ERROR_OUT_OF_MEMORY;
                 JS_SetPendingException(aCx, exception);