From f126919604048b2c0f05bdaf51ea060c8b5f6076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Qu=C3=A8ze?= Date: Tue, 14 Mar 2023 14:35:45 +0000 Subject: [PATCH] Bug 1819135 - Cleanup expired origins in StorageActivityService on the idle-daily notification and when accessing the data instead of with a 5 minutes repeating timer, r=asuth. Differential Revision: https://phabricator.services.mozilla.com/D172036 --- dom/storage/StorageActivityService.cpp | 71 +++++++++----------------- dom/storage/StorageActivityService.h | 11 +--- 2 files changed, 26 insertions(+), 56 deletions(-) diff --git a/dom/storage/StorageActivityService.cpp b/dom/storage/StorageActivityService.cpp index d8af9c60c524..8652d8265325 100644 --- a/dom/storage/StorageActivityService.cpp +++ b/dom/storage/StorageActivityService.cpp @@ -19,7 +19,7 @@ #include "nsIMutableArray.h" #include "nsIObserverService.h" #include "nsIPrincipal.h" -#include "nsITimer.h" +#include "nsIUserIdleService.h" #include "nsSupportsPrimitives.h" #include "nsXPCOM.h" @@ -136,7 +136,6 @@ StorageActivityService::StorageActivityService() { StorageActivityService::~StorageActivityService() { MOZ_ASSERT(NS_IsMainThread()); - MOZ_ASSERT(!mTimer); } void StorageActivityService::SendActivityInternal(nsIPrincipal* aPrincipal) { @@ -162,8 +161,17 @@ void StorageActivityService::SendActivityInternal(nsIPrincipal* aPrincipal) { void StorageActivityService::SendActivityInternal(const nsACString& aOrigin) { MOZ_ASSERT(XRE_IsParentProcess()); + bool shouldAddObserver = mActivities.Count() == 0; mActivities.InsertOrUpdate(aOrigin, PR_Now()); - MaybeStartTimer(); + + if (shouldAddObserver) { + nsCOMPtr obs = mozilla::services::GetObserverService(); + if (NS_WARN_IF(!obs)) { + return; + } + + obs->AddObserver(this, OBSERVER_TOPIC_IDLE_DAILY, true); + } } void StorageActivityService::SendActivityToParent(nsIPrincipal* aPrincipal) { @@ -190,47 +198,22 @@ NS_IMETHODIMP StorageActivityService::Observe(nsISupports* aSubject, const char* aTopic, const char16_t* aData) { MOZ_ASSERT(NS_IsMainThread()); - MOZ_ASSERT(!strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID)); - MaybeStopTimer(); - - nsCOMPtr obs = mozilla::services::GetObserverService(); - if (obs) { - obs->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID); + if (!strcmp(aTopic, OBSERVER_TOPIC_IDLE_DAILY)) { + CleanUp(); + return NS_OK; } + MOZ_ASSERT(!strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID)); + gStorageActivityShutdown = true; gStorageActivityService = nullptr; return NS_OK; } -void StorageActivityService::MaybeStartTimer() { +void StorageActivityService::CleanUp() { MOZ_ASSERT(NS_IsMainThread()); - if (!mTimer) { - nsresult rv = NS_NewTimerWithCallback( - getter_AddRefs(mTimer), this, 1000 * 5 * 60 /* any 5 minutes */, - nsITimer::TYPE_REPEATING_SLACK, nullptr); - if (NS_FAILED(rv)) { - NS_WARNING("Could not start StorageActivityService timer"); - } - } -} - -void StorageActivityService::MaybeStopTimer() { - MOZ_ASSERT(NS_IsMainThread()); - - if (mTimer) { - mTimer->Cancel(); - mTimer = nullptr; - } -} - -NS_IMETHODIMP -StorageActivityService::Notify(nsITimer* aTimer) { - MOZ_ASSERT(NS_IsMainThread()); - MOZ_ASSERT(mTimer == aTimer); - uint64_t now = PR_Now(); for (auto iter = mActivities.Iter(); !iter.Done(); iter.Next()) { @@ -239,18 +222,13 @@ StorageActivityService::Notify(nsITimer* aTimer) { } } - // If no activities, let's stop the timer. + // If no activities, remove the observer. if (mActivities.Count() == 0) { - MaybeStopTimer(); + nsCOMPtr obs = mozilla::services::GetObserverService(); + if (obs) { + obs->RemoveObserver(this, OBSERVER_TOPIC_IDLE_DAILY); + } } - - return NS_OK; -} - -NS_IMETHODIMP -StorageActivityService::GetName(nsACString& aName) { - aName.AssignLiteral("StorageActivityService"); - return NS_OK; } NS_IMETHODIMP @@ -261,6 +239,9 @@ StorageActivityService::GetActiveOrigins(PRTime aFrom, PRTime aTo, return NS_ERROR_INVALID_ARG; } + // Remove expired entries first. + CleanUp(); + nsresult rv = NS_OK; nsCOMPtr devices = do_CreateInstance(NS_ARRAY_CONTRACTID, &rv); @@ -312,8 +293,6 @@ NS_INTERFACE_MAP_BEGIN(StorageActivityService) NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStorageActivityService) NS_INTERFACE_MAP_ENTRY(nsIStorageActivityService) NS_INTERFACE_MAP_ENTRY(nsIObserver) - NS_INTERFACE_MAP_ENTRY(nsITimerCallback) - NS_INTERFACE_MAP_ENTRY(nsINamed) NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference) NS_INTERFACE_MAP_END diff --git a/dom/storage/StorageActivityService.h b/dom/storage/StorageActivityService.h index 6202e0eb1dc9..0b61bd128ab4 100644 --- a/dom/storage/StorageActivityService.h +++ b/dom/storage/StorageActivityService.h @@ -10,7 +10,6 @@ #include "nsTHashMap.h" #include "nsIObserver.h" #include "nsIStorageActivityService.h" -#include "nsITimer.h" #include "nsWeakReference.h" namespace mozilla { @@ -23,15 +22,11 @@ namespace dom { class StorageActivityService final : public nsIStorageActivityService, public nsIObserver, - public nsITimerCallback, - public nsINamed, public nsSupportsWeakReference { public: NS_DECL_ISUPPORTS NS_DECL_NSISTORAGEACTIVITYSERVICE NS_DECL_NSIOBSERVER - NS_DECL_NSITIMERCALLBACK - NS_DECL_NSINAMED // Main-thread only. static void SendActivity(nsIPrincipal* aPrincipal); @@ -55,14 +50,10 @@ class StorageActivityService final : public nsIStorageActivityService, void SendActivityToParent(nsIPrincipal* aPrincipal); - void MaybeStartTimer(); - - void MaybeStopTimer(); + void CleanUp(); // Activities grouped by origin (+OriginAttributes). nsTHashMap mActivities; - - nsCOMPtr mTimer; }; } // namespace dom