From 1e58b980f59b115d2a34eb067eb6c3e57dcf5fe7 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Tue, 17 Jan 2017 15:43:12 -0500 Subject: [PATCH] Bug 1285898 - [e10s-multi] Localstorage "storage" event is not fired with multiple content processes. r=asuth --HG-- extra : rebase_source : 9968fd96960aecd516bfcc5c239b9de85280ab8a extra : source : 6681b50c1f6d0d2d22d5f631234402e020c0b78a --- dom/base/nsGlobalWindow.cpp | 94 +++++++++++--------- dom/events/StorageEvent.h | 15 ++++ dom/ipc/ContentChild.cpp | 15 ++++ dom/ipc/ContentChild.h | 8 ++ dom/ipc/ContentParent.cpp | 20 +++++ dom/ipc/ContentParent.h | 8 ++ dom/ipc/PContent.ipdl | 13 +++ dom/storage/Storage.cpp | 79 ++++++++++++++-- dom/storage/Storage.h | 15 ++++ dom/storage/StorageCache.cpp | 57 ------------ dom/storage/StorageCache.h | 3 - editor/libeditor/tests/test_bug674770-1.html | 2 +- 12 files changed, 221 insertions(+), 108 deletions(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 4fcbc8525534..6b5a65e6f5a3 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -11563,48 +11563,43 @@ nsGlobalWindow::Observe(nsISupports* aSubject, const char* aTopic, } bool isPrivateBrowsing = IsPrivateBrowsing(); + // In addition to determining if this is a storage event, the + // isPrivateBrowsing checks here 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 ((!nsCRT::strcmp(aTopic, "dom-storage2-changed") && !isPrivateBrowsing) || (!nsCRT::strcmp(aTopic, "dom-private-storage2-changed") && isPrivateBrowsing)) { if (!IsInnerWindow() || !AsInner()->IsCurrentInnerWindow()) { return NS_OK; } - nsIPrincipal *principal; - nsresult rv; + nsIPrincipal *principal = GetPrincipal(); + if (!principal) { + return NS_OK; + } RefPtr event = static_cast(aSubject); if (!event) { return NS_ERROR_FAILURE; } - RefPtr changingStorage = event->GetStorageArea(); - if (!changingStorage) { - return NS_ERROR_FAILURE; - } - - nsCOMPtr istorage = changingStorage.get(); - bool fireMozStorageChanged = false; nsAutoString eventType; eventType.AssignLiteral("storage"); - principal = GetPrincipal(); - if (!principal) { - return NS_OK; - } - if (changingStorage->IsPrivate() != IsPrivateBrowsing()) { - return NS_OK; - } + if (!NS_strcmp(aData, u"sessionStorage")) { + nsCOMPtr changingStorage = event->GetStorageArea(); + MOZ_ASSERT(changingStorage); - switch (changingStorage->GetType()) - { - case Storage::SessionStorage: - { bool check = false; nsCOMPtr storageManager = do_QueryInterface(GetDocShell()); if (storageManager) { - rv = storageManager->CheckStorage(principal, istorage, &check); + nsresult rv = storageManager->CheckStorage(principal, changingStorage, + &check); if (NS_FAILED(rv)) { return rv; } @@ -11625,44 +11620,43 @@ nsGlobalWindow::Observe(nsISupports* aSubject, const char* aTopic, if (fireMozStorageChanged) { eventType.AssignLiteral("MozSessionStorageChanged"); } - break; } - case Storage::LocalStorage: - { - // Allow event fire only for the same principal storages - // XXX We have to use EqualsIgnoreDomain after bug 495337 lands - nsIPrincipal* storagePrincipal = changingStorage->GetPrincipal(); + else { + MOZ_ASSERT(!NS_strcmp(aData, u"localStorage")); + nsIPrincipal* storagePrincipal = event->GetPrincipal(); + if (!storagePrincipal) { + return NS_OK; + } bool equals = false; - rv = storagePrincipal->Equals(principal, &equals); + nsresult rv = storagePrincipal->Equals(principal, &equals); NS_ENSURE_SUCCESS(rv, rv); - if (!equals) + if (!equals) { return NS_OK; + } + + fireMozStorageChanged = mLocalStorage == event->GetStorageArea(); - fireMozStorageChanged = mLocalStorage == changingStorage; if (fireMozStorageChanged) { eventType.AssignLiteral("MozLocalStorageChanged"); } - break; - } - default: - return NS_OK; } // Clone the storage event included in the observer notification. We want // to dispatch clones rather than the original event. ErrorResult error; - RefPtr newEvent = CloneStorageEvent(eventType, event, error); + RefPtr clonedEvent = + CloneStorageEvent(eventType, event, error); if (error.Failed()) { return error.StealNSResult(); } - newEvent->SetTrusted(true); + clonedEvent->SetTrusted(true); if (fireMozStorageChanged) { - WidgetEvent* internalEvent = newEvent->WidgetEventPtr(); + WidgetEvent* internalEvent = clonedEvent->WidgetEventPtr(); internalEvent->mFlags.mOnlyChromeDispatch = true; } @@ -11671,12 +11665,12 @@ nsGlobalWindow::Observe(nsISupports* aSubject, const char* aTopic, // store the domain in which the change happened and fire the // events if we're ever thawed. - mPendingStorageEvents.AppendElement(newEvent); + mPendingStorageEvents.AppendElement(clonedEvent); return NS_OK; } bool defaultActionEnabled; - DispatchEvent(newEvent, &defaultActionEnabled); + DispatchEvent(clonedEvent, &defaultActionEnabled); return NS_OK; } @@ -11767,10 +11761,19 @@ nsGlobalWindow::CloneStorageEvent(const nsAString& aType, aEvent->GetUrl(dict.mUrl); RefPtr storageArea = aEvent->GetStorageArea(); - MOZ_ASSERT(storageArea); RefPtr storage; - if (storageArea->GetType() == Storage::LocalStorage) { + + // If null, this is a localStorage event received by IPC. + if (!storageArea) { + storage = GetLocalStorage(aRv); + if (aRv.Failed() || !storage) { + return nullptr; + } + + // We must apply the current change to the 'local' localStorage. + storage->ApplyEvent(aEvent); + } else if (storageArea->GetType() == Storage::LocalStorage) { storage = GetLocalStorage(aRv); } else { MOZ_ASSERT(storageArea->GetType() == Storage::SessionStorage); @@ -11782,7 +11785,7 @@ nsGlobalWindow::CloneStorageEvent(const nsAString& aType, } MOZ_ASSERT(storage); - MOZ_ASSERT(storage->IsForkOf(storageArea)); + MOZ_ASSERT_IF(storageArea, storage->IsForkOf(storageArea)); dict.mStorageArea = storage; @@ -12943,6 +12946,13 @@ nsGlobalWindow::EventListenerAdded(nsIAtom* aType) aType == nsGkAtoms::onvrdisplaypresentchange) { NotifyVREventListenerAdded(); } + + // We need to initialize localStorage in order to receive notifications. + if (aType == nsGkAtoms::onstorage) { + ErrorResult rv; + GetLocalStorage(rv); + rv.SuppressException(); + } } void diff --git a/dom/events/StorageEvent.h b/dom/events/StorageEvent.h index e82baf34131f..f9639ef58b50 100644 --- a/dom/events/StorageEvent.h +++ b/dom/events/StorageEvent.h @@ -13,6 +13,8 @@ #include "mozilla/dom/Event.h" #include "mozilla/dom/StorageEventBinding.h" +class nsIPrincipal; + namespace mozilla { namespace dom { @@ -34,6 +36,7 @@ protected: nsString mNewValue; nsString mUrl; RefPtr mStorageArea; + nsCOMPtr mPrincipal; public: virtual StorageEvent* AsStorageEvent(); @@ -79,6 +82,18 @@ public: { return mStorageArea; } + + // Non WebIDL methods + void SetPrincipal(nsIPrincipal* aPrincipal) + { + MOZ_ASSERT(!mPrincipal); + mPrincipal = aPrincipal; + } + + nsIPrincipal* GetPrincipal() const + { + return mPrincipal; + } }; } // namespace dom diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 4889745c71a6..365376af67c7 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -35,6 +35,7 @@ #include "mozilla/dom/PCrashReporterChild.h" #include "mozilla/dom/ProcessGlobal.h" #include "mozilla/dom/PushNotifier.h" +#include "mozilla/dom/Storage.h" #include "mozilla/dom/StorageIPC.h" #include "mozilla/dom/TabGroup.h" #include "mozilla/dom/workers/ServiceWorkerManager.h" @@ -3050,6 +3051,20 @@ ContentChild::RecvBlobURLUnregistration(const nsCString& aURI) return IPC_OK(); } +mozilla::ipc::IPCResult +ContentChild::RecvDispatchLocalStorageChange(const nsString& aDocumentURI, + const nsString& aKey, + const nsString& aOldValue, + const nsString& aNewValue, + const IPC::Principal& aPrincipal, + const bool& aIsPrivate) +{ + Storage::DispatchStorageEvent(Storage::LocalStorage, + aDocumentURI, aKey, aOldValue, aNewValue, + aPrincipal, aIsPrivate, nullptr); + return IPC_OK(); +} + #if defined(XP_WIN) && defined(ACCESSIBILITY) bool ContentChild::SendGetA11yContentId() diff --git a/dom/ipc/ContentChild.h b/dom/ipc/ContentChild.h index 2b12f638d948..5ed147dee780 100644 --- a/dom/ipc/ContentChild.h +++ b/dom/ipc/ContentChild.h @@ -408,6 +408,14 @@ public: virtual mozilla::ipc::IPCResult RecvInitBlobURLs(nsTArray&& aRegistations) override; + virtual mozilla::ipc::IPCResult + RecvDispatchLocalStorageChange(const nsString& aDocumentURI, + const nsString& aKey, + const nsString& aOldValue, + const nsString& aNewValue, + const IPC::Principal& aPrincipal, + const bool& aIsPrivate) override; + virtual mozilla::ipc::IPCResult RecvLastPrivateDocShellDestroyed() override; virtual mozilla::ipc::IPCResult RecvFilePathUpdate(const nsString& aStorageType, diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 0eff89258c98..b26110f6685b 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -51,6 +51,7 @@ #include "mozilla/dom/PContentPermissionRequestParent.h" #include "mozilla/dom/PCycleCollectWithLogsParent.h" #include "mozilla/dom/ServiceWorkerRegistrar.h" +#include "mozilla/dom/Storage.h" #include "mozilla/dom/StorageIPC.h" #include "mozilla/dom/devicestorage/DeviceStorageRequestParent.h" #include "mozilla/dom/power/PowerManagerService.h" @@ -4738,6 +4739,25 @@ ContentParent::RecvUnstoreAndBroadcastBlobURLUnregistration(const nsCString& aUR return IPC_OK(); } +mozilla::ipc::IPCResult +ContentParent::RecvBroadcastLocalStorageChange(const nsString& aDocumentURI, + const nsString& aKey, + const nsString& aOldValue, + const nsString& aNewValue, + const Principal& aPrincipal, + const bool& aIsPrivate) +{ + for (auto* cp : ContentParent::AllProcesses(ContentParent::eLive)) { + if (cp != this) { + Unused << cp->SendDispatchLocalStorageChange( + nsString(aDocumentURI), nsString(aKey), nsString(aOldValue), + nsString(aNewValue), IPC::Principal(aPrincipal), aIsPrivate); + } + } + + return IPC_OK(); +} + mozilla::ipc::IPCResult ContentParent::RecvGetA11yContentId(uint32_t* aContentId) { diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index c78f5fd83f4e..7dfb1b950f9a 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -577,6 +577,14 @@ public: virtual mozilla::ipc::IPCResult RecvUnstoreAndBroadcastBlobURLUnregistration(const nsCString& aURI) override; + virtual mozilla::ipc::IPCResult + RecvBroadcastLocalStorageChange(const nsString& aDocumentURI, + const nsString& aKey, + const nsString& aOldValue, + const nsString& aNewValue, + const IPC::Principal& aPrincipal, + const bool& aIsPrivate) override; + virtual mozilla::ipc::IPCResult RecvGetA11yContentId(uint32_t* aContentId) override; diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 474882426f41..78f4bb9982ef 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -643,6 +643,12 @@ child: async BlobURLUnregistration(nsCString aURI); + async DispatchLocalStorageChange(nsString documentURI, + nsString key, + nsString oldValue, + nsString newValue, + Principal principal, + bool isPrivate); async GMPsChanged(GMPCapabilityData[] capabilities); @@ -1151,6 +1157,13 @@ parent: async UnstoreAndBroadcastBlobURLUnregistration(nsCString url); + async BroadcastLocalStorageChange(nsString documentURI, + nsString key, + nsString oldValue, + nsString newValue, + Principal principal, + bool isPrivate); + /** * Messages for communicating child Telemetry to the parent process */ diff --git a/dom/storage/Storage.cpp b/dom/storage/Storage.cpp index 71fd932b7dc6..d839c8bba50a 100644 --- a/dom/storage/Storage.cpp +++ b/dom/storage/Storage.cpp @@ -14,6 +14,9 @@ #include "nsIPrincipal.h" #include "nsICookiePermission.h" +#include "mozilla/dom/ContentChild.h" +#include "mozilla/dom/ContentParent.h" +#include "mozilla/dom/PermissionMessageUtils.h" #include "mozilla/dom/StorageBinding.h" #include "mozilla/dom/StorageEvent.h" #include "mozilla/dom/StorageEventBinding.h" @@ -25,6 +28,9 @@ #include "nsServiceManagerUtils.h" namespace mozilla { + +using namespace ipc; + namespace dom { NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(Storage, mManager, mPrincipal, mWindow) @@ -58,7 +64,6 @@ Storage::Storage(nsPIDOMWindowInner* aWindow, Storage::~Storage() { - mCache->KeepAlive(); } /* virtual */ JSObject* @@ -212,6 +217,29 @@ void Storage::BroadcastChangeNotification(const nsSubstring& aKey, const nsSubstring& aOldValue, const nsSubstring& aNewValue) +{ + if (!XRE_IsParentProcess() && GetType() == LocalStorage && mPrincipal) { + // If we are in a child process, we want to send a message to the parent in + // order to broadcast the StorageEvent correctly to any child process. + dom::ContentChild* cc = dom::ContentChild::GetSingleton(); + Unused << NS_WARN_IF(!cc->SendBroadcastLocalStorageChange( + mDocumentURI, nsString(aKey), nsString(aOldValue), nsString(aNewValue), + IPC::Principal(mPrincipal), mIsPrivate)); + } + + DispatchStorageEvent(GetType(), mDocumentURI, aKey, aOldValue, aNewValue, + mPrincipal, mIsPrivate, this); +} + +/* static */ void +Storage::DispatchStorageEvent(StorageType aStorageType, + const nsAString& aDocumentURI, + const nsAString& aKey, + const nsAString& aOldValue, + const nsAString& aNewValue, + nsIPrincipal* aPrincipal, + bool aIsPrivate, + Storage* aStorage) { StorageEventInit dict; dict.mBubbles = false; @@ -219,21 +247,62 @@ Storage::BroadcastChangeNotification(const nsSubstring& aKey, dict.mKey = aKey; dict.mNewValue = aNewValue; dict.mOldValue = aOldValue; - dict.mStorageArea = this; - dict.mUrl = mDocumentURI; + dict.mStorageArea = aStorage; + dict.mUrl = aDocumentURI; // Note, this DOM event should never reach JS. It is cloned later in // nsGlobalWindow. RefPtr event = StorageEvent::Constructor(nullptr, NS_LITERAL_STRING("storage"), dict); + event->SetPrincipal(aPrincipal); + RefPtr r = new StorageNotifierRunnable(event, - GetType() == LocalStorage + aStorageType == LocalStorage ? u"localStorage" : u"sessionStorage", - IsPrivate()); + aIsPrivate); NS_DispatchToMainThread(r); + + // If we are in the parent process and we have the principal, we want to + // broadcast this event to every other process. + if (aStorageType == LocalStorage && XRE_IsParentProcess() && aPrincipal) { + for (auto* cp : ContentParent::AllProcesses(ContentParent::eLive)) { + Unused << cp->SendDispatchLocalStorageChange( + nsString(aDocumentURI), nsString(aKey), nsString(aOldValue), + nsString(aNewValue), IPC::Principal(aPrincipal), aIsPrivate); + } + } +} + +void +Storage::ApplyEvent(StorageEvent* aStorageEvent) +{ + MOZ_ASSERT(aStorageEvent); + + nsAutoString key; + nsAutoString old; + nsAutoString value; + + aStorageEvent->GetKey(key); + aStorageEvent->GetNewValue(value); + + // No key means clearing the full storage. + if (key.IsVoid()) { + MOZ_ASSERT(value.IsVoid()); + mCache->Clear(this); + return; + } + + // No new value means removing the key. + if (value.IsVoid()) { + mCache->RemoveItem(this, key, old); + return; + } + + // Otherwise, we set the new value. + mCache->SetItem(this, key, value, old); } static const char kPermissionType[] = "cookie"; diff --git a/dom/storage/Storage.h b/dom/storage/Storage.h index fb0c93966341..62361f669ee3 100644 --- a/dom/storage/Storage.h +++ b/dom/storage/Storage.h @@ -24,6 +24,7 @@ namespace dom { class StorageManagerBase; class StorageCache; +class StorageEvent; class Storage final : public nsIDOMStorage @@ -129,6 +130,20 @@ public: return mCache == aOther->mCache; } + // aStorage can be null if this method is called by ContentChild. + static void + DispatchStorageEvent(StorageType aStorageType, + const nsAString& aDocumentURI, + const nsAString& aKey, + const nsAString& aOldValue, + const nsAString& aNewValue, + nsIPrincipal* aPrincipal, + bool aIsPrivate, + Storage* aStorage); + + void + ApplyEvent(StorageEvent* aStorageEvent); + protected: // The method checks whether the caller can use a storage. // CanUseStorage is called before any DOM initiated operation diff --git a/dom/storage/StorageCache.cpp b/dom/storage/StorageCache.cpp index 85da26bb4764..6b1dff2417b5 100644 --- a/dom/storage/StorageCache.cpp +++ b/dom/storage/StorageCache.cpp @@ -246,60 +246,6 @@ StorageCache::Preload() namespace { -// This class is passed to timer as a tick observer. It refers the cache -// and keeps it alive for a time. -class StorageCacheHolder : public nsITimerCallback -{ - virtual ~StorageCacheHolder() {} - - NS_DECL_ISUPPORTS - - NS_IMETHOD - Notify(nsITimer* aTimer) override - { - mCache = nullptr; - return NS_OK; - } - - RefPtr mCache; - -public: - explicit StorageCacheHolder(StorageCache* aCache) : mCache(aCache) {} -}; - -NS_IMPL_ISUPPORTS(StorageCacheHolder, nsITimerCallback) - -} // namespace - -void -StorageCache::KeepAlive() -{ - // Missing reference back to the manager means the cache is not responsible - // for its lifetime. Used for keeping sessionStorage live forever. - if (!mManager) { - return; - } - - if (!NS_IsMainThread()) { - // Timer and the holder must be initialized on the main thread. - NS_DispatchToMainThread(NewRunnableMethod(this, &StorageCache::KeepAlive)); - return; - } - - nsCOMPtr timer = do_CreateInstance("@mozilla.org/timer;1"); - if (!timer) { - return; - } - - RefPtr holder = new StorageCacheHolder(this); - timer->InitWithCallback(holder, DOM_STORAGE_CACHE_KEEP_ALIVE_TIME_MS, - nsITimer::TYPE_ONE_SHOT); - - mKeepAliveTimer.swap(timer); -} - -namespace { - // The AutoTimer provided by telemetry headers is only using static, // i.e. compile time known ID, but here we know the ID only at run time. // Hence a new class. @@ -667,9 +613,6 @@ StorageCache::LoadItem(const nsAString& aKey, const nsString& aValue) void StorageCache::LoadDone(nsresult aRv) { - // Keep the preloaded cache alive for a time - KeepAlive(); - MonitorAutoLock monitor(mMonitor); mLoadResult = aRv; mLoaded = true; diff --git a/dom/storage/StorageCache.h b/dom/storage/StorageCache.h index dad9daf395f8..3b156a9bfafc 100644 --- a/dom/storage/StorageCache.h +++ b/dom/storage/StorageCache.h @@ -196,9 +196,6 @@ private: // Obtained from the manager during initialization (Init method). RefPtr mUsage; - // Timer that holds this cache alive for a while after it has been preloaded. - nsCOMPtr mKeepAliveTimer; - // Principal the cache has been initially created for, this is used only for // sessionStorage access checks since sessionStorage objects are strictly // scoped by a principal. localStorage objects on the other hand are scoped diff --git a/editor/libeditor/tests/test_bug674770-1.html b/editor/libeditor/tests/test_bug674770-1.html index 4ba65f507f0f..f2e65b2b91cd 100644 --- a/editor/libeditor/tests/test_bug674770-1.html +++ b/editor/libeditor/tests/test_bug674770-1.html @@ -24,7 +24,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=674770 SimpleTest.waitForExplicitFinish(); SimpleTest.waitForFocus(function() { - SpecialPowers.pushPrefEnv({"set":[["middlemouse.paste", true], ["dom.ipc.processCount", 1]]}, startTests); + SpecialPowers.pushPrefEnv({"set":[["middlemouse.paste", true]]}, startTests); }); function startTests() {