Bug 1438945 - Pass around URIs instead of strings in RemoteWorker. r=asuth
authorBlake Kaplan <mrbkap@gmail.com>
Mon, 19 Nov 2018 15:18:34 -0800
changeset 503579 7733e4286f9c615bde46ad140329ab73dd613e1e
parent 503578 5fa24f8069cbffe7635ca7b928956cea63aa5763
child 503580 fab208773c488ca77ce4778ad40598c5bc51c1b2
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1438945
milestone65.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 1438945 - Pass around URIs instead of strings in RemoteWorker. r=asuth In order to fix the problem mentioned in comment 91 & co, we need to hold onto the URI object that we resolve in the child process when we construct the SharedWorker. Otherwise, we risk the Blob getting deallocated from under us. This patch isn't sufficient to fix that problem, however, because the worker code itself ends up going back through strings. I fix that in the next couple of patches. Differential Revision: https://phabricator.services.mozilla.com/D11825
dom/workers/remoteworkers/RemoteWorkerChild.cpp
dom/workers/remoteworkers/RemoteWorkerTypes.ipdlh
dom/workers/sharedworkers/SharedWorker.cpp
dom/workers/sharedworkers/SharedWorkerManager.cpp
dom/workers/sharedworkers/SharedWorkerManager.h
dom/workers/sharedworkers/SharedWorkerService.cpp
--- a/dom/workers/remoteworkers/RemoteWorkerChild.cpp
+++ b/dom/workers/remoteworkers/RemoteWorkerChild.cpp
@@ -11,16 +11,17 @@
 #include "mozilla/dom/RemoteWorkerTypes.h"
 #include "mozilla/dom/ServiceWorkerInterceptController.h"
 #include "mozilla/dom/workerinternals/ScriptLoader.h"
 #include "mozilla/dom/WorkerError.h"
 #include "mozilla/dom/WorkerPrivate.h"
 #include "mozilla/dom/WorkerRef.h"
 #include "mozilla/dom/WorkerRunnable.h"
 #include "mozilla/ipc/BackgroundUtils.h"
