Bug 1556812 - Only notify same-origin actors in LocalStorageCacheParent::RecvNotify(); r=baku
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 07 Jun 2019 15:09:21 +0000
changeset 477866 9b4358c2ea5fb38b7d8208be96efcdd1fc7e77dd
parent 477865 bab948f512c2f5ffe5a6638676143c7063bf2ad3
child 477867 0fc603b37e654a5fb4205c8494c95d3102e604d9
push id36125
push userapavel@mozilla.com
push dateFri, 07 Jun 2019 22:00:07 +0000
treeherdermozilla-central@d820bbb356aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1556812
milestone69.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 1556812 - Only notify same-origin actors in LocalStorageCacheParent::RecvNotify(); r=baku Differential Revision: https://phabricator.services.mozilla.com/D33867
dom/storage/PBackgroundLocalStorageCache.ipdl
dom/storage/StorageIPC.cpp
dom/storage/StorageIPC.h
--- a/dom/storage/PBackgroundLocalStorageCache.ipdl
+++ b/dom/storage/PBackgroundLocalStorageCache.ipdl
@@ -23,16 +23,17 @@ parent:
 
 child:
   // The principalInfo and privateBrowsingId could instead be retained by the
   // LocalStorageCacheChild/LocalStorageCache instead of being re-transmitted.
   // However, these changes are a temporary optimization intended for uplift,
   // and this constant factor overhead is very small compared to the upside of
   // filtering.
   async Observe(PrincipalInfo principalInfo,
+                PrincipalInfo cachePrincipalInfo,
                 uint32_t privateBrowsingId,
                 nsString documentURI,
                 nsString key,
                 nsString oldValue,
                 nsString newValue);
 
   async __delete__();
 };
--- a/dom/storage/StorageIPC.cpp
+++ b/dom/storage/StorageIPC.cpp
@@ -64,33 +64,43 @@ void LocalStorageCacheChild::ActorDestro
 
   if (mCache) {
     mCache->ClearActor();
     mCache = nullptr;
   }
 }
 
 mozilla::ipc::IPCResult LocalStorageCacheChild::RecvObserve(
-    const PrincipalInfo& aPrincipalInfo, const uint32_t& aPrivateBrowsingId,
-    const nsString& aDocumentURI, const nsString& aKey,
-    const nsString& aOldValue, const nsString& aNewValue) {
+    const PrincipalInfo& aPrincipalInfo,
+    const PrincipalInfo& aCachePrincipalInfo,
+    const uint32_t& aPrivateBrowsingId, const nsString& aDocumentURI,
+    const nsString& aKey, const nsString& aOldValue,
+    const nsString& aNewValue) {
   AssertIsOnOwningThread();
 
   nsresult rv;
   nsCOMPtr<nsIPrincipal> principal =
       PrincipalInfoToPrincipal(aPrincipalInfo, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return IPC_FAIL_NO_REASON(this);
   }
 
-  Storage::NotifyChange(/* aStorage */ nullptr, principal, aKey, aOldValue,
-                        aNewValue,
-                        /* aStorageType */ u"localStorage", aDocumentURI,
-                        /* aIsPrivate */ !!aPrivateBrowsingId,
-                        /* aImmediateDispatch */ true);
+  nsCOMPtr<nsIPrincipal> cachePrincipal =
+      PrincipalInfoToPrincipal(aCachePrincipalInfo, &rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return IPC_FAIL_NO_REASON(this);
+  }
+
+  if (StorageUtils::PrincipalsEqual(principal, cachePrincipal)) {
+    Storage::NotifyChange(/* aStorage */ nullptr, principal, aKey, aOldValue,
+                          aNewValue,
+                          /* aStorageType */ u"localStorage", aDocumentURI,
+                          /* aIsPrivate */ !!aPrivateBrowsingId,
+                          /* aImmediateDispatch */ true);
+  }
 
   return IPC_OK();
 }
 
 // ----------------------------------------------------------------------------
 // Child
 // ----------------------------------------------------------------------------
 
