Bug 1350637 - Part 5: Factor out the parent actor observer sink to work on the PBackground thread, fix the rest of observer handling to use IPC. r=asuth, a=lizzard
authorJan Varga <jan.varga@gmail.com>
Tue, 08 Aug 2017 23:01:33 +0200
changeset 421298 9c97e6d93100
parent 421297 d06e37723df2
child 421299 e77772fae3c2
push id7647
push userryanvm@gmail.com
push dateMon, 21 Aug 2017 19:15:39 +0000
treeherdermozilla-beta@6a2167880016 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth, lizzard
bugs1350637
milestone56.0
Bug 1350637 - Part 5: Factor out the parent actor observer sink to work on the PBackground thread, fix the rest of observer handling to use IPC. r=asuth, a=lizzard
dom/storage/PBackgroundStorage.ipdl
dom/storage/StorageIPC.cpp
dom/storage/StorageIPC.h
dom/storage/StorageObserver.cpp
--- a/dom/storage/PBackgroundStorage.ipdl
+++ b/dom/storage/PBackgroundStorage.ipdl
@@ -1,16 +1,21 @@
 /* -*- Mode: C++; c-basic-offset: 2; indent-tabs-mode: nil; tab-width: 8 -*- */
 /* vim: set sw=4 ts=8 et tw=80 ft=cpp : */
 /* 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 protocol PBackground;
 
+include "mozilla/dom/quota/SerializationHelpers.h";
+
+using mozilla::OriginAttributesPattern
+  from "mozilla/OriginAttributes.h";
+
 namespace mozilla {
 namespace dom {
 
 /* This protocol bridges async access to the database thread running on the
  * parent process and caches running on the child process.
  */
 sync protocol PBackgroundStorage
 {
@@ -30,17 +35,27 @@ parent:
   async AsyncAddItem(nsCString originSuffix, nsCString originNoSuffix,
                      nsString key, nsString value);
   async AsyncUpdateItem(nsCString originSuffix, nsCString originNoSuffix,
                         nsString key, nsString value);
   async AsyncRemoveItem(nsCString originSuffix, nsCString originNoSuffix,
                         nsString key);
   async AsyncClear(nsCString originSuffix, nsCString originNoSuffix);
   async AsyncFlush();
-  
+
+  // These are privileged operations for use only by the observer API for
+  // delayed initialization and clearing origins and will never be used from
+  // content process children.  Ideally these would be guarded by checks or
+  // exist on a separate, privileged interface, but PBackgroundStorage is
+  // already insecure.
+  async Startup();
+  async ClearAll();
+  async ClearMatchingOrigin(nsCString originNoSuffix);
+  async ClearMatchingOriginAttributes(OriginAttributesPattern pattern);
+
 child:
   async Observe(nsCString topic,
                 nsString originAttributesPattern,
                 nsCString originScope);
   async OriginsHavingData(nsCString[] origins);
   async LoadItem(nsCString originSuffix, nsCString originNoSuffix, nsString key,
                  nsString value);
   async LoadDone(nsCString originSuffix, nsCString originNoSuffix, nsresult rv);
--- a/dom/storage/StorageIPC.cpp
+++ b/dom/storage/StorageIPC.cpp
@@ -298,16 +298,18 @@ StorageDBChild::ShouldPreloadOrigin(cons
   return !mOriginsHavingData || mOriginsHavingData->Contains(aOrigin);
 }
 
 mozilla::ipc::IPCResult
 StorageDBChild::RecvObserve(const nsCString& aTopic,
                             const nsString& aOriginAttributesPattern,
                             const nsCString& aOriginScope)
 {
+  MOZ_ASSERT(!XRE_IsParentProcess());
+
   StorageObserver::Self()->Notify(
     aTopic.get(), aOriginAttributesPattern, aOriginScope);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 StorageDBChild::RecvOriginsHavingData(nsTArray<nsCString>&& aOrigins)
 {
@@ -401,16 +403,62 @@ ShutdownObserver::Observe(nsISupports* a
 
   return NS_OK;
 }
 
 // ----------------------------------------------------------------------------
 // Parent
 // ----------------------------------------------------------------------------
 
+class StorageDBParent::ObserverSink
+  : public StorageObserverSink
+{
+  nsCOMPtr<nsIEventTarget> mOwningEventTarget;
+
+  // Only touched on the PBackground thread.
+  StorageDBParent* MOZ_NON_OWNING_REF mActor;
+
+public:
+  explicit ObserverSink(StorageDBParent* aActor)
+    : mOwningEventTarget(GetCurrentThreadEventTarget())
+    , mActor(aActor)
+  {
+    AssertIsOnBackgroundThread();
+    MOZ_ASSERT(aActor);
+  }
+
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(StorageDBParent::ObserverSink);
+
+  void
+  Start();
+
+  void
+  Stop();
+
+private:
+  ~ObserverSink() = default;
+
+  void
+  AddSink();
+
+  void
+  RemoveSink();
+
+  void
+  Notify(const nsCString& aTopic,
+         const nsString& aOriginAttributesPattern,
+         const nsCString& aOriginScope);
+
+  // StorageObserverSink
+  nsresult
+  Observe(const char* aTopic,
+          const nsAString& aOriginAttrPattern,
+          const nsACString& aOriginScope) override;
+};
+
 NS_IMPL_ADDREF(StorageDBParent)
 NS_IMPL_RELEASE(StorageDBParent)
 
 void
 StorageDBParent::AddIPDLReference()
 {
   MOZ_ASSERT(!mIPCOpen, "Attempting to retain multiple IPDL references");
   mIPCOpen = true;
@@ -437,16 +485,18 @@ public:
 
 private:
   NS_IMETHOD Run() override
   {
     if (!mParent->IPCOpen()) {
       return NS_OK;
     }
 
+    mParent->Init();
+
     StorageDBThread* storageThread = StorageDBThread::Get();
     if (storageThread) {
       InfallibleTArray<nsCString> scopes;
       storageThread->GetOriginsHavingData(&scopes);
       mozilla::Unused << mParent->SendOriginsHavingData(scopes);
     }
 
     // XXX Fix me!
@@ -474,36 +524,49 @@ private:
   RefPtr<StorageDBParent> mParent;
 };
 
 } // namespace
 
 StorageDBParent::StorageDBParent()
 : mIPCOpen(false)
 {
-  StorageObserver* observer = StorageObserver::Self();
-  if (observer) {
-    observer->AddSink(this);
-  }
+  AssertIsOnBackgroundThread();
 
   // We are always open by IPC only
   AddIPDLReference();
 
   // Cannot send directly from here since the channel
   // is not completely built at this moment.
   RefPtr<SendInitialChildDataRunnable> r =
     new SendInitialChildDataRunnable(this);
   NS_DispatchToCurrentThread(r);
 }
 
 StorageDBParent::~StorageDBParent()
 {
-  StorageObserver* observer = StorageObserver::Self();
-  if (observer) {
-    observer->RemoveSink(this);
+  AssertIsOnBackgroundThread();
+
+  if (mObserverSink) {
+    mObserverSink->Stop();
+    mObserverSink = nullptr;
+  }
+}
+
+void
+StorageDBParent::Init()
+{
+  AssertIsOnBackgroundThread();
+
+  PBackgroundParent* actor = Manager();
+  MOZ_ASSERT(actor);
+
+  if (BackgroundParent::IsOtherProcessActor(actor)) {
+    mObserverSink = new ObserverSink(this);
+    mObserverSink->Start();
   }
 }
 
 StorageDBParent::CacheParentBridge*
 StorageDBParent::NewCache(const nsACString& aOriginSuffix,
                           const nsACString& aOriginNoSuffix)
 {
   return new CacheParentBridge(this, aOriginSuffix, aOriginNoSuffix);
@@ -739,30 +802,76 @@ StorageDBParent::RecvAsyncFlush()
     return IPC_FAIL_NO_REASON(this);
   }
 
   storageThread->AsyncFlush();
 
   return IPC_OK();
 }
 
-// StorageObserverSink
+mozilla::ipc::IPCResult
+StorageDBParent::RecvStartup()
+{
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  if (!storageThread) {
+    return IPC_FAIL_NO_REASON(this);
+  }
+
+  return IPC_OK();
+}
+
+mozilla::ipc::IPCResult
+StorageDBParent::RecvClearAll()
+{
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  if (!storageThread) {
+    return IPC_FAIL_NO_REASON(this);
+  }
+
+  storageThread->AsyncClearAll();
+
+  return IPC_OK();
+}
 
-nsresult
-StorageDBParent::Observe(const char* aTopic,
-                         const nsAString& aOriginAttributesPattern,
-                         const nsACString& aOriginScope)
+mozilla::ipc::IPCResult
+StorageDBParent::RecvClearMatchingOrigin(const nsCString& aOriginNoSuffix)
+{
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  if (!storageThread) {
+    return IPC_FAIL_NO_REASON(this);
+  }
+
+  storageThread->AsyncClearMatchingOrigin(aOriginNoSuffix);
+
+  return IPC_OK();
+}
+
+mozilla::ipc::IPCResult
+StorageDBParent::RecvClearMatchingOriginAttributes(
+                                        const OriginAttributesPattern& aPattern)
+{
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  if (!storageThread) {
+    return IPC_FAIL_NO_REASON(this);
+  }
+
+  storageThread->AsyncClearMatchingOriginAttributes(aPattern);
+
+  return IPC_OK();
+}
+
+void
+StorageDBParent::Observe(const nsCString& aTopic,
+                         const nsString& aOriginAttributesPattern,
+                         const nsCString& aOriginScope)
 {
   if (mIPCOpen) {
-      mozilla::Unused << SendObserve(nsDependentCString(aTopic),
-                                     nsString(aOriginAttributesPattern),
-                                     nsCString(aOriginScope));
+    mozilla::Unused <<
+      SendObserve(aTopic, aOriginAttributesPattern, aOriginScope);
   }
-
-  return NS_OK;
 }
 
 namespace {
 
 // Results must be sent back on the main thread
 class LoadRunnable : public Runnable
 {
 public:
@@ -995,16 +1104,106 @@ StorageDBParent::UsageParentBridge::Dest
     NewNonOwningRunnableMethod("UsageParentBridge::Destroy",
                                this,
                                &UsageParentBridge::Destroy);
 
   MOZ_ALWAYS_SUCCEEDS(mOwningEventTarget->Dispatch(destroyRunnable,
                                                    NS_DISPATCH_NORMAL));
 }
 
+void
+StorageDBParent::
+ObserverSink::Start()
+{
+  AssertIsOnBackgroundThread();
+
+  RefPtr<Runnable> runnable =
+    NewRunnableMethod("StorageDBParent::ObserverSink::AddSink",
+                      this,
+                      &StorageDBParent::ObserverSink::AddSink);
+
+  MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(runnable));
+}
+
+void
+StorageDBParent::
+ObserverSink::Stop()
+{
+  AssertIsOnBackgroundThread();
+
+  mActor = nullptr;
+
+  RefPtr<Runnable> runnable =
+    NewRunnableMethod("StorageDBParent::ObserverSink::RemoveSink",
+                      this,
+                      &StorageDBParent::ObserverSink::RemoveSink);
+
+  MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(runnable));
+}
+
+void
+StorageDBParent::
+ObserverSink::AddSink()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  StorageObserver* observer = StorageObserver::Self();
+  if (observer) {
+    observer->AddSink(this);
+  }
+}
+
+void
+StorageDBParent::
+ObserverSink::RemoveSink()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  StorageObserver* observer = StorageObserver::Self();
+  if (observer) {
+    observer->RemoveSink(this);
+  }
+}
+
+void
+StorageDBParent::
+ObserverSink::Notify(const nsCString& aTopic,
+                     const nsString& aOriginAttributesPattern,
+                     const nsCString& aOriginScope)
+{
+  AssertIsOnBackgroundThread();
+
+  if (mActor) {
+    mActor->Observe(aTopic, aOriginAttributesPattern, aOriginScope);
+  }
+}
+
+nsresult
+StorageDBParent::
+ObserverSink::Observe(const char* aTopic,
+                      const nsAString& aOriginAttributesPattern,
+                      const nsACString& aOriginScope)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  RefPtr<Runnable> runnable =
+    NewRunnableMethod<nsCString, nsString, nsCString>(
+      "StorageDBParent::ObserverSink::Observe2",
+      this,
+      &StorageDBParent::ObserverSink::Notify,
+      aTopic,
+      aOriginAttributesPattern,
+      aOriginScope);
+
+  MOZ_ALWAYS_SUCCEEDS(
+    mOwningEventTarget->Dispatch(runnable, NS_DISPATCH_NORMAL));
+
+  return NS_OK;
+}
+
 /*******************************************************************************
  * Exported functions
  ******************************************************************************/
 
 PBackgroundStorageParent*
 AllocPBackgroundStorageParent()
 {
   AssertIsOnBackgroundThread();
--- a/dom/storage/StorageIPC.h
+++ b/dom/storage/StorageIPC.h
@@ -132,23 +132,27 @@ private:
 
 
 // Receives async requests from child processes and is responsible
 // to send back responses from the DB thread.  Exposes as a fake
 // LocalStorageCache consumer.
 // Also responsible for forwardning all chrome operation notifications
 // such as cookie cleaning etc to the child process.
 class StorageDBParent final : public PBackgroundStorageParent
-                            , public StorageObserverSink
 {
+  class ObserverSink;
+
   virtual ~StorageDBParent();
 
 public:
   StorageDBParent();
 
+  void
+  Init();
+
   NS_IMETHOD_(MozExternalRefCountType) AddRef(void);
   NS_IMETHOD_(MozExternalRefCountType) Release(void);
 
   void AddIPDLReference();
   void ReleaseIPDLReference();
 
   bool IPCOpen() { return mIPCOpen; }
 
@@ -245,25 +249,33 @@ private:
                                               const nsString& aValue) override;
   mozilla::ipc::IPCResult RecvAsyncRemoveItem(const nsCString& aOriginSuffix,
                                               const nsCString& aOriginNoSuffix,
                                               const nsString& aKey) override;
   mozilla::ipc::IPCResult RecvAsyncClear(const nsCString& aOriginSuffix,
                                          const nsCString& aOriginNoSuffix) override;
   mozilla::ipc::IPCResult RecvAsyncFlush() override;
 
-  // StorageObserverSink
-  virtual nsresult Observe(const char* aTopic,
-                           const nsAString& aOriginAttrPattern,
-                           const nsACString& aOriginScope) override;
+  mozilla::ipc::IPCResult RecvStartup() override;
+  mozilla::ipc::IPCResult RecvClearAll() override;
+  mozilla::ipc::IPCResult RecvClearMatchingOrigin(
+                                     const nsCString& aOriginNoSuffix) override;
+  mozilla::ipc::IPCResult RecvClearMatchingOriginAttributes(
+                              const OriginAttributesPattern& aPattern) override;
+
+  void Observe(const nsCString& aTopic,
+               const nsString& aOriginAttrPattern,
+               const nsCString& aOriginScope);
 
 private:
   CacheParentBridge* NewCache(const nsACString& aOriginSuffix,
                               const nsACString& aOriginNoSuffix);
 
+  RefPtr<ObserverSink> mObserverSink;
+
   ThreadSafeAutoRefCnt mRefCnt;
   NS_DECL_OWNINGTHREAD
 
   // True when IPC channel is open and Send*() methods are OK to use.
   bool mIPCOpen;
 };
 
 PBackgroundStorageParent*
