Bug 1333573 P2 Make nested worker pass WorkerLoadInfo to main thread when getting channel. r=baku
authorBen Kelly <ben@wanderview.com>
Tue, 07 Feb 2017 10:28:38 -0500
changeset 387836 b42d5fed4729b4aed9d9485d8573ddbfbc077746
parent 387835 8453100666999fa2bcc6d075cd6296b730e04519
child 387837 d32c5ac40db9baac2ea258cd0d1c21278deb515f
push id7198
push userjlorenzo@mozilla.com
push dateTue, 18 Apr 2017 12:07:49 +0000
treeherdermozilla-beta@d57aa49c3948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1333573
milestone54.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 1333573 P2 Make nested worker pass WorkerLoadInfo to main thread when getting channel. r=baku
dom/workers/ScriptLoader.cpp
dom/workers/ScriptLoader.h
dom/workers/WorkerPrivate.cpp
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -1790,27 +1790,27 @@ CacheScriptLoader::OnStreamComplete(nsIS
   mRunnable->DataReceivedFromCache(mIndex, aString, aStringLen, mChannelInfo,
                                    Move(mPrincipalInfo));
   return NS_OK;
 }
 
 class ChannelGetterRunnable final : public WorkerMainThreadRunnable
 {
   const nsAString& mScriptURL;
-  nsIChannel** mChannel;
+  WorkerLoadInfo& mLoadInfo;
   nsresult mResult;
 
 public:
   ChannelGetterRunnable(WorkerPrivate* aParentWorker,
                         const nsAString& aScriptURL,
-                        nsIChannel** aChannel)
+                        WorkerLoadInfo& aLoadInfo)
     : WorkerMainThreadRunnable(aParentWorker,
                                NS_LITERAL_CSTRING("ScriptLoader :: ChannelGetter"))
     , mScriptURL(aScriptURL)
-    , mChannel(aChannel)
+    , mLoadInfo(aLoadInfo)
     , mResult(NS_ERROR_FAILURE)
   {
     MOZ_ASSERT(aParentWorker);
     aParentWorker->AssertIsOnWorkerThread();
   }
 
   virtual bool
   MainThreadRun() override
@@ -1835,17 +1835,17 @@ public:
                                                    parentDoc, loadGroup,
                                                    mScriptURL,
                                                    // Nested workers are always dedicated.
                                                    nsIContentPolicy::TYPE_INTERNAL_WORKER,
                                                    // Nested workers use default uri encoding.
                                                    true,
                                                    getter_AddRefs(channel));
     if (NS_SUCCEEDED(mResult)) {
-      channel.forget(mChannel);
+      mLoadInfo.mChannel = channel.forget();
     }
 
     return true;
   }
 
   nsresult
   GetResult() const
   {
@@ -2173,22 +2173,22 @@ ChannelFromScriptURLMainThread(nsIPrinci
                               aContentPolicyType, nsIRequest::LOAD_NORMAL,
                               aDefaultURIEncoding, aChannel);
 }
 
 nsresult
 ChannelFromScriptURLWorkerThread(JSContext* aCx,
                                  WorkerPrivate* aParent,
                                  const nsAString& aScriptURL,
-                                 nsIChannel** aChannel)
+                                 WorkerLoadInfo& aLoadInfo)
 {
   aParent->AssertIsOnWorkerThread();
 
   RefPtr<ChannelGetterRunnable> getter =
-    new ChannelGetterRunnable(aParent, aScriptURL, aChannel);
+    new ChannelGetterRunnable(aParent, aScriptURL, aLoadInfo);
 
   ErrorResult rv;
   getter->Dispatch(Terminating, rv);
   if (rv.Failed()) {
     NS_ERROR("Failed to dispatch!");
     return rv.StealNSResult();
   }
 
--- a/dom/workers/ScriptLoader.h
+++ b/dom/workers/ScriptLoader.h
@@ -41,17 +41,17 @@ ChannelFromScriptURLMainThread(nsIPrinci
                                nsContentPolicyType aContentPolicyType,
                                bool aDefaultURIEncoding,
                                nsIChannel** aChannel);
 
 nsresult
 ChannelFromScriptURLWorkerThread(JSContext* aCx,
                                  WorkerPrivate* aParent,
                                  const nsAString& aScriptURL,
-                                 nsIChannel** aChannel);
+                                 WorkerLoadInfo& aLoadInfo);
 
 void ReportLoadError(ErrorResult& aRv, nsresult aLoadResult,
                      const nsAString& aScriptURL);
 
 void LoadMainScript(WorkerPrivate* aWorkerPrivate,
                     const nsAString& aScriptURL,
                     WorkerScriptType aWorkerScriptType,
                     ErrorResult& aRv);
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -4448,21 +4448,20 @@ WorkerPrivate::GetLoadInfo(JSContext* aC
       MutexAutoLock lock(aParent->mMutex);
       parentStatus = aParent->mStatus;
     }
 
     if (parentStatus > Running) {
       return NS_ERROR_FAILURE;
     }
 
-    // StartAssignment() is used instead getter_AddRefs because, getter_AddRefs
-    // does QI in debug build and, if this worker runs in a child process,
-    // HttpChannelChild will crash because it's not thread-safe.
+    // Passing a pointer to our stack loadInfo is safe here because this
+    // method uses a sync runnable to get the channel from the main thread.
     rv = ChannelFromScriptURLWorkerThread(aCx, aParent, aScriptURL,
-                                          loadInfo.mChannel.StartAssignment());
+                                          loadInfo);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // Now that we've spun the loop there's no guarantee that our parent is
     // still alive.  We may have received control messages initiating shutdown.
     {
       MutexAutoLock lock(aParent->mMutex);
       parentStatus = aParent->mStatus;
     }