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
authorJan Varga <jan.varga@gmail.com>
Tue, 08 Aug 2017 23:01:33 +0200
changeset 373544 a781f14e975bb4e58c38dc5e86a216647f3a61d7
parent 373543 e3e1b4922005e00d6067c2b6920d3bc70a801c14
child 373545 371e75eb2b60281508ef8f7535bdda3964975921
push id32304
push usercbook@mozilla.com
push dateWed, 09 Aug 2017 09:37:21 +0000
treeherdermozilla-central@4c5fbf493763 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1350637
milestone57.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 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
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
@@ -156,62 +156,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
@@ -262,23 +270,26 @@ StorageObserver::Observe(nsISupports* aS
 
     Notify("session-only-cleared", NS_ConvertUTF8toUTF16(originSuffix),
            originScope);
 
     return NS_OK;
   }
 
   if (!strcmp(aTopic, "extension:purge-localStorage")) {
-    // 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("extension:purge-localStorage-caches");
 
     return NS_OK;
   }
 
   // Clear everything (including so and pb data) from caches and database
   // for the gived domain and subdomains.
@@ -298,51 +309,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");
@@ -376,23 +389,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");