Introduce the in-process Storage Service

This wires up the Storage Service implementation in-process as a private
detail of content::StoragePartitionImpl. All DOM Storage implementation
is moved behind Storage Service public Mojo interfaces, and DOM Storage
support in the browser no longer depends directly on any private details
of the Storage Service.

No functional changes are intended here, as the in-process service
instance still runs on the IO thread, where DOM Storage
implementation has also run for some time.

Bug: 1000959
Change-Id: I6fddd8605e3ba426576d9d9348964c5083dd4b4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2052609Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#741640}

components/services/storage/partition_impl.cc

@@ -7,10 +7,21 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/task/post_task.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "build/build_config.h"
+#include "components/services/storage/dom_storage/local_storage_impl.h"
+#include "components/services/storage/dom_storage/session_storage_impl.h"
 #include "components/services/storage/storage_service_impl.h"
 
 namespace storage {
 
+namespace {
+
+const char kSessionStorageDirectory[] = "Session Storage";
+
+}  // namespace
+
 PartitionImpl::PartitionImpl(StorageServiceImpl* service,
                              const base::Optional<base::FilePath>& path)
     : service_(service), path_(path) {
@@ -41,6 +52,38 @@ void PartitionImpl::BindOriginContext(
   iter->second->BindReceiver(std::move(receiver));
 }
 
+void PartitionImpl::BindSessionStorageControl(
+    mojo::PendingReceiver<mojom::SessionStorageControl> receiver) {
+  // This object deletes itself on disconnection.
+  session_storage_ = new SessionStorageImpl(
+      path_.value_or(base::FilePath()),
+      base::CreateSequencedTaskRunner(
+          {base::ThreadPool(), base::MayBlock(),
+           base::TaskShutdownBehavior::BLOCK_SHUTDOWN}),
+      base::SequencedTaskRunnerHandle::Get(),
+#if defined(OS_ANDROID)
+      // On Android there is no support for session storage restoring, and since
+      // the restoring code is responsible for database cleanup, we must
+      // manually delete the old database here before we open a new one.
+      SessionStorageImpl::BackingMode::kClearDiskStateOnOpen,
+#else
+      path_.has_value() ? SessionStorageImpl::BackingMode::kRestoreDiskState
+                        : SessionStorageImpl::BackingMode::kNoDisk,
+#endif
+      std::string(kSessionStorageDirectory), std::move(receiver));
+}
+
+void PartitionImpl::BindLocalStorageControl(
+    mojo::PendingReceiver<mojom::LocalStorageControl> receiver) {
+  // This object deletes itself on disconnection.
+  local_storage_ = new LocalStorageImpl(
+      path_.value_or(base::FilePath()), base::SequencedTaskRunnerHandle::Get(),
+      base::CreateSequencedTaskRunner(
+          {base::ThreadPool(), base::MayBlock(),
+           base::TaskShutdownBehavior::BLOCK_SHUTDOWN}),
+      std::move(receiver));
+}
+
 void PartitionImpl::OnDisconnect() {
   if (receivers_.empty()) {
     // Deletes |this|.

components/services/storage/partition_impl.h

@@ -18,6 +18,8 @@
 
 namespace storage {
 
+class LocalStorageImpl;
+class SessionStorageImpl;
 class StorageServiceImpl;
 
 // A PartitionImpl instance exclusively owns an isolated storage partition
@@ -45,6 +47,10 @@ class PartitionImpl : public mojom::Partition {
   void BindOriginContext(
       const url::Origin& origin,
       mojo::PendingReceiver<mojom::OriginContext> receiver) override;
+  void BindSessionStorageControl(
+      mojo::PendingReceiver<mojom::SessionStorageControl> receiver) override;
+  void BindLocalStorageControl(
+      mojo::PendingReceiver<mojom::LocalStorageControl> receiver) override;
 
  private:
   friend class OriginContextImpl;
@@ -57,6 +63,10 @@ class PartitionImpl : public mojom::Partition {
   mojo::ReceiverSet<mojom::Partition> receivers_;
   std::map<url::Origin, std::unique_ptr<OriginContextImpl>> origin_contexts_;
 
+  // These objects own themselves.
+  SessionStorageImpl* session_storage_;
+  LocalStorageImpl* local_storage_;
+
   DISALLOW_COPY_AND_ASSIGN(PartitionImpl);
 };
 

components/services/storage/public/mojom/partition.mojom

@@ -4,7 +4,9 @@
 
 module storage.mojom;
 
+import "components/services/storage/public/mojom/local_storage_control.mojom";
 import "components/services/storage/public/mojom/origin_context.mojom";
+import "components/services/storage/public/mojom/session_storage_control.mojom";
 import "url/mojom/origin.mojom";
 
 // Partition controls an isolated storage partition owned by the Storage
@@ -13,4 +15,10 @@ interface Partition {
   // Binds a new OriginContext scoped to |origin| within this Partition.
   BindOriginContext(url.mojom.Origin origin,
                     pending_receiver<OriginContext> receiver);
+
+  // Binds the main control interface for Session Storage in this partition.
+  BindSessionStorageControl(pending_receiver<SessionStorageControl> receiver);
+
+  // Binds the main control interface for Local Storage in this partition.
+  BindLocalStorageControl(pending_receiver<LocalStorageControl> receiver);
 };

components/services/storage/public/mojom/storage_service.mojom

@@ -10,6 +10,10 @@ import "mojo/public/mojom/base/file_path.mojom";
 // The main interface into the Storage Service. The browser maintains a single
 // global connection to this interface.
 interface StorageService {
+  // Enables more aggressive flushing of DOM Storage data to disk in order to
+  // minimize chance of data loss.
+  EnableAggressiveDomStorageFlushing();
+
   // Binds a new Partition endpoint.
   //
   // |path| if non-null must be an absolute path, and it identifies a persistent

components/services/storage/storage_service_impl.cc

@@ -4,6 +4,7 @@
 
 #include "components/services/storage/storage_service_impl.h"
 
+#include "components/services/storage/dom_storage/storage_area_impl.h"
 #include "components/services/storage/partition_impl.h"
 
 namespace storage {
@@ -14,6 +15,10 @@ StorageServiceImpl::StorageServiceImpl(
 
 StorageServiceImpl::~StorageServiceImpl() = default;
 
+void StorageServiceImpl::EnableAggressiveDomStorageFlushing() {
+  StorageAreaImpl::EnableAggressiveCommitDelay();
+}
+
 void StorageServiceImpl::BindPartition(
     const base::Optional<base::FilePath>& path,
     mojo::PendingReceiver<mojom::Partition> receiver) {

components/services/storage/storage_service_impl.h

@@ -32,6 +32,7 @@ class StorageServiceImpl : public mojom::StorageService {
   const auto& partitions() const { return partitions_; }
 
   // mojom::StorageService implementation:
+  void EnableAggressiveDomStorageFlushing() override;
   void BindPartition(const base::Optional<base::FilePath>& path,
                      mojo::PendingReceiver<mojom::Partition> receiver) override;
 

content/browser/browser_main_loop.cc

@@ -808,13 +808,6 @@ void BrowserMainLoop::PostMainMessageLoopStart() {
       base::trace_event::CPUFreqMonitor::GetInstance());
 #endif
 
-  if (parsed_command_line_.HasSwitch(
-          switches::kEnableAggressiveDOMStorageFlushing)) {
-    TRACE_EVENT0("startup",
-                 "BrowserMainLoop::Subsystem:EnableAggressiveCommitDelay");
-    storage::StorageAreaImpl::EnableAggressiveCommitDelay();
-  }
-
   // Enable memory-infra dump providers.
   InitSkiaEventTracer();
   base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(

content/browser/dom_storage/dom_storage_context_wrapper.cc

@@ -22,6 +22,7 @@
 #include "components/services/storage/dom_storage/local_storage_impl.h"
 #include "components/services/storage/dom_storage/session_storage_impl.h"
 #include "components/services/storage/public/cpp/constants.h"
+#include "components/services/storage/public/mojom/partition.mojom.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -38,8 +39,6 @@
 namespace content {
 namespace {
 
-const char kSessionStorageDirectory[] = "Session Storage";
-
 void AdaptSessionStorageUsageInfo(
     DOMStorageContext::GetSessionStorageUsageCallback callback,
     std::vector<storage::mojom::SessionStorageUsageInfoPtr> usage) {
@@ -117,64 +116,14 @@ class DOMStorageContextWrapper::StoragePolicyObserver
 };
 
 scoped_refptr<DOMStorageContextWrapper> DOMStorageContextWrapper::Create(
-    const base::FilePath& profile_path,
-    const base::FilePath& local_partition_path,
+    storage::mojom::Partition* partition,
     storage::SpecialStoragePolicy* special_storage_policy) {
-  base::FilePath data_path;
-  const bool is_profile_persistent = !profile_path.empty();
-  if (is_profile_persistent)
-    data_path = profile_path.Append(local_partition_path);
-
-  auto mojo_task_runner =
-      base::CreateSingleThreadTaskRunner({BrowserThread::IO});
-
-  // TODO(https://crbug.com/1000959): These should be bound in an instance of
-  // the Storage Service. For now we bind them alone on the IO thread because
-  // that's where the implementation has effectively lived for some time.
   mojo::Remote<storage::mojom::SessionStorageControl> session_storage_control;
+  partition->BindSessionStorageControl(
+      session_storage_control.BindNewPipeAndPassReceiver());
   mojo::Remote<storage::mojom::LocalStorageControl> local_storage_control;
-  mojo_task_runner->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          [](const base::FilePath& storage_root, bool is_profile_persistent,
-             mojo::PendingReceiver<storage::mojom::SessionStorageControl>
-                 session_storage_receiver,
-             mojo::PendingReceiver<storage::mojom::LocalStorageControl>
-                 local_storage_receiver) {
-            // Deletes itself on shutdown completion.
-            new storage::SessionStorageImpl(
-                storage_root,
-                base::CreateSequencedTaskRunner(
-                    {base::ThreadPool(), base::MayBlock(),
-                     base::TaskShutdownBehavior::BLOCK_SHUTDOWN}),
-                base::CreateSingleThreadTaskRunner({BrowserThread::IO}),
-#if defined(OS_ANDROID)
-                // On Android there is no support for session storage restoring,
-                // and since the restoring code is responsible for database
-                // cleanup, we must manually delete the old database here before
-                // we open it.
-                storage::SessionStorageImpl::BackingMode::kClearDiskStateOnOpen,
-#else
-                is_profile_persistent
-                    ? storage::SessionStorageImpl::BackingMode ::
-                          kRestoreDiskState
-                    : storage::SessionStorageImpl::BackingMode::kNoDisk,
-#endif
-                std::string(kSessionStorageDirectory),
-                std::move(session_storage_receiver));
-            new storage::LocalStorageImpl(
-                storage_root,
-                base::CreateSingleThreadTaskRunner({BrowserThread::IO}),
-                base::CreateSequencedTaskRunner(
-                    {base::ThreadPool(), base::MayBlock(),
-                     base::TaskPriority::USER_BLOCKING,
-                     base::TaskShutdownBehavior::BLOCK_SHUTDOWN}),
-                std::move(local_storage_receiver));
-          },
-          data_path, is_profile_persistent,
-          session_storage_control.BindNewPipeAndPassReceiver(),
-          local_storage_control.BindNewPipeAndPassReceiver()));
-
+  partition->BindLocalStorageControl(
+      local_storage_control.BindNewPipeAndPassReceiver());
   auto wrapper = base::WrapRefCounted(new DOMStorageContextWrapper(
       std::move(session_storage_control), std::move(local_storage_control),
       special_storage_policy));

content/browser/dom_storage/dom_storage_context_wrapper.h

@@ -27,12 +27,11 @@
 #include "third_party/blink/public/mojom/dom_storage/session_storage_namespace.mojom.h"
 #include "third_party/blink/public/mojom/dom_storage/storage_area.mojom.h"
 
-namespace base {
-class FilePath;
-}
-
 namespace storage {
 class SpecialStoragePolicy;
+namespace mojom {
+class Partition;
+}
 }
 
 namespace content {
@@ -60,10 +59,8 @@ class CONTENT_EXPORT DOMStorageContextWrapper
     PURGE_AGGRESSIVE,
   };
 
-  // If |profile_path| is empty, nothing will be saved to disk.
   static scoped_refptr<DOMStorageContextWrapper> Create(
-      const base::FilePath& profile_path,
-      const base::FilePath& local_partition_path,
+      storage::mojom::Partition* partition,
       storage::SpecialStoragePolicy* special_storage_policy);
 
   DOMStorageContextWrapper(

content/browser/storage_partition_impl.cc

@@ -25,10 +25,13 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/syslog_logging.h"
 #include "base/task/post_task.h"
+#include "base/threading/sequence_local_storage_slot.h"
 #include "base/time/default_clock.h"
 #include "build/build_config.h"
 #include "components/leveldb_proto/public/proto_database_provider.h"
 #include "components/services/storage/public/mojom/indexed_db_control.mojom.h"
+#include "components/services/storage/public/mojom/storage_service.mojom.h"
+#include "components/services/storage/storage_service_impl.h"
 #include "content/browser/background_fetch/background_fetch_context.h"
 #include "content/browser/blob_storage/blob_registry_wrapper.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
@@ -71,6 +74,8 @@
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "mojo/public/cpp/bindings/callback_helpers.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "net/base/net_errors.h"
 #include "net/cookies/canonical_cookie.h"
@@ -85,7 +90,6 @@
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/mojom/cookie_manager.mojom.h"
 #include "services/network/public/mojom/network_context.mojom.h"
-#include "services/service_manager/public/cpp/connector.h"
 #include "storage/browser/blob/blob_registry_impl.h"
 #include "storage/browser/blob/blob_storage_context.h"
 #include "storage/browser/database/database_tracker.h"
@@ -112,6 +116,41 @@ namespace {
 
 const storage::QuotaSettings* g_test_quota_settings;
 
+mojo::Remote<storage::mojom::StorageService>& GetStorageServiceRemote() {
+  // NOTE: This use of sequence-local storage is only to ensure that the Remote
+  // only lives as long as the UI-thread sequence, since the UI-thread sequence
+  // may be torn down and reinitialized e.g. between unit tests.
+  static base::NoDestructor<base::SequenceLocalStorageSlot<
+      mojo::Remote<storage::mojom::StorageService>>>
+      remote_slot;
+  return remote_slot->GetOrCreateValue();
+}
+
+void RunInProcessStorageService(
+    mojo::PendingReceiver<storage::mojom::StorageService> receiver) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  static base::NoDestructor<base::SequenceLocalStorageSlot<
+      std::unique_ptr<storage::StorageServiceImpl>>>
+      service_storage_slot;
+  service_storage_slot->GetOrCreateValue() =
+      std::make_unique<storage::StorageServiceImpl>(std::move(receiver));
+}
+
+storage::mojom::StorageService* GetStorageService() {
+  mojo::Remote<storage::mojom::StorageService>& remote =
+      GetStorageServiceRemote();
+  if (!remote) {
+    base::PostTask(FROM_HERE, {BrowserThread::IO},
+                   base::BindOnce(&RunInProcessStorageService,
+                                  remote.BindNewPipeAndPassReceiver()));
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kEnableAggressiveDOMStorageFlushing)) {
+      remote->EnableAggressiveDomStorageFlushing();
+    }
+  }
+  return remote.get();
+}
+
 // A callback to create a URLLoaderFactory that is used in tests.
 StoragePartitionImpl::CreateNetworkFactoryCallback&
 GetCreateURLLoaderFactoryCallback() {
@@ -1324,8 +1363,8 @@ void StoragePartitionImpl::Initialize() {
       browser_context_->GetSpecialStoragePolicy(), quota_manager_proxy.get());
 
   dom_storage_context_ = DOMStorageContextWrapper::Create(
-      is_in_memory_ ? base::FilePath() : browser_context_->GetPath(),
-      relative_partition_path_, browser_context_->GetSpecialStoragePolicy());
+      GetStorageServicePartition(),
+      browser_context_->GetSpecialStoragePolicy());
 
   idle_manager_ = std::make_unique<IdleManager>();
   lock_manager_ = std::make_unique<LockManager>();
@@ -2381,6 +2420,19 @@ BrowserContext* StoragePartitionImpl::browser_context() const {
   return browser_context_;
 }
 
+storage::mojom::Partition* StoragePartitionImpl::GetStorageServicePartition() {
+  if (!remote_partition_) {
+    base::Optional<base::FilePath> storage_path;
+    if (!is_in_memory_) {
+      storage_path =
+          browser_context_->GetPath().Append(relative_partition_path_);
+    }
+    GetStorageService()->BindPartition(
+        storage_path, remote_partition_.BindNewPipeAndPassReceiver());
+  }
+  return remote_partition_.get();
+}
+
 mojo::ReceiverId StoragePartitionImpl::Bind(
     int process_id,
     mojo::PendingReceiver<blink::mojom::StoragePartitionService> receiver) {

content/browser/storage_partition_impl.h

@@ -18,6 +18,7 @@
 #include "base/process/process_handle.h"
 #include "build/build_config.h"
 #include "components/services/storage/public/mojom/indexed_db_control.mojom.h"
+#include "components/services/storage/public/mojom/partition.mojom.h"
 #include "content/browser/appcache/chrome_appcache_service.h"
 #include "content/browser/background_sync/background_sync_context_impl.h"
 #include "content/browser/bluetooth/bluetooth_allowed_devices_map.h"
@@ -282,6 +283,10 @@ class CONTENT_EXPORT StoragePartitionImpl
   // Can return nullptr while |this| is being destroyed.
   BrowserContext* browser_context() const;
 
+  // Returns the interface used to control the corresponding remote Partition in
+  // the Storage Service.
+  storage::mojom::Partition* GetStorageServicePartition();
+
   // Called by each renderer process for each StoragePartitionService interface
   // it binds in the renderer process. Returns the id of the created receiver.
   mojo::ReceiverId Bind(
@@ -435,6 +440,7 @@ class CONTENT_EXPORT StoragePartitionImpl
   // querying its path abd BrowserContext is allowed.
   bool initialized_ = false;
 
+  mojo::Remote<storage::mojom::Partition> remote_partition_;
   scoped_refptr<URLLoaderFactoryGetter> url_loader_factory_getter_;
   scoped_refptr<storage::QuotaManager> quota_manager_;
   scoped_refptr<ChromeAppCacheService> appcache_service_;