Bug 1469873 Make ClientSource::Control() fail if storage access is not available and make claim() respect the result. r=mrbkap a=lizzard
authorBen Kelly <ben@wanderview.com>
Thu, 28 Jun 2018 12:58:23 -0700
changeset 477789 cb10387ab1cff10b532ced4ad1ef965df964c427
parent 477788 ea5d50cccb4b7979734751bb51a161f4f33cb084
child 477790 801112336847960bbb9a018695cf09ea437dc137
push id9423
push userarchaeopteryx@coole-files.de
push dateMon, 02 Jul 2018 16:45:36 +0000
treeherdermozilla-beta@cb10387ab1cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap, lizzard
bugs1469873
milestone62.0
Bug 1469873 Make ClientSource::Control() fail if storage access is not available and make claim() respect the result. r=mrbkap a=lizzard
dom/clients/manager/ClientManagerService.cpp
dom/clients/manager/ClientSource.cpp
dom/clients/manager/ClientSource.h
dom/clients/manager/ClientSourceOpParent.cpp
dom/clients/manager/ClientSourceOpParent.h
dom/clients/manager/ClientSourceParent.cpp
dom/clients/manager/ClientSourceParent.h
dom/serviceworkers/ServiceWorkerManager.cpp
--- a/dom/clients/manager/ClientManagerService.cpp
+++ b/dom/clients/manager/ClientManagerService.cpp
@@ -470,16 +470,18 @@ ClaimOnMainThread(const ClientInfo& aCli
 
       RefPtr<GenericPromise> inner = swm->MaybeClaimClient(clientInfo, desc);
       inner->Then(SystemGroup::EventTargetFor(TaskCategory::Other), __func__,
         [promise] (bool aResult) {
           promise->Resolve(NS_OK, __func__);
         }, [promise] (nsresult aRv) {
           promise->Reject(aRv, __func__);
         });
+
+      scopeExit.release();
     });
 
   MOZ_ALWAYS_SUCCEEDS(SystemGroup::Dispatch(TaskCategory::Other, r.forget()));
 
   return promise.forget();
 }
 
 } // anonymous namespace
--- a/dom/clients/manager/ClientSource.cpp
+++ b/dom/clients/manager/ClientSource.cpp
@@ -119,16 +119,38 @@ ClientSource::GetDocShell() const
 {
   NS_ASSERT_OWNINGTHREAD(ClientSource);
   if (!mOwner.is<nsCOMPtr<nsIDocShell>>()) {
     return nullptr;
   }
   return mOwner.as<nsCOMPtr<nsIDocShell>>();
 }
 