@@ -448,18 +458,18 @@ mozilla::ipc::IPCResult SessionStorageOb
   AssertIsOnOwningThread();
 
   StorageObserver::Self()->Notify(aTopic.get(), aOriginAttributesPattern,
                                   aOriginScope);
   return IPC_OK();
 }
 
 LocalStorageCacheParent::LocalStorageCacheParent(
-    const PrincipalInfo& aPrincipalInfo, const nsACString& aOriginKey,
-    uint32_t aPrivateBrowsingId)
+    const mozilla::ipc::PrincipalInfo& aPrincipalInfo,
+    const nsACString& aOriginKey, uint32_t aPrivateBrowsingId)
     : mPrincipalInfo(aPrincipalInfo),
       mOriginKey(aOriginKey),
       mPrivateBrowsingId(aPrivateBrowsingId),
       mActorDestroyed(false) {
   AssertIsOnBackgroundThread();
 }
 
 LocalStorageCacheParent::~LocalStorageCacheParent() {
@@ -507,19 +517,24 @@ mozilla::ipc::IPCResult LocalStorageCach
   MOZ_ASSERT(gLocalStorageCacheParents);
 
   nsTArray<LocalStorageCacheParent*>* array;
   gLocalStorageCacheParents->Get(mOriginKey, &array);
   MOZ_ASSERT(array);
 
   for (LocalStorageCacheParent* localStorageCacheParent : *array) {
     if (localStorageCacheParent != this) {
+      // When bug 1443925 is fixed, we can compare mPrincipalInfo against
+      // localStorageCacheParent->PrincipalInfo() here on the background thread
+      // instead of posting it to the main thread.  The advantage of doing so is
+      // that it would save an IPC message in the case where the principals do
+      // not match.
       Unused << localStorageCacheParent->SendObserve(
-          mPrincipalInfo, mPrivateBrowsingId, aDocumentURI, aKey, aOldValue,
-          aNewValue);
+          mPrincipalInfo, localStorageCacheParent->PrincipalInfo(),
+          mPrivateBrowsingId, aKey, aDocumentURI, aOldValue, aNewValue);
     }
   }
 
   return IPC_OK();
 }
 
 // ----------------------------------------------------------------------------
 // Parent
--- a/dom/storage/StorageIPC.h
+++ b/dom/storage/StorageIPC.h
@@ -65,16 +65,17 @@ class LocalStorageCacheChild final : pub
 
   // Only called by LocalStorageCache.
   void SendDeleteMeInternal();
 
   // IPDL methods are only called by IPDL.
   void ActorDestroy(ActorDestroyReason aWhy) override;
 
   mozilla::ipc::IPCResult RecvObserve(const PrincipalInfo& aPrincipalInfo,
+                                      const PrincipalInfo& aCachePrincipalInfo,
                                       const uint32_t& aPrivateBrowsingId,
                                       const nsString& aDocumentURI,
                                       const nsString& aKey,
                                       const nsString& aOldValue,
                                       const nsString& aNewValue) override;
 };
 
 // Child side of the IPC protocol, exposes as DB interface but
@@ -210,22 +211,24 @@ class LocalStorageCacheParent final
     : public PBackgroundLocalStorageCacheParent {
   const PrincipalInfo mPrincipalInfo;
   const nsCString mOriginKey;
   uint32_t mPrivateBrowsingId;
   bool mActorDestroyed;
 
  public:
   // Created in AllocPBackgroundLocalStorageCacheParent.
-  LocalStorageCacheParent(const PrincipalInfo& aPrincipalInfo,
+  LocalStorageCacheParent(const mozilla::ipc::PrincipalInfo& aPrincipalInfo,
                           const nsACString& aOriginKey,
                           uint32_t aPrivateBrowsingId);
 
   NS_INLINE_DECL_REFCOUNTING(mozilla::dom::LocalStorageCacheParent)
 
+  const PrincipalInfo& PrincipalInfo() const { return mPrincipalInfo; }
+
  private:
   // Reference counted.
   ~LocalStorageCacheParent();
 
   // IPDL methods are only called by IPDL.
   void ActorDestroy(ActorDestroyReason aWhy) override;
 
   mozilla::ipc::IPCResult RecvDeleteMe() override;