From 8e035934607fda3b05c43f1dba7e08f478c3626d Mon Sep 17 00:00:00 2001 From: Jan Varga Date: Thu, 3 Oct 2024 11:26:38 +0000 Subject: [PATCH] Bug 1919555 - LSNG: Convert PBackgroundLSDatabase to a top level actor; r=dom-storage-reviewers,asuth The conversion will allow: - returning an endpoint instead of a datastore id when datastore preparation is done - binding the child actor to a different event target Long term, it would be possible to bind the parent actor to a dedicated task queue or existing LS thread, helping to relieve the PBackground thread. Differential Revision: https://phabricator.services.mozilla.com/D222417 --- dom/localstorage/ActorsChild.cpp | 4 +- dom/localstorage/ActorsChild.h | 10 ++-- dom/localstorage/ActorsParent.cpp | 64 ++++++++++++++++++--- dom/localstorage/ActorsParent.h | 12 ++-- dom/localstorage/LSDatabase.cpp | 4 +- dom/localstorage/LSObject.cpp | 12 +++- dom/localstorage/PBackgroundLSDatabase.ipdl | 7 +-- ipc/glue/BackgroundParentImpl.cpp | 25 +++----- ipc/glue/BackgroundParentImpl.h | 13 ++--- ipc/glue/PBackground.ipdl | 9 +-- 10 files changed, 99 insertions(+), 61 deletions(-) diff --git a/dom/localstorage/ActorsChild.cpp b/dom/localstorage/ActorsChild.cpp index f56d9cb582d6..1b95beea48ef 100644 --- a/dom/localstorage/ActorsChild.cpp +++ b/dom/localstorage/ActorsChild.cpp @@ -38,14 +38,14 @@ LSDatabaseChild::~LSDatabaseChild() { MOZ_COUNT_DTOR(LSDatabaseChild); } -void LSDatabaseChild::SendDelete() { +void LSDatabaseChild::Shutdown() { AssertIsOnOwningThread(); if (mDatabase) { mDatabase->ClearActor(); mDatabase = nullptr; - MOZ_ALWAYS_TRUE(PBackgroundLSDatabaseChild::Send__delete__(this)); + Close(); } } diff --git a/dom/localstorage/ActorsChild.h b/dom/localstorage/ActorsChild.h index e48a9663cd6c..334cb3ba9429 100644 --- a/dom/localstorage/ActorsChild.h +++ b/dom/localstorage/ActorsChild.h @@ -45,16 +45,16 @@ class LSSnapshot; * Most of the interesting bits happen via PBackgroundLSSnapshot. * * Mutual raw pointers are maintained between LSDatabase and this class that are - * cleared at either (expected) when the child starts the deletion process - * (SendDelete) or unexpected actor death (ActorDestroy). + * cleared at either (expected) when the child starts the shutdown process + * (Shutdown) or unexpected actor death (ActorDestroy). * * See `PBackgroundLSDatabase.ipdl` for more information. * * * ## Low-Level Lifecycle ## * - Created by LSObject::EnsureDatabase if it had to create a database. - * - Deletion begun by LSDatabase's destructor invoking SendDelete which sends - * __delete__ which destroys the actor. + * - Shutdown begun by LSDatabase's destructor invoking Shutdown which destroys + * the actor. */ class LSDatabaseChild final : public PBackgroundLSDatabaseChild { friend class mozilla::ipc::BackgroundChildImpl; @@ -76,7 +76,7 @@ class LSDatabaseChild final : public PBackgroundLSDatabaseChild { ~LSDatabaseChild(); - void SendDelete(); + void Shutdown(); // IPDL methods are only called by IPDL. void ActorDestroy(ActorDestroyReason aWhy) override; diff --git a/dom/localstorage/ActorsParent.cpp b/dom/localstorage/ActorsParent.cpp index ddfd367f8259..e3304ab5fab5 100644 --- a/dom/localstorage/ActorsParent.cpp +++ b/dom/localstorage/ActorsParent.cpp @@ -2791,6 +2791,10 @@ using PrivateDatastoreHashtable = // window actually goes away. UniquePtr gPrivateDatastores; +using DatabaseArray = nsTArray; + +StaticAutoPtr gDatabases; + using LiveDatabaseArray = nsTArray>>; StaticAutoPtr gLiveDatabases; @@ -3202,6 +3206,8 @@ void InitializeLocalStorage() { #endif } +namespace { + already_AddRefed AllocPBackgroundLSDatabaseParent( const PrincipalInfo& aPrincipalInfo, const uint32_t& aPrivateBrowsingId, const uint64_t& aDatastoreId) { @@ -3225,14 +3231,14 @@ already_AddRefed AllocPBackgroundLSDatabaseParent( // If we ever decide to return null from this point on, we need to make sure // that the datastore is closed and the prepared datastore is removed from the // gPreparedDatastores hashtable. - // We also assume that IPDL must call RecvPBackgroundLSDatabaseConstructor - // once we return a valid actor in this method. + // We also assume that RecvCreateBackgroundLSDatabaseParent must call + // RecvPBackgroundLSDatabaseConstructor once we return a valid actor in this + // method. RefPtr database = new Database(aPrincipalInfo, preparedDatastore->GetContentParentId(), preparedDatastore->Origin(), aPrivateBrowsingId); - // Transfer ownership to IPDL. return database.forget(); } @@ -3246,8 +3252,8 @@ bool RecvPBackgroundLSDatabaseConstructor(PBackgroundLSDatabaseParent* aActor, MOZ_ASSERT(gPreparedDatastores->Get(aDatastoreId)); MOZ_ASSERT(!QuotaClient::IsShuttingDownOnBackgroundThread()); - // The actor is now completely built (it has a manager, channel and it's - // registered as a subprotocol). + // The actor is now completely built (it has a channel and it's registered + // as a top level protocol). // ActorDestroy will be called if we fail here. mozilla::UniquePtr preparedDatastore; @@ -3268,6 +3274,25 @@ bool RecvPBackgroundLSDatabaseConstructor(PBackgroundLSDatabaseParent* aActor, return true; } +} // namespace + +bool RecvCreateBackgroundLSDatabaseParent( + const PrincipalInfo& aPrincipalInfo, const uint32_t& aPrivateBrowsingId, + const uint64_t& aDatastoreId, + Endpoint&& aParentEndpoint) { + RefPtr parent = AllocPBackgroundLSDatabaseParent( + aPrincipalInfo, aPrivateBrowsingId, aDatastoreId); + if (!parent) { + return false; + } + + // Transfer ownership to IPDL. + MOZ_ALWAYS_TRUE(aParentEndpoint.Bind(parent)); + + return RecvPBackgroundLSDatabaseConstructor(parent, aPrincipalInfo, + aPrivateBrowsingId, aDatastoreId); +} + PBackgroundLSObserverParent* AllocPBackgroundLSObserverParent( const uint64_t& aObserverId) { AssertIsOnBackgroundThread(); @@ -5305,12 +5330,25 @@ Database::Database(const PrincipalInfo& aPrincipalInfo, #endif { AssertIsOnOwningThread(); + + if (!gDatabases) { + gDatabases = new DatabaseArray(); + } + + gDatabases->AppendElement(this); } Database::~Database() { AssertIsOnOwningThread(); MOZ_ASSERT_IF(mActorWasAlive, mAllowedToClose); MOZ_ASSERT_IF(mActorWasAlive, mActorDestroyed); + + MOZ_ASSERT(gDatabases); + gDatabases->RemoveElement(this); + + if (gDatabases->IsEmpty()) { + gDatabases = nullptr; + } } void Database::SetActorAlive(Datastore* aDatastore) { @@ -5387,7 +5425,7 @@ void Database::ForceKill() { return; } - Unused << PBackgroundLSDatabaseParent::Send__delete__(this); + Close(); } void Database::Stringify(nsACString& aResult) const { @@ -5398,7 +5436,7 @@ void Database::Stringify(nsACString& aResult) const { aResult.Append(kQuotaGenericDelimiter); aResult.AppendLiteral("OtherProcessActor:"); - aResult.AppendInt(BackgroundParent::IsOtherProcessActor(Manager())); + aResult.AppendInt(mContentParentId.isSome()); aResult.Append(kQuotaGenericDelimiter); aResult.AppendLiteral("Origin:"); @@ -8859,6 +8897,18 @@ void QuotaClient::FinalizeShutdown() { gConnectionThread = nullptr; } + + if (gDatabases) { + nsTArray> databases; + + for (const auto& database : *gDatabases) { + databases.AppendElement(database); + } + + for (const auto& database : databases) { + database->Close(); + } + } } Result, nsresult> diff --git a/dom/localstorage/ActorsParent.h b/dom/localstorage/ActorsParent.h index b317ee9d116a..05691ca6d19d 100644 --- a/dom/localstorage/ActorsParent.h +++ b/dom/localstorage/ActorsParent.h @@ -14,6 +14,8 @@ namespace mozilla { namespace ipc { +template +class Endpoint; class PBackgroundParent; class PrincipalInfo; @@ -36,14 +38,10 @@ class Client; void InitializeLocalStorage(); -already_AddRefed AllocPBackgroundLSDatabaseParent( +bool RecvCreateBackgroundLSDatabaseParent( const mozilla::ipc::PrincipalInfo& aPrincipalInfo, - const uint32_t& aPrivateBrowsingId, const uint64_t& aDatastoreId); - -bool RecvPBackgroundLSDatabaseConstructor( - PBackgroundLSDatabaseParent* aActor, - const mozilla::ipc::PrincipalInfo& aPrincipalInfo, - const uint32_t& aPrivateBrowsingId, const uint64_t& aDatastoreId); + const uint32_t& aPrivateBrowsingId, const uint64_t& aDatastoreId, + mozilla::ipc::Endpoint&& aParentEndpoint); PBackgroundLSObserverParent* AllocPBackgroundLSObserverParent( const uint64_t& aObserverId); diff --git a/dom/localstorage/LSDatabase.cpp b/dom/localstorage/LSDatabase.cpp index 304c31377600..2ed0ab6f414c 100644 --- a/dom/localstorage/LSDatabase.cpp +++ b/dom/localstorage/LSDatabase.cpp @@ -98,8 +98,8 @@ LSDatabase::~LSDatabase() { } if (mActor) { - mActor->SendDelete(); - MOZ_ASSERT(!mActor, "SendDelete should have cleared!"); + mActor->Shutdown(); + MOZ_ASSERT(!mActor, "Shutdown should have cleared!"); } } diff --git a/dom/localstorage/LSObject.cpp b/dom/localstorage/LSObject.cpp index 457953525c10..99a61dd32c9d 100644 --- a/dom/localstorage/LSObject.cpp +++ b/dom/localstorage/LSObject.cpp @@ -938,8 +938,16 @@ nsresult LSObject::EnsureDatabase() { RefPtr actor = new LSDatabaseChild(database); - MOZ_ALWAYS_TRUE(backgroundActor->SendPBackgroundLSDatabaseConstructor( - actor, *mStoragePrincipalInfo, mPrivateBrowsingId, datastoreId)); + mozilla::ipc::Endpoint parentEndpoint; + mozilla::ipc::Endpoint childEndpoint; + MOZ_ALWAYS_SUCCEEDS( + PBackgroundLSDatabase::CreateEndpoints(&parentEndpoint, &childEndpoint)); + + MOZ_ALWAYS_TRUE(childEndpoint.Bind(actor)); + + MOZ_ALWAYS_TRUE(backgroundActor->SendCreateBackgroundLSDatabaseParent( + *mStoragePrincipalInfo, mPrivateBrowsingId, datastoreId, + std::move(parentEndpoint))); database->SetActor(actor); diff --git a/dom/localstorage/PBackgroundLSDatabase.ipdl b/dom/localstorage/PBackgroundLSDatabase.ipdl index 62c482fe0f0f..35a68a0ee08e 100644 --- a/dom/localstorage/PBackgroundLSDatabase.ipdl +++ b/dom/localstorage/PBackgroundLSDatabase.ipdl @@ -2,7 +2,6 @@ * 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 protocol PBackgroundLSSnapshot; include PBackgroundLSSharedTypes; @@ -92,10 +91,9 @@ struct LSSnapshotInitInfo * ContentParent::AboutToLoadHttpDocumentForChild, the central place * for pre-loading.) */ -[ChildImpl=virtual, ParentImpl=virtual] +[ChildImpl=virtual, ParentImpl=virtual, ChildProc=anydom] sync protocol PBackgroundLSDatabase { - manager PBackground; manages PBackgroundLSSnapshot; parent: @@ -153,9 +151,6 @@ child: * ContentParent::ShutDownProcess. */ async RequestAllowToClose(); - -both: - async __delete__(); }; } // namespace dom diff --git a/ipc/glue/BackgroundParentImpl.cpp b/ipc/glue/BackgroundParentImpl.cpp index c37875a095ea..f435d4ac401c 100644 --- a/ipc/glue/BackgroundParentImpl.cpp +++ b/ipc/glue/BackgroundParentImpl.cpp @@ -274,29 +274,20 @@ BackgroundParentImpl::RecvPBackgroundSDBConnectionConstructor( return IPC_OK(); } -already_AddRefed -BackgroundParentImpl::AllocPBackgroundLSDatabaseParent( - const PrincipalInfo& aPrincipalInfo, const uint32_t& aPrivateBrowsingId, - const uint64_t& aDatastoreId) { - AssertIsInMainProcess(); - AssertIsOnBackgroundThread(); - - return mozilla::dom::AllocPBackgroundLSDatabaseParent( - aPrincipalInfo, aPrivateBrowsingId, aDatastoreId); -} - mozilla::ipc::IPCResult -BackgroundParentImpl::RecvPBackgroundLSDatabaseConstructor( - PBackgroundLSDatabaseParent* aActor, const PrincipalInfo& aPrincipalInfo, - const uint32_t& aPrivateBrowsingId, const uint64_t& aDatastoreId) { +BackgroundParentImpl::RecvCreateBackgroundLSDatabaseParent( + const PrincipalInfo& aPrincipalInfo, const uint32_t& aPrivateBrowsingId, + const uint64_t& aDatastoreId, + Endpoint&& aParentEndpoint) { AssertIsInMainProcess(); AssertIsOnBackgroundThread(); - MOZ_ASSERT(aActor); - if (!mozilla::dom::RecvPBackgroundLSDatabaseConstructor( - aActor, aPrincipalInfo, aPrivateBrowsingId, aDatastoreId)) { + if (!mozilla::dom::RecvCreateBackgroundLSDatabaseParent( + aPrincipalInfo, aPrivateBrowsingId, aDatastoreId, + std::move(aParentEndpoint))) { return IPC_FAIL_NO_REASON(this); } + return IPC_OK(); } diff --git a/ipc/glue/BackgroundParentImpl.h b/ipc/glue/BackgroundParentImpl.h index 20cf4e9125e2..a95c156eb47e 100644 --- a/ipc/glue/BackgroundParentImpl.h +++ b/ipc/glue/BackgroundParentImpl.h @@ -56,15 +56,10 @@ class BackgroundParentImpl : public PBackgroundParent { const PersistenceType& aPersistenceType, const PrincipalInfo& aPrincipalInfo) override; - already_AddRefed - AllocPBackgroundLSDatabaseParent(const PrincipalInfo& aPrincipalInfo, - const uint32_t& aPrivateBrowsingId, - const uint64_t& aDatastoreId) override; - - mozilla::ipc::IPCResult RecvPBackgroundLSDatabaseConstructor( - PBackgroundLSDatabaseParent* aActor, const PrincipalInfo& aPrincipalInfo, - const uint32_t& aPrivateBrowsingId, - const uint64_t& aDatastoreId) override; + mozilla::ipc::IPCResult RecvCreateBackgroundLSDatabaseParent( + const PrincipalInfo& aPrincipalInfo, const uint32_t& aPrivateBrowsingId, + const uint64_t& aDatastoreId, + Endpoint&& aParentEndpoint) override; PBackgroundLSObserverParent* AllocPBackgroundLSObserverParent( const uint64_t& aObserverId) override; diff --git a/ipc/glue/PBackground.ipdl b/ipc/glue/PBackground.ipdl index 3b21914f64b8..7cd71eff9d11 100644 --- a/ipc/glue/PBackground.ipdl +++ b/ipc/glue/PBackground.ipdl @@ -86,7 +86,6 @@ sync protocol PBackground manages PBackgroundIDBFactory; manages PBackgroundIndexedDBUtils; manages PBackgroundSDBConnection; - manages PBackgroundLSDatabase; manages PBackgroundLSObserver; manages PBackgroundLSRequest; manages PBackgroundLSSimpleRequest; @@ -139,9 +138,11 @@ parent: async PBackgroundSDBConnection(PersistenceType persistenceType, PrincipalInfo principalInfo); - async PBackgroundLSDatabase(PrincipalInfo principalInfo, - uint32_t privateBrowsingId, - uint64_t datastoreId); + async CreateBackgroundLSDatabaseParent( + PrincipalInfo principalInfo, + uint32_t privateBrowsingId, + uint64_t datastoreId, + Endpoint aParentEndpoint); async PBackgroundLSObserver(uint64_t observerId);