Bug 1379568 - Filtering for StorageEvent broadcasting, r=asuth
☠☠ backed out by da31f44d2f8a ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 12 Jul 2017 14:49:05 +0200
changeset 368622 960a0dc9092fdba8ef5a367a42ddb3f9a3ec3398
parent 368621 e4c0cb48a3f18462e581cbf99315426f214bb705
child 368623 50bb4bb1902f84a7335754f62e7a124ca9b5c732
push id32167
push usercbook@mozilla.com
push dateThu, 13 Jul 2017 14:35:41 +0000
treeherdermozilla-central@d4e6f45db733 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1379568
milestone56.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 1379568 - Filtering for StorageEvent broadcasting, r=asuth
dom/base/nsGlobalWindow.cpp
dom/storage/StorageNotifierService.cpp
dom/storage/StorageNotifierService.h
dom/storage/moz.build
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -19,16 +19,17 @@
 #include "nsIDOMStorageManager.h"
 #include "mozilla/dom/LocalStorage.h"
 #include "mozilla/dom/Storage.h"
 #include "mozilla/dom/IdleRequest.h"
 #include "mozilla/dom/Performance.h"
 #include "mozilla/dom/StorageEvent.h"
 #include "mozilla/dom/StorageEventBinding.h"
 #include "mozilla/dom/StorageNotifierService.h"
+#include "mozilla/dom/StorageUtils.h"
 #include "mozilla/dom/Timeout.h"
 #include "mozilla/dom/TimeoutHandler.h"
 #include "mozilla/dom/TimeoutManager.h"
 #include "mozilla/IntegerPrintfMacros.h"
 #if defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_GONK)
 #include "mozilla/dom/WindowOrientationObserver.h"
 #endif
 #include "nsDOMOfflineResourceList.h"
@@ -493,17 +494,29 @@ public:
                              bool aPrivateBrowsing) override
   {
     if (mWindow) {
       mWindow->ObserveStorageNotification(aEvent, aStorageType,
                                           aPrivateBrowsing);
     }
   }
 
-  virtual nsIEventTarget*
+  nsIPrincipal*
+  GetPrincipal() const override
+  {
+    return mWindow ? mWindow->GetPrincipal() : nullptr;
+  }
+
+  bool
+  IsPrivateBrowsing() const override
+  {
+    return mWindow ? mWindow->IsPrivateBrowsing() : false;
+  }
+
+  nsIEventTarget*
   GetEventTarget() const override
   {
     return mWindow ? mWindow->EventTargetFor(TaskCategory::Other) : nullptr;
   }
 
 private:
   ~nsGlobalWindowObserver() = default;
 