--- a/dom/storage/StorageObserver.cpp
+++ b/dom/storage/StorageObserver.cpp
@@ -155,62 +155,70 @@ NS_IMETHODIMP
 StorageObserver::Observe(nsISupports* aSubject,
                          const char* aTopic,
                          const char16_t* aData)
 {
   nsresult rv;
 
   // Start the thread that opens the database.
   if (!strcmp(aTopic, kStartupTopic)) {
+    MOZ_ASSERT(XRE_IsParentProcess());
+
     nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
     obs->RemoveObserver(this, kStartupTopic);
 
     mDBThreadStartDelayTimer = do_CreateInstance(NS_TIMER_CONTRACTID);
     if (!mDBThreadStartDelayTimer) {
       return NS_ERROR_UNEXPECTED;
     }
 
     mDBThreadStartDelayTimer->Init(this, nsITimer::TYPE_ONE_SHOT, kStartupDelay);
 
     return NS_OK;
   }
 
   // Timer callback used to start the database a short timer after startup
   if (!strcmp(aTopic, NS_TIMER_CALLBACK_TOPIC)) {
+    MOZ_ASSERT(XRE_IsParentProcess());
+
     nsCOMPtr<nsITimer> timer = do_QueryInterface(aSubject);
     if (!timer) {
       return NS_ERROR_UNEXPECTED;
     }
 
     if (timer == mDBThreadStartDelayTimer) {
       mDBThreadStartDelayTimer = nullptr;
 
-    // XXX Fix me!
-#if 0
-      StorageDBBridge* db = LocalStorageCache::StartDatabase();
-      NS_ENSURE_TRUE(db, NS_ERROR_FAILURE);
-#endif
+      StorageDBChild* storageChild = StorageDBChild::GetOrCreate();
+      if (NS_WARN_IF(!storageChild)) {
+        return NS_ERROR_FAILURE;
+      }
+
+      storageChild->SendStartup();
     }
 
     return NS_OK;
   }
 
   // Clear everything, caches + database
   if (!strcmp(aTopic, "cookie-changed")) {
     if (!NS_LITERAL_STRING("cleared").Equals(aData)) {
       return NS_OK;
     }
 
-    // XXX Fix me!
-#if 0
-    StorageDBBridge* db = LocalStorageCache::StartDatabase();
-    NS_ENSURE_TRUE(db, NS_ERROR_FAILURE);
+    StorageDBChild* storageChild = StorageDBChild::GetOrCreate();
+    if (NS_WARN_IF(!storageChild)) {
+      return NS_ERROR_FAILURE;
+    }
 
-    db->AsyncClearAll();
-#endif
+    storageChild->AsyncClearAll();
+
+    if (XRE_IsParentProcess()) {
+      storageChild->SendClearAll();
+    }
 
     Notify("cookie-cleared");
 
     return NS_OK;
   }
 
   // Clear from caches everything that has been stored
   // while in session-only mode
@@ -283,51 +291,53 @@ StorageObserver::Observe(nsISupports* aS
                         fallible);
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     nsAutoCString originScope;
     rv = CreateReversedDomain(aceDomain, originScope);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    // XXX Fix me!
-#if 0
-    StorageDBBridge* db = LocalStorageCache::StartDatabase();
-    NS_ENSURE_TRUE(db, NS_ERROR_FAILURE);
+    if (XRE_IsParentProcess()) {
+      StorageDBChild* storageChild = StorageDBChild::GetOrCreate();
+      if (NS_WARN_IF(!storageChild)) {
+        return NS_ERROR_FAILURE;
+      }
 
-    db->AsyncClearMatchingOrigin(originScope);
-#endif
+      storageChild->SendClearMatchingOrigin(originScope);
+    }
 
     Notify("domain-data-cleared", EmptyString(), originScope);
 
     return NS_OK;
   }
 
   // Clear all private-browsing caches
   if (!strcmp(aTopic, "last-pb-context-exited")) {
     Notify("private-browsing-data-cleared");
 
     return NS_OK;
   }
 
   // Clear data of the origins whose prefixes will match the suffix.
   if (!strcmp(aTopic, "clear-origin-attributes-data")) {
+    MOZ_ASSERT(XRE_IsParentProcess());
+
     OriginAttributesPattern pattern;
     if (!pattern.Init(nsDependentString(aData))) {
       NS_ERROR("Cannot parse origin attributes pattern");
       return NS_ERROR_FAILURE;
     }
 
-    // XXX Fix me!
-#if 0
-    StorageDBBridge* db = LocalStorageCache::StartDatabase();
-    NS_ENSURE_TRUE(db, NS_ERROR_FAILURE);
+    StorageDBChild* storageChild = StorageDBChild::GetOrCreate();
+    if (NS_WARN_IF(!storageChild)) {
+      return NS_ERROR_FAILURE;
+    }
 
-    db->AsyncClearMatchingOriginAttributes(pattern);
-#endif
+    storageChild->SendClearMatchingOriginAttributes(pattern);
 
     Notify("origin-attr-pattern-cleared", nsDependentString(aData));
 
     return NS_OK;
   }
 
   if (!strcmp(aTopic, "profile-after-change")) {
     Notify("profile-change");
@@ -361,23 +371,22 @@ StorageObserver::Observe(nsISupports* aS
       Notify("no-low-disk-space");
     }
 
     return NS_OK;
   }
 
 #ifdef DOM_STORAGE_TESTS
   if (!strcmp(aTopic, "domstorage-test-flush-force")) {
-    // XXX Fix me!
-#if 0
-    StorageDBBridge* db = LocalStorageCache::GetDatabase();
-    if (db) {
-      db->AsyncFlush();
+    StorageDBChild* storageChild = StorageDBChild::GetOrCreate();
+    if (NS_WARN_IF(!storageChild)) {
+      return NS_ERROR_FAILURE;
     }
-#endif
+
+    storageChild->SendAsyncFlush();
 
     return NS_OK;
   }
 
   if (!strcmp(aTopic, "domstorage-test-flushed")) {
     // Only used to propagate to IPC children
     Notify("test-flushed");