+nsIGlobalObject*
+ClientSource::GetGlobal() const
+{
+  NS_ASSERT_OWNINGTHREAD(ClientSource);
+  nsPIDOMWindowInner* win = GetInnerWindow();
+  if (win) {
+    return win->AsGlobal();
+  }
+
+  WorkerPrivate* wp = GetWorkerPrivate();
+  if (wp) {
+    return wp->GlobalScope();
+  }
+
+  // Note, ClientSource objects attached to docshell for conceptual
+  // initial about:blank will get nullptr here.  The caller should
+  // use MaybeCreateIntitialDocument() to create the window before
+  // GetGlobal() if it wants this before.
+
+  return nullptr;
+}
+
 void
 ClientSource::MaybeCreateInitialDocument()
 {
   nsIDocShell* docshell = GetDocShell();
   if (docshell) {
     // Force the create of the initial document if it does not exist yet.
     Unused << docshell->GetDocument();
 
@@ -426,20 +448,57 @@ ClientSource::SetController(const Servic
   }
 }
 
 RefPtr<ClientOpPromise>
 ClientSource::Control(const ClientControlledArgs& aArgs)
 {
   NS_ASSERT_OWNINGTHREAD(ClientSource);
 
+  // Determine if the client is allowed to be controlled.  Currently we
+  // prevent service workers from controlling clients that cannot access
+  // storage.  We exempt this restriction for local URL clients, like about:blank
+  // and blob:, since access to service workers is dictated by their parent.
+  //
+  // Note, we default to allowing the client to be controlled in the case
+  // where we are not execution ready yet.  This can only happen if the
+  // the non-subresource load is intercepted by a service worker.  Since
+  // ServiceWorkerInterceptController() uses StorageAllowedForChannel()
+  // it should be fine to accept these control messages.
+  //
+  // Its also fine to default to allowing ClientSource attached to a docshell
+  // to be controlled.  These clients represent inital about:blank windows
+  // that do not have an inner window created yet.  We explicitly allow initial
+  // about:blank.
+  bool controlAllowed = true;
+  if (GetInnerWindow()) {
+
+    // Local URL windows and windows with access to storage can be controlled.
+    controlAllowed = Info().URL().LowerCaseEqualsLiteral("about:blank") ||
+                     StringBeginsWith(Info().URL(), NS_LITERAL_CSTRING("blob:")) ||
+                     nsContentUtils::StorageAllowedForWindow(GetInnerWindow()) ==
+                      nsContentUtils::StorageAccess::eAllow;
+  } else if (GetWorkerPrivate()) {
+    // Local URL workers and workers with access to storage cna be controlled.
+    controlAllowed = GetWorkerPrivate()->IsStorageAllowed() ||
+                     StringBeginsWith(GetWorkerPrivate()->ScriptURL(),
+                                      NS_LITERAL_STRING("blob:"));
+  }
+
+  RefPtr<ClientOpPromise> ref;
+
+  if (NS_WARN_IF(!controlAllowed)) {
+    ref = ClientOpPromise::CreateAndReject(NS_ERROR_DOM_INVALID_STATE_ERR,
+                                           __func__);
+    return ref.forget();
+  }
+
   SetController(ServiceWorkerDescriptor(aArgs.serviceWorker()));
 
-  RefPtr<ClientOpPromise> ref =
-    ClientOpPromise::CreateAndResolve(NS_OK, __func__);
+  ref = ClientOpPromise::CreateAndResolve(NS_OK, __func__);
   return ref.forget();
 }
 
 void
 ClientSource::InheritController(const ServiceWorkerDescriptor& aServiceWorker)
 {
   NS_ASSERT_OWNINGTHREAD(ClientSource);
 
@@ -665,45 +724,65 @@ ClientSource::PostMessage(const ClientPo
 
   ref = ClientOpPromise::CreateAndResolve(NS_OK, __func__);
   return ref.forget();
 }
 
 RefPtr<ClientOpPromise>
 ClientSource::Claim(const ClientClaimArgs& aArgs)
 {
+  // The ClientSource::Claim method is only needed in the legacy
+  // mode where the ServiceWorkerManager is run in each child-process.
+  // In parent-process mode this method should not be called.
+  MOZ_DIAGNOSTIC_ASSERT(!ServiceWorkerParentInterceptEnabled());
+
   RefPtr<ClientOpPromise> ref;
 
+  nsIGlobalObject* global = GetGlobal();
+  if (NS_WARN_IF(!global)) {
+    ref = ClientOpPromise::CreateAndReject(NS_ERROR_DOM_INVALID_STATE_ERR,
+                                           __func__);
+    return ref.forget();
+  }
+
+  // Note, we cannot just mark the ClientSource controlled.  We must go through
+  // the SWM so that it can keep track of which clients are controlled by each
+  // registration.  We must tell the child-process SWM in legacy child-process
+  // mode.  In parent-process service worker mode the SWM is notified in the
+  // parent-process in ClientManagerService::Claim().
+
+  RefPtr<GenericPromise::Private> innerPromise =
+    new GenericPromise::Private(__func__);
   ServiceWorkerDescriptor swd(aArgs.serviceWorker());
 
-  // Today the ServiceWorkerManager maintains its own list of
-  // nsIDocument objects controlled by each service worker.  We
-  // need to try to update that data structure for now.  If we
-  // can't, however, then simply mark the Client as controlled.
-  // In the future this will be enough for the SWM as well since
-  // it will eventually hold ClientHandle objects instead of
-  // nsIDocuments.
-  nsPIDOMWindowInner* innerWindow = GetInnerWindow();
-  nsIDocument* doc = innerWindow ? innerWindow->GetExtantDoc() : nullptr;
-  RefPtr<ServiceWorkerManager> swm = doc ? ServiceWorkerManager::GetInstance()
-                                         : nullptr;
-  if (!swm || !doc) {
-    SetController(swd);
-    ref = ClientOpPromise::CreateAndResolve(NS_OK, __func__);
-    return ref.forget();
+  nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
+    "ClientSource::Claim",
+    [innerPromise, clientInfo = mClientInfo, swd] () mutable {
+      RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+      if (NS_WARN_IF(!swm)) {
+        innerPromise->Reject(NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
+        return;
+      }
+
+      RefPtr<GenericPromise> p = swm->MaybeClaimClient(clientInfo, swd);
+      p->ChainTo(innerPromise.forget(), __func__);
+    });
+
+  if (NS_IsMainThread()) {
+    r->Run();
+  } else {
+    MOZ_ALWAYS_SUCCEEDS(SystemGroup::Dispatch(TaskCategory::Other, r.forget()));
   }
 
   RefPtr<ClientOpPromise::Private> outerPromise =
     new ClientOpPromise::Private(__func__);
 
-  auto holder =
-    MakeRefPtr<DOMMozPromiseRequestHolder<GenericPromise>>(innerWindow->AsGlobal());
+  auto holder = MakeRefPtr<DOMMozPromiseRequestHolder<GenericPromise>>(global);
 
-  RefPtr<GenericPromise> p = swm->MaybeClaimClient(mClientInfo, swd);
-  p->Then(mEventTarget, __func__,
+  innerPromise->Then(mEventTarget, __func__,
     [outerPromise, holder] (bool aResult) {
       holder->Complete();
       outerPromise->Resolve(NS_OK, __func__);
     }, [outerPromise, holder] (nsresult aResult) {
       holder->Complete();
       outerPromise->Reject(aResult, __func__);
     })->Track(*holder);
 
--- a/dom/clients/manager/ClientSource.h
+++ b/dom/clients/manager/ClientSource.h
@@ -12,16 +12,17 @@
 #include "mozilla/dom/ServiceWorkerDescriptor.h"
 #include "mozilla/Variant.h"
 
 #ifdef XP_WIN
 #undef PostMessage
 #endif
 
 class nsIDocShell;
+class nsIGlobalObject;
 class nsISerialEventTarget;
 class nsPIDOMWindowInner;
 
 namespace mozilla {
 namespace dom {
 
 class ClientClaimArgs;
 class ClientControlledArgs;
@@ -73,16 +74,19 @@ class ClientSource final : public Client
   ExecutionReady(const ClientSourceExecutionReadyArgs& aArgs);
 
   WorkerPrivate*
   GetWorkerPrivate() const;
 
   nsIDocShell*
   GetDocShell() const;
 
+  nsIGlobalObject*
+  GetGlobal() const;
+
   void
   MaybeCreateInitialDocument();
 
   nsresult
   SnapshotWindowState(ClientState* aStateOut);
 
   // Private methods called by ClientManager
   ClientSource(ClientManager* aManager,
--- a/dom/clients/manager/ClientSourceOpParent.cpp
+++ b/dom/clients/manager/ClientSourceOpParent.cpp
@@ -1,16 +1,18 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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 "ClientSourceOpParent.h"
 
+#include "ClientSourceParent.h"
+
 namespace mozilla {
 namespace dom {
 
 using mozilla::ipc::IPCResult;
 
 void
 ClientSourceOpParent::ActorDestroy(ActorDestroyReason aReason)
 {
@@ -20,28 +22,41 @@ ClientSourceOpParent::ActorDestroy(Actor
   }
 }
 
 IPCResult
 ClientSourceOpParent::Recv__delete__(const ClientOpResult& aResult)
 {
   if (aResult.type() == ClientOpResult::Tnsresult &&
       NS_FAILED(aResult.get_nsresult())) {
+
+    // If a control message fails then clear the controller from
+    // the ClientSourceParent.  We eagerly marked it controlled at
+    // the start of the operation.
+    if (mArgs.type() == ClientOpConstructorArgs::TClientControlledArgs) {
+      auto source = static_cast<ClientSourceParent*>(Manager());
+      if (source) {
+        source->ClearController();
+      }
+    }
+
     mPromise->Reject(aResult.get_nsresult(), __func__);
     mPromise = nullptr;
     return IPC_OK();
   }
+
   mPromise->Resolve(aResult, __func__);
   mPromise = nullptr;
   return IPC_OK();
 }
 
 ClientSourceOpParent::ClientSourceOpParent(const ClientOpConstructorArgs& aArgs,
                                            ClientOpPromise::Private* aPromise)
-  : mPromise(aPromise)
+  : mArgs(aArgs)
+  , mPromise(aPromise)
 {
   MOZ_DIAGNOSTIC_ASSERT(mPromise);
 }
 
 ClientSourceOpParent::~ClientSourceOpParent()
 {
   MOZ_DIAGNOSTIC_ASSERT(!mPromise);
 }
--- a/dom/clients/manager/ClientSourceOpParent.h
+++ b/dom/clients/manager/ClientSourceOpParent.h
@@ -9,16 +9,17 @@
 #include "mozilla/dom/ClientOpPromise.h"
 #include "mozilla/dom/PClientSourceOpParent.h"
 
 namespace mozilla {
 namespace dom {
 
 class ClientSourceOpParent final : public PClientSourceOpParent
 {
+  const ClientOpConstructorArgs mArgs;
   RefPtr<ClientOpPromise::Private> mPromise;
 
   // PClientSourceOpParent interface
   void
   ActorDestroy(ActorDestroyReason aReason) override;
 
   mozilla::ipc::IPCResult
   Recv__delete__(const ClientOpResult& aResult) override;
--- a/dom/clients/manager/ClientSourceParent.cpp
+++ b/dom/clients/manager/ClientSourceParent.cpp
@@ -270,16 +270,22 @@ ClientSourceParent::ExecutionReady() con
 
 const Maybe<ServiceWorkerDescriptor>&
 ClientSourceParent::GetController() const
 {
   return mController;
 }
 
 void
+ClientSourceParent::ClearController()
+{
+  mController.reset();
+}
+
+void
 ClientSourceParent::AttachHandle(ClientHandleParent* aClientHandle)
 {
   MOZ_DIAGNOSTIC_ASSERT(aClientHandle);
   MOZ_DIAGNOSTIC_ASSERT(!mFrozen);
   MOZ_ASSERT(!mHandleList.Contains(aClientHandle));
   mHandleList.AppendElement(aClientHandle);
 }
 
@@ -293,17 +299,21 @@ ClientSourceParent::DetachHandle(ClientH
 
 RefPtr<ClientOpPromise>
 ClientSourceParent::StartOp(const ClientOpConstructorArgs& aArgs)
 {
   RefPtr<ClientOpPromise::Private> promise =
     new ClientOpPromise::Private(__func__);
 
   // If we are being controlled, remember that data before propagating
-  // on to the ClientSource.
+  // on to the ClientSource.  This must be set prior to triggering
+  // the controllerchange event from the ClientSource since some tests
+  // expect matchAll() to find the controlled client immediately after.
+  // If the control operation fails, then we reset the controller value
+  // to reflect the final state.
   if (aArgs.type() == ClientOpConstructorArgs::TClientControlledArgs) {
     mController.reset();
     mController.emplace(aArgs.get_ClientControlledArgs().serviceWorker());
   }
 
   // Constructor failure will reject the promise via ActorDestroy().
   ClientSourceOpParent* actor = new ClientSourceOpParent(aArgs, promise);
   Unused << SendPClientSourceOpConstructor(actor, aArgs);
--- a/dom/clients/manager/ClientSourceParent.h
+++ b/dom/clients/manager/ClientSourceParent.h
@@ -75,16 +75,19 @@ public:
 
   bool
   ExecutionReady() const;
 
   const Maybe<ServiceWorkerDescriptor>&
   GetController() const;
 
   void
+  ClearController();
+
+  void
   AttachHandle(ClientHandleParent* aClientSource);
 
   void
   DetachHandle(ClientHandleParent* aClientSource);
 
   RefPtr<ClientOpPromise>
   StartOp(const ClientOpConstructorArgs& aArgs);
 };
--- a/dom/serviceworkers/ServiceWorkerManager.cpp
+++ b/dom/serviceworkers/ServiceWorkerManager.cpp
@@ -312,35 +312,52 @@ ServiceWorkerManager::Init(ServiceWorker
 RefPtr<GenericPromise>
 ServiceWorkerManager::StartControllingClient(const ClientInfo& aClientInfo,
                                              ServiceWorkerRegistrationInfo* aRegistrationInfo,
                                              bool aControlClientHandle)
 {
   MOZ_DIAGNOSTIC_ASSERT(aRegistrationInfo->GetActive());
 
   RefPtr<GenericPromise> ref;
+  RefPtr<ServiceWorkerManager> self(this);
 
   const ServiceWorkerDescriptor& active =
     aRegistrationInfo->GetActive()->Descriptor();
 
   auto entry = mControlledClients.LookupForAdd(aClientInfo.Id());
   if (entry) {
     RefPtr<ServiceWorkerRegistrationInfo> old =
       entry.Data()->mRegistrationInfo.forget();
 
-    ref = entry.Data()->mClientHandle->Control(active);
+    if (aControlClientHandle) {
+      ref = entry.Data()->mClientHandle->Control(active);
+    } else {
+      ref = GenericPromise::CreateAndResolve(false, __func__);
+    }
+
     entry.Data()->mRegistrationInfo = aRegistrationInfo;
 
     if (old != aRegistrationInfo) {
       StopControllingRegistration(old);
       aRegistrationInfo->StartControllingClient();
     }
 
     Telemetry::Accumulate(Telemetry::SERVICE_WORKER_CONTROLLED_DOCUMENTS, 1);
 
+    // Always check to see if we failed to actually control the client.  In
+    // that case removed the client from our list of controlled clients.
+    ref->Then(
+      SystemGroup::EventTargetFor(TaskCategory::Other), __func__,
+      [] (bool) {
+        // do nothing on success
+      }, [self, aClientInfo] (nsresult aRv) {
+        // failed to control, forget about this client
+        self->StopControllingClient(aClientInfo);
+      });
+
     return ref;
   }
 
   RefPtr<ClientHandle> clientHandle =
     ClientManager::CreateHandle(aClientInfo,
                                 SystemGroup::EventTargetFor(TaskCategory::Other));
 
   if (aControlClientHandle) {
@@ -350,25 +367,35 @@ ServiceWorkerManager::StartControllingCl
   }
 
   aRegistrationInfo->StartControllingClient();
 
   entry.OrInsert([&] {
     return new ControlledClientData(clientHandle, aRegistrationInfo);
   });
 
-  RefPtr<ServiceWorkerManager> self(this);
   clientHandle->OnDetach()->Then(
     SystemGroup::EventTargetFor(TaskCategory::Other), __func__,
-    [self = std::move(self), aClientInfo] {
+    [self, aClientInfo] {
       self->StopControllingClient(aClientInfo);
     });
 
   Telemetry::Accumulate(Telemetry::SERVICE_WORKER_CONTROLLED_DOCUMENTS, 1);
 
+  // Always check to see if we failed to actually control the client.  In
+  // that case removed the client from our list of controlled clients.
+  ref->Then(
+    SystemGroup::EventTargetFor(TaskCategory::Other), __func__,
+    [] (bool) {
+      // do nothing on success
+    }, [self, aClientInfo] (nsresult aRv) {
+      // failed to control, forget about this client
+      self->StopControllingClient(aClientInfo);
+    });
+
   return ref;
 }
 
 void
 ServiceWorkerManager::StopControllingClient(const ClientInfo& aClientInfo)
 {
   auto entry = mControlledClients.Lookup(aClientInfo.Id());
   if (!entry) {
@@ -2690,17 +2717,30 @@ ServiceWorkerManager::UpdateClientContro
     }
 
     handleList.AppendElement(iter.UserData()->mClientHandle);
   }
 
   // Fire event after iterating mControlledClients is done to prevent
   // modification by reentering from the event handlers during iteration.
   for (auto& handle : handleList) {
-    handle->Control(activeWorker->Descriptor());
+    RefPtr<GenericPromise> p = handle->Control(activeWorker->Descriptor());
+
+    RefPtr<ServiceWorkerManager> self = this;
+
+    // If we fail to control the client, then automatically remove it
+    // from our list of controlled clients.
+    p->Then(
+      SystemGroup::EventTargetFor(TaskCategory::Other), __func__,
+      [] (bool) {
+        // do nothing on success
+      }, [self, clientInfo = handle->Info()] (nsresult aRv) {
+        // failed to control, forget about this client
+        self->StopControllingClient(clientInfo);
+      });
   }
 }
 
 already_AddRefed<ServiceWorkerRegistrationInfo>
 ServiceWorkerManager::GetRegistration(nsIPrincipal* aPrincipal,
                                       const nsACString& aScope) const
 {
   MOZ_ASSERT(aPrincipal);