+#include "mozilla/ipc/URIUtils.h"
 #include "nsIConsoleReportCollector.h"
 #include "nsIPrincipal.h"
 #include "nsNetUtil.h"
 #include "nsProxyRelease.h"
 
 namespace mozilla {
 
 using namespace ipc;
@@ -281,27 +282,18 @@ RemoteWorkerChild::ExecWorkerOnMainThrea
   rv = PopulatePrincipalContentSecurityPolicy(loadingPrincipal,
                                               aData.loadingPrincipalCsp(),
                                               aData.loadingPrincipalPreloadCsp());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   WorkerLoadInfo info;
-  rv = NS_NewURI(getter_AddRefs(info.mBaseURI), aData.baseScriptURL(),
-                 nullptr, nullptr);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  rv = NS_NewURI(getter_AddRefs(info.mResolvedScriptURI),
-                 aData.resolvedScriptURL(), nullptr, info.mBaseURI);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
+  info.mBaseURI = DeserializeURI(aData.baseScriptURL());
+  info.mResolvedScriptURI = DeserializeURI(aData.resolvedScriptURL());
 
   info.mPrincipalInfo = new PrincipalInfo(aData.principalInfo());
 
   info.mDomain = aData.domain();
   info.mPrincipal = principal;
   info.mLoadingPrincipal = loadingPrincipal;
 
   nsContentUtils::StorageAccess access =
--- a/dom/workers/remoteworkers/RemoteWorkerTypes.ipdlh
+++ b/dom/workers/remoteworkers/RemoteWorkerTypes.ipdlh
@@ -1,32 +1,43 @@
 /* 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/. */
 
 include ClientIPCTypes;
 include PBackgroundSharedTypes;
+include URIParams;
 
 using struct mozilla::void_t from "ipc/IPCMessageUtils.h";
 
 namespace mozilla {
 namespace dom {
 
 struct ContentSecurityPolicy
 {
   nsString policy;
   bool reportOnlyFlag;
   bool deliveredViaMetaTagFlag;
 };
 
 struct RemoteWorkerData
 {
+  // This should only be used for devtools.
   nsString originalScriptURL;
-  nsCString baseScriptURL;
-  nsCString resolvedScriptURL;
+
+  // It is important to pass these as URIParams instead of strings for blob
+  // URLs: they carry an additional bit of state with them (mIsRevoked) that
+  // gives us a chance to use them, even after they've been revoked. Because
+  // we're asynchronously calling into the parent process before potentially
+  // loading the worker, it is important to keep this state. Note that this
+  // isn't a panacea: once the URL has been revoked, it'll give the worker 5
+  // seconds to actually load it; so it's possible to still fail to load the
+  // blob URL if it takes too long to do the round trip.
+  URIParams baseScriptURL;
+  URIParams resolvedScriptURL;
 
   nsString name;
 
   PrincipalInfo loadingPrincipalInfo;
   ContentSecurityPolicy[] loadingPrincipalCsp;
   ContentSecurityPolicy[] loadingPrincipalPreloadCsp;
 
   PrincipalInfo principalInfo;
--- a/dom/workers/sharedworkers/SharedWorker.cpp
+++ b/dom/workers/sharedworkers/SharedWorker.cpp
@@ -18,16 +18,17 @@
 #include "mozilla/dom/SharedWorkerBinding.h"
 #include "mozilla/dom/SharedWorkerChild.h"
 #include "mozilla/dom/WorkerBinding.h"
 #include "mozilla/dom/WorkerLoadInfo.h"
 #include "mozilla/dom/WorkerPrivate.h"
 #include "mozilla/ipc/BackgroundChild.h"
 #include "mozilla/ipc/BackgroundUtils.h"
 #include "mozilla/ipc/PBackgroundChild.h"
+#include "mozilla/ipc/URIUtils.h"
 #include "nsContentUtils.h"
 #include "nsGlobalWindowInner.h"
 #include "nsPIDOMWindow.h"
 
 #ifdef XP_WIN
 #undef PostMessage
 #endif
 
@@ -218,27 +219,21 @@ SharedWorker::Constructor(const GlobalOb
   RefPtr<MessageChannel> channel = MessageChannel::Constructor(global, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   MessagePortIdentifier portIdentifier;
   channel->Port1()->CloneAndDisentangle(portIdentifier);
 
-  nsAutoCString resolvedScriptURL;
-  aRv = loadInfo.mResolvedScriptURI->GetSpec(resolvedScriptURL);
-  if (NS_WARN_IF(aRv.Failed())) {
-    return nullptr;
-  }
+  URIParams resolvedScriptURL;
+  SerializeURI(loadInfo.mResolvedScriptURI, resolvedScriptURL);
 
-  nsAutoCString baseURL;
-  aRv = loadInfo.mBaseURI->GetSpec(baseURL);
-  if (NS_WARN_IF(aRv.Failed())) {
-    return nullptr;
-  }
+  URIParams baseURL;
+  SerializeURI(loadInfo.mBaseURI, baseURL);
 
   // Register this component to PBackground.
   PBackgroundChild* actorChild = BackgroundChild::GetOrCreateForCurrentThread();
 
   bool isSecureContext = JS::GetIsSecureContext(js::GetContextRealm(cx));
 
   OptionalIPCClientInfo ipcClientInfo;
   Maybe<ClientInfo> clientInfo = window->GetClientInfo();
--- a/dom/workers/sharedworkers/SharedWorkerManager.cpp
+++ b/dom/workers/sharedworkers/SharedWorkerManager.cpp
@@ -4,32 +4,33 @@
  * 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/. */
 
 #include "SharedWorkerManager.h"
 #include "SharedWorkerParent.h"
 #include "SharedWorkerService.h"
 #include "mozilla/dom/PSharedWorker.h"
 #include "mozilla/ipc/BackgroundParent.h"
+#include "mozilla/ipc/URIUtils.h"
 #include "mozilla/dom/RemoteWorkerController.h"
 #include "nsIConsoleReportCollector.h"
 #include "nsINetworkInterceptController.h"
 #include "nsIPrincipal.h"
 #include "nsProxyRelease.h"
 
 namespace mozilla {
 namespace dom {
 
 SharedWorkerManager::SharedWorkerManager(nsIEventTarget* aPBackgroundEventTarget,
                                          const RemoteWorkerData& aData,
                                          nsIPrincipal* aLoadingPrincipal)
   : mPBackgroundEventTarget(aPBackgroundEventTarget)
   , mLoadingPrincipal(aLoadingPrincipal)
   , mDomain(aData.domain())
-  , mResolvedScriptURL(aData.resolvedScriptURL())
+  , mResolvedScriptURL(DeserializeURI(aData.resolvedScriptURL()))
   , mName(aData.name())
   , mIsSecureContext(aData.isSecureContext())
   , mSuspended(false)
   , mFrozen(false)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aLoadingPrincipal);
 }
@@ -66,23 +67,28 @@ SharedWorkerManager::MaybeCreateRemoteWo
   }
 
   mRemoteWorkerController->AddPortIdentifier(aPortIdentifier);
   return true;
 }
 
 bool
 SharedWorkerManager::MatchOnMainThread(const nsACString& aDomain,
-                                       const nsACString& aScriptURL,
+                                       nsIURI* aScriptURL,
                                        const nsAString& aName,
                                        nsIPrincipal* aLoadingPrincipal) const
 {
   MOZ_ASSERT(NS_IsMainThread());
+  bool urlEquals;
+  if (NS_FAILED(aScriptURL->Equals(mResolvedScriptURL, &urlEquals))) {
+    return false;
+  }
+
   return aDomain == mDomain &&
-         aScriptURL == mResolvedScriptURL &&
+         urlEquals &&
          aName == mName &&
          // We want to be sure that the window's principal subsumes the
          // SharedWorker's loading principal and vice versa.
          mLoadingPrincipal->Subsumes(aLoadingPrincipal) &&
          aLoadingPrincipal->Subsumes(mLoadingPrincipal);
 }
 
 void
--- a/dom/workers/sharedworkers/SharedWorkerManager.h
+++ b/dom/workers/sharedworkers/SharedWorkerManager.h
@@ -28,17 +28,17 @@ public:
   // Called on main-thread thread methods
 
   SharedWorkerManager(nsIEventTarget* aPBackgroundEventTarget,
                       const RemoteWorkerData& aData,
                       nsIPrincipal* aLoadingPrincipal);
 
   bool
   MatchOnMainThread(const nsACString& aDomain,
-                    const nsACString& aScriptURL,
+                    nsIURI* aScriptURL,
                     const nsAString& aName,
                     nsIPrincipal* aLoadingPrincipal) const;
 
   // RemoteWorkerObserver
 
   void
   CreationFailed() override;
 
@@ -76,17 +76,17 @@ public:
 
 private:
   ~SharedWorkerManager();
 
   nsCOMPtr<nsIEventTarget> mPBackgroundEventTarget;
 
   nsCOMPtr<nsIPrincipal> mLoadingPrincipal;
   nsCString mDomain;
-  nsCString mResolvedScriptURL;
+  nsCOMPtr<nsIURI> mResolvedScriptURL;
   nsString mName;
   bool mIsSecureContext;
   bool mSuspended;
   bool mFrozen;
 
   // Raw pointers because SharedWorkerParent unregisters itself in ActorDestroy().
   nsTArray<SharedWorkerParent*> mActors;
 
--- a/dom/workers/sharedworkers/SharedWorkerService.cpp
+++ b/dom/workers/sharedworkers/SharedWorkerService.cpp
@@ -299,19 +299,20 @@ SharedWorkerService::GetOrCreateWorkerMa
                                               aData.loadingPrincipalCsp(),
                                               aData.loadingPrincipalPreloadCsp());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     ErrorPropagationOnMainThread(aBackgroundEventTarget, aActor, rv);
     return;
   }
 
   // Let's see if there is already a SharedWorker to share.
+  nsCOMPtr<nsIURI> resolvedScriptURL = DeserializeURI(aData.resolvedScriptURL());
   for (SharedWorkerManager* workerManager : mWorkerManagers) {
     if (workerManager->MatchOnMainThread(aData.domain(),
-                                         aData.resolvedScriptURL(),
+                                         resolvedScriptURL,
                                          aData.name(), loadingPrincipal)) {
       manager = workerManager;
       break;
     }
   }
 
   // Let's create a new one.
   if (!manager) {