Bug 1022773 - Return value rooting for workers, r=bent, a=abillings
authorSteve Fink <sfink@mozilla.com>
Tue, 01 Jul 2014 14:23:31 -0700
changeset 208639 17e4424469045396e42f3c8067fabda05a47b67a
parent 208638 87216353f2dc4cc7665e2ae3d233300777b34406
child 208640 07d419bd7fd0ef4244d27b8f9b5dbd56b076d2e5
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent, abillings
bugs1022773
milestone32.0a2
Bug 1022773 - Return value rooting for workers, r=bent, a=abillings
dom/workers/Navigator.cpp
dom/workers/WorkerPrivate.cpp
--- a/dom/workers/Navigator.cpp
+++ b/dom/workers/Navigator.cpp
@@ -112,44 +112,47 @@ GetDataStoresStructuredCloneCallbacksRea
 
   // Read the holder from the buffer, which points to the data store.
   nsMainThreadPtrHolder<DataStore>* dataStoreholder;
   if (!JS_ReadBytes(aReader, &dataStoreholder, sizeof(dataStoreholder))) {
     MOZ_ASSERT(false, "cannot read bytes for dataStoreholder!");
     return nullptr;
   }
 
-  nsRefPtr<WorkerDataStore> workerStore =
-    new WorkerDataStore(workerPrivate->GlobalScope());
-  nsMainThreadPtrHandle<DataStore> backingStore = dataStoreholder;
+  // Protect workerStoreObj from moving GC during ~nsRefPtr.
+  JS::Rooted<JSObject*> workerStoreObj(aCx, nullptr);
+  {
+    nsRefPtr<WorkerDataStore> workerStore =
+      new WorkerDataStore(workerPrivate->GlobalScope());
+    nsMainThreadPtrHandle<DataStore> backingStore = dataStoreholder;
 
-  // When we're on the worker thread, prepare a DataStoreChangeEventProxy.
-  nsRefPtr<DataStoreChangeEventProxy> eventProxy =
-    new DataStoreChangeEventProxy(workerPrivate, workerStore);
+    // When we're on the worker thread, prepare a DataStoreChangeEventProxy.
+    nsRefPtr<DataStoreChangeEventProxy> eventProxy =
+      new DataStoreChangeEventProxy(workerPrivate, workerStore);
 
-  // Add the DataStoreChangeEventProxy as an event listener on the main thread.
-  nsRefPtr<DataStoreAddEventListenerRunnable> runnable =
-    new DataStoreAddEventListenerRunnable(workerPrivate,
-                                          backingStore,
-                                          eventProxy);
-  runnable->Dispatch(aCx);
+    // Add the DataStoreChangeEventProxy as an event listener on the main thread.
+    nsRefPtr<DataStoreAddEventListenerRunnable> runnable =
+      new DataStoreAddEventListenerRunnable(workerPrivate,
+                                            backingStore,
+                                            eventProxy);
+    runnable->Dispatch(aCx);
 
-  // Point WorkerDataStore to DataStore.
-  workerStore->SetBackingDataStore(backingStore);
+    // Point WorkerDataStore to DataStore.
+    workerStore->SetBackingDataStore(backingStore);
 
-  JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
-  if (!global) {
-    MOZ_ASSERT(false, "cannot get global!");
-    return nullptr;
-  }
-
-  JS::Rooted<JSObject*> workerStoreObj(aCx, workerStore->WrapObject(aCx));
-  if (!JS_WrapObject(aCx, &workerStoreObj)) {
-    MOZ_ASSERT(false, "cannot wrap object for workerStoreObj!");
-    return nullptr;
+    JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
+    if (!global) {
+      MOZ_ASSERT(false, "cannot get global!");
+    } else {
+      workerStoreObj = workerStore->WrapObject(aCx);
+      if (!JS_WrapObject(aCx, &workerStoreObj)) {
+        MOZ_ASSERT(false, "cannot wrap object for workerStoreObj!");
+        workerStoreObj = nullptr;
+      }
+    }
   }
 
   return workerStoreObj;
 }
 
 static bool
 GetDataStoresStructuredCloneCallbacksWrite(JSContext* aCx,
                                            JSStructuredCloneWriter* aWriter,
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -359,21 +359,25 @@ struct WorkerStructuredCloneCallbacks
       JS::Rooted<JS::Value> dataArray(aCx);
       if (!JS_ReadUint32Pair(aReader, &width, &height) ||
           !JS_ReadTypedArray(aReader, &dataArray))
       {
         return nullptr;
       }
       MOZ_ASSERT(dataArray.isObject());
 
-      // Construct the ImageData.
-      nsRefPtr<ImageData> imageData = new ImageData(width, height,
-                                                    dataArray.toObject());
-      // Wrap it in a JS::Value.
-      return imageData->WrapObject(aCx);
+      JS::Rooted<JSObject*> result(aCx);
+      {
+        // Construct the ImageData.
+        nsRefPtr<ImageData> imageData = new ImageData(width, height,
+                                                      dataArray.toObject());
+        // Wrap it in a JS::Value, protected from a moving GC during ~nsRefPtr.
+        result = imageData->WrapObject(aCx);
+      }
+      return result;
     }
 
     Error(aCx, 0);
     return nullptr;
   }
 
   static bool
   Write(JSContext* aCx, JSStructuredCloneWriter* aWriter,