@@ -12364,26 +12377,17 @@ nsGlobalWindow::Observe(nsISupports* aSu
 
 void
 nsGlobalWindow::ObserveStorageNotification(StorageEvent* aEvent,
                                            const char16_t* aStorageType,
                                            bool aPrivateBrowsing)
 {
   MOZ_ASSERT(aEvent);
 
-  // Enforce that the source storage area's private browsing state matches
-  // this window's state.  These flag checks and their maintenance independent
-  // from the principal's OriginAttributes matter because chrome docshells
-  // that are part of private browsing windows can be private browsing without
-  // having their OriginAttributes set (because they have the system
-  // principal).
-  bool isPrivateBrowsing = IsPrivateBrowsing();
-  if (isPrivateBrowsing != aPrivateBrowsing) {
-    return;
-  }
+  MOZ_DIAGNOSTIC_ASSERT(IsPrivateBrowsing() == aPrivateBrowsing);
 
   // LocalStorage can only exist on an inner window, and we don't want to
   // generate events on frozen or otherwise-navigated-away from windows.
   // (Actually, this code used to try and buffer events for frozen windows,
   // but it never worked, so we've removed it.  See bug 1285898.)
   if (!IsInnerWindow() || !AsInner()->IsCurrentInnerWindow() || IsFrozen()) {
     return;
   }
@@ -12425,28 +12429,19 @@ nsGlobalWindow::ObserveStorageNotificati
     fireMozStorageChanged = mSessionStorage == changingStorage;
     if (fireMozStorageChanged) {
       eventType.AssignLiteral("MozSessionStorageChanged");
     }
   }
 
   else {
     MOZ_ASSERT(!NS_strcmp(aStorageType, u"localStorage"));
-    nsIPrincipal* storagePrincipal = aEvent->GetPrincipal();
-    if (!storagePrincipal) {
-      return;
-    }
-
-    bool equals = false;
-    nsresult rv = storagePrincipal->Equals(principal, &equals);
-    NS_ENSURE_SUCCESS_VOID(rv);
-
-    if (!equals) {
-      return;
-    }
+
+    MOZ_DIAGNOSTIC_ASSERT(StorageUtils::PrincipalsEqual(aEvent->GetPrincipal(),
+                                                        principal));
 
     fireMozStorageChanged = mLocalStorage == aEvent->GetStorageArea();
 
     if (fireMozStorageChanged) {
       eventType.AssignLiteral("MozLocalStorageChanged");
     }
   }
 
--- a/dom/storage/StorageNotifierService.cpp
+++ b/dom/storage/StorageNotifierService.cpp
@@ -62,20 +62,38 @@ StorageNotifierService::Broadcast(Storag
   RefPtr<StorageEvent> event = aEvent;
 
   nsTObserverArray<RefPtr<StorageNotificationObserver>>::ForwardIterator
     iter(service->mObservers);
 
   while (iter.HasMore()) {
     RefPtr<StorageNotificationObserver> observer = iter.GetNext();
 
+    // Enforce that the source storage area's private browsing state matches
+    // this window's state.  These flag checks and their maintenance independent
+    // from the principal's OriginAttributes matter because chrome docshells
+    // that are part of private browsing windows can be private browsing without
+    // having their OriginAttributes set (because they have the system
+    // principal).
+    if (aPrivateBrowsing != observer->IsPrivateBrowsing()) {
+      continue;
+    }
+
+    // No reasons to continue if the principal of the event doesn't match with
+    // the window's one.
+    if (!StorageUtils::PrincipalsEqual(aEvent->GetPrincipal(),
+                                       observer->GetPrincipal())) {
+      continue;
+    }
+
     RefPtr<Runnable> r = NS_NewRunnableFunction(
       "StorageNotifierService::Broadcast",
       [observer, event, aStorageType, aPrivateBrowsing] () {
-        observer->ObserveStorageNotification(event, aStorageType, aPrivateBrowsing);
+        observer->ObserveStorageNotification(event, aStorageType,
+                                             aPrivateBrowsing);
       });
 
     if (aImmediateDispatch) {
       r->Run();
     } else {
       nsCOMPtr<nsIEventTarget> et = observer->GetEventTarget();
       if (et) {
         et->Dispatch(r.forget());
--- a/dom/storage/StorageNotifierService.h
+++ b/dom/storage/StorageNotifierService.h
@@ -19,16 +19,22 @@ class StorageNotificationObserver
 public:
   NS_INLINE_DECL_PURE_VIRTUAL_REFCOUNTING
 
   virtual void
   ObserveStorageNotification(StorageEvent* aEvent,
                              const char16_t* aStorageType,
                              bool aPrivateBrowsing) = 0;
 
+  virtual bool
+  IsPrivateBrowsing() const = 0;
+
+  virtual nsIPrincipal*
+  GetPrincipal() const = 0;
+
   virtual nsIEventTarget*
   GetEventTarget() const = 0;
 };
 
 class StorageNotifierService final
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(StorageNotifierService)
--- a/dom/storage/moz.build
+++ b/dom/storage/moz.build
@@ -9,16 +9,17 @@ with Files("**"):
 
 EXPORTS.mozilla.dom += [
     'LocalStorage.h',
     'LocalStorageManager.h',
     'SessionStorageManager.h',
     'Storage.h',
     'StorageIPC.h',
     'StorageNotifierService.h',
+    'StorageUtils.h',
 ]
 
 UNIFIED_SOURCES += [
     'LocalStorage.cpp',
     'LocalStorageCache.cpp',
     'LocalStorageManager.cpp',
     'SessionStorage.cpp',
     'SessionStorageCache.cpp',