Hide LocalStorageContextMojo

This insulates LocalStorageContextMojo behind a new
LocalStorageControl mojom interface defined by the Storage Service.

DOMStorageContextWrapper no longer has direct access to the
LocalStorageContextMojo instances, instead interacting with them only
through the mojom interface.

This simplifies DOMStorageContextWrapper, but also helps prepare
LocalStorageContextMojo for migration into private Storage Service
code.

Bug: 1000959
Change-Id: I12c6ece0b1dd01c3e17e66e6249b2d01935675f6
Tbr: jam@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902244Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#717745}

components/BUILD.gn

@@ -138,7 +138,6 @@ test("components_unittests") {
     "//components/security_interstitials/core:unit_tests",
     "//components/security_state/core:unit_tests",
     "//components/services/heap_profiling/public/cpp:unit_tests",
-    "//components/services/storage:tests",
     "//components/services/unzip:unit_tests",
     "//components/sessions:unit_tests",
     "//components/signin/core/browser:unit_tests",
@@ -265,6 +264,7 @@ test("components_unittests") {
       "//components/services/heap_profiling:unit_tests",
       "//components/services/paint_preview_compositor:unit_tests",
       "//components/services/quarantine:unit_tests",
+      "//components/services/storage:tests",
       "//components/spellcheck/browser:unit_tests",
       "//components/spellcheck/renderer:unit_tests",
       "//components/subresource_filter/content/browser:unit_tests",

components/services/storage/public/mojom/BUILD.gn

@@ -6,6 +6,7 @@ import("//mojo/public/tools/bindings/mojom.gni")
 
 mojom("mojom") {
   sources = [
+    "local_storage_control.mojom",
     "origin_context.mojom",
     "partition.mojom",
     "storage_service.mojom",
@@ -13,6 +14,7 @@ mojom("mojom") {
 
   public_deps = [
     "//mojo/public/mojom/base",
+    "//third_party/blink/public/mojom/dom_storage",
     "//url/mojom:url_mojom_origin",
   ]
 }

components/services/storage/public/mojom/local_storage_control.mojom

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module storage.mojom;
+
+import "mojo/public/mojom/base/time.mojom";
+import "third_party/blink/public/mojom/dom_storage/storage_area.mojom";
+import "url/mojom/origin.mojom";
+
+// Conveys basic information about a single origin's data in Local Storage.
+struct LocalStorageUsageInfo {
+  // The origin whose metadata is represented by this structure.
+  url.mojom.Origin origin;
+
+  // The total size in bytes occupied by this origin within Local Storage.
+  uint64 size_in_bytes;
+
+  // The last time any Local Storage data was modified for this origin.
+  mojo_base.mojom.Time last_modified_time;
+};
+
+// Controls the state of Local Storage within a partition. This is a privileged
+// interface and must not be brokered to untrusted clients.
+//
+// Currently this is consumed and implemented in the browser process, but the
+// implementation will eventually live in the storage service which may run
+// out-of-process.
+interface LocalStorageControl {
+  // Binds a blink.mojom.StorageArea receiver for a specific origin.
+  BindStorageArea(url.mojom.Origin origin,
+                  pending_receiver<blink.mojom.StorageArea> receiver);
+
+  // Retrieves some basic usage information about the Local Storage state.
+  GetUsage() => (array<LocalStorageUsageInfo> info);
+
+  // Deletes all Local State state for the given origin. Responds when the
+  // deletion is complete.
+  DeleteStorage(url.mojom.Origin origin) => ();
+
+  // Ensures that no traces of deleted Local Storage data are left in the
+  // backing storage for this origin. Responds when the cleanup is complete.
+  CleanUpStorage() => ();
+
+  // Tells the service to immediately commit any pending operations to disk.
+  Flush() => ();
+
+  // Purges any in-memory caches to free up as much memory as possible. The next
+  // access to the StorageArea will reload data from the backing database.
+  PurgeMemory();
+
+  // Some origins may be configured to be purged at the end of a session rather
+  // than persisted as in the default Local Storage behavior. If this is called,
+  // that behavior is overridden and Local Storage data is persisted for all
+  // origins.
+  ForceKeepSessionState();
+};

content/browser/DEPS

@@ -143,4 +143,7 @@ specific_include_rules = {
   "cross_site_document_blocking_browsertest.cc": [
     "+services/network/cross_origin_read_blocking.h",
   ],
+  "storage_partition_impl_unittest.cc": [
+    "+third_party/leveldatabase",
+  ],
 }

content/browser/dom_storage/dom_storage_browsertest.cc

@@ -90,29 +90,9 @@ class DOMStorageBrowserTest : public ContentBrowserTest {
     return context_wrapper()->mojo_task_runner();
   }
 
-  LocalStorageContextMojo* context() { return context_wrapper()->mojo_state_; }
-
   SessionStorageContextMojo* session_storage_context() {
     return context_wrapper()->mojo_session_state_;
   }
-
-  void EnsureConnected() {
-    base::RunLoop run_loop;
-    mojo_task_runner()->PostTask(
-        FROM_HERE,
-        base::BindOnce(&LocalStorageContextMojo::RunWhenConnected,
-                       base::Unretained(context()), run_loop.QuitClosure()));
-    run_loop.Run();
-  }
-
-  void EnsureSessionStorageConnected() {
-    base::RunLoop run_loop;
-    mojo_task_runner()->PostTask(
-        FROM_HERE, base::BindOnce(&SessionStorageContextMojo::RunWhenConnected,
-                                  base::Unretained(session_storage_context()),
-                                  run_loop.QuitClosure()));
-    run_loop.Run();
-  }
 };
 
 static const bool kIncognito = true;
@@ -133,7 +113,6 @@ IN_PROC_BROWSER_TEST_F(DOMStorageBrowserTest, SanityCheckIncognito) {
 #define MAYBE_DataPersists DataPersists
 #endif
 IN_PROC_BROWSER_TEST_F(DOMStorageBrowserTest, PRE_DataPersists) {
-  EnsureConnected();
   SimpleTest(GetTestUrl("dom_storage", "store_data.html"), kNotIncognito);
 }
 

content/browser/dom_storage/dom_storage_context_wrapper.h

@@ -15,11 +15,12 @@
 #include "base/memory/weak_ptr.h"
 #include "base/synchronization/lock.h"
 #include "base/thread_annotations.h"
+#include "components/services/storage/public/mojom/local_storage_control.mojom.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/dom_storage_context.h"
 #include "mojo/public/cpp/bindings/message.h"
-#include "mojo/public/cpp/bindings/pending_associated_remote.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #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"
 
@@ -33,7 +34,6 @@ class SpecialStoragePolicy;
 
 namespace content {
 
-class LocalStorageContextMojo;
 class SessionStorageContextMojo;
 class SessionStorageNamespaceImpl;
 
@@ -66,8 +66,10 @@ class CONTENT_EXPORT DOMStorageContextWrapper
 
   DOMStorageContextWrapper(
       scoped_refptr<base::SequencedTaskRunner> mojo_task_runner,
-      LocalStorageContextMojo* mojo_local_storage_context,
-      SessionStorageContextMojo* mojo_session_storage_context);
+      SessionStorageContextMojo* mojo_session_storage_context,
+      mojo::Remote<storage::mojom::LocalStorageControl> local_storage_control);
+
+  storage::mojom::LocalStorageControl* GetLocalStorageControl();
 
   // DOMStorageContext implementation.
   void GetLocalStorageUsage(GetLocalStorageUsageCallback callback) override;
@@ -93,7 +95,6 @@ class CONTENT_EXPORT DOMStorageContextWrapper
 
   void Flush();
 
-  // See mojom::StoragePartitionService interface.
   void OpenLocalStorage(
       const url::Origin& origin,
       mojo::PendingReceiver<blink::mojom::StorageArea> receiver);
@@ -103,11 +104,6 @@ class CONTENT_EXPORT DOMStorageContextWrapper
       mojo::ReportBadMessageCallback bad_message_callback,
       mojo::PendingReceiver<blink::mojom::SessionStorageNamespace> receiver);
 
-  using LocalStorageDatabaseOpenCallback =
-      base::OnceCallback<void(LocalStorageContextMojo*)>;
-  void SetLocalStorageDatabaseOpenCallbackForTesting(
-      LocalStorageDatabaseOpenCallback callback);
-
   SessionStorageContextMojo* mojo_session_state() {
     return mojo_session_state_;
   }
@@ -141,9 +137,8 @@ class CONTENT_EXPORT DOMStorageContextWrapper
   void PurgeMemory(PurgeOption purge_option);
 
   // Keep all mojo-ish details together and not bleed them through the public
-  // interface. The |mojo_state_| object is owned by this object, but destroyed
-  // asynchronously on the |mojo_task_runner_|.
-  LocalStorageContextMojo* mojo_state_ = nullptr;
+  // interface. The |mojo_session_state_| object is owned by this object, but
+  // destroyed asynchronously on the |mojo_task_runner_|.
   SessionStorageContextMojo* mojo_session_state_ = nullptr;
   scoped_refptr<base::SequencedTaskRunner> mojo_task_runner_;
 
@@ -164,6 +159,10 @@ class CONTENT_EXPORT DOMStorageContextWrapper
   // To receive memory pressure signals.
   std::unique_ptr<base::MemoryPressureListener> memory_pressure_listener_;
 
+  // Connection to the partition's LocalStorageControl interface within the
+  // Storage Service.
+  mojo::Remote<storage::mojom::LocalStorageControl> local_storage_control_;
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(DOMStorageContextWrapper);
 };
 

content/browser/dom_storage/dom_storage_context_wrapper_unittest.cc

@@ -39,8 +39,8 @@ class DOMStorageContextWrapperTest : public testing::Test {
         /*leveldb_name=*/"");
     session_storage_context->PretendToConnectForTesting();
     context_ = new DOMStorageContextWrapper(
-        fake_mojo_task_runner_, /*mojo_local_storage_context=*/nullptr,
-        session_storage_context);
+        fake_mojo_task_runner_, session_storage_context,
+        mojo::Remote<storage::mojom::LocalStorageControl>());
   }
 
   void TearDown() override {

content/browser/dom_storage/local_storage_context_mojo.h

@@ -20,10 +20,10 @@
 #include "base/trace_event/memory_dump_provider.h"
 #include "components/services/storage/dom_storage/async_dom_storage_database.h"
 #include "components/services/storage/dom_storage/dom_storage_database.h"
+#include "components/services/storage/public/mojom/local_storage_control.mojom.h"
 #include "content/common/content_export.h"
-#include "mojo/public/cpp/bindings/associated_remote.h"
-#include "mojo/public/cpp/bindings/pending_associated_remote.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/blink/public/mojom/dom_storage/storage_area.mojom.h"
 #include "url/origin.h"
@@ -34,8 +34,6 @@ class SpecialStoragePolicy;
 
 namespace content {
 
-struct StorageUsageInfo;
-
 // Used for mojo-based LocalStorage implementation (can be disabled with
 // --disable-mojo-local-storage for now).
 // Created on the UI thread, but all further methods are called on the task
@@ -43,32 +41,27 @@ struct StorageUsageInfo;
 // can involve asynchronous steps, it can only be deleted by calling
 // ShutdownAndDelete (on the correct task runner).
 class CONTENT_EXPORT LocalStorageContextMojo
-    : public base::trace_event::MemoryDumpProvider {
+    : public base::trace_event::MemoryDumpProvider,
+      public storage::mojom::LocalStorageControl {
  public:
-  using GetStorageUsageCallback =
-      base::OnceCallback<void(std::vector<StorageUsageInfo>)>;
-
   static base::FilePath LegacyDatabaseFileNameFromOrigin(
       const url::Origin& origin);
   static url::Origin OriginFromLegacyDatabaseFileName(
       const base::FilePath& file_name);
 
+  // Constructs a Local Storage implementation which will create its root
+  // "Local Storage" directory in |storage_root| if non-empty. |task_runner|
+  // run tasks on the same sequence as the one which constructs this object.
+  // |legacy_task_runner| must support blocking operations and its tasks must
+  // be able to block shutdown. If valid, |receiver| will be bound to this
+  // object to allow for remote control via the LocalStorageControl interface.
   LocalStorageContextMojo(
       const base::FilePath& storage_root,
       scoped_refptr<base::SequencedTaskRunner> task_runner,
       scoped_refptr<base::SequencedTaskRunner> legacy_task_runner,
-      scoped_refptr<storage::SpecialStoragePolicy> special_storage_policy);
+      scoped_refptr<storage::SpecialStoragePolicy> special_storage_policy,
+      mojo::PendingReceiver<storage::mojom::LocalStorageControl> receiver);
 
-  void OpenLocalStorage(
-      const url::Origin& origin,
-      mojo::PendingReceiver<blink::mojom::StorageArea> receiver);
-  void GetStorageUsage(GetStorageUsageCallback callback);
-  // |callback| is called when the deletion is sent to the database and
-  // GetStorageUsage() will not return entries for |origin| anymore.
-  void DeleteStorage(const url::Origin& origin, base::OnceClosure callback);
-  // Ensure that no traces of deleted data are left in the backing storage.
-  void PerformStorageCleanup(base::OnceClosure callback);
-  void Flush();
   void FlushOriginForTesting(const url::Origin& origin);
 
   // Used by content settings to alter the behavior around
@@ -83,13 +76,21 @@ class CONTENT_EXPORT LocalStorageContextMojo
   // policies.
   void ShutdownAndDelete();
 
-  // Clears any caches, to free up as much memory as possible. Next access to
-  // storage for a particular origin will reload the data from the database.
-  void PurgeMemory();
-
   // Clears unused storage areas, when thresholds are reached.
   void PurgeUnusedAreasIfNeeded();
 
+  // storage::mojom::LocalStorageControl implementation:
+  void BindStorageArea(
+      const url::Origin& origin,
+      mojo::PendingReceiver<blink::mojom::StorageArea> receiver) override;
+  void GetUsage(GetUsageCallback callback) override;
+  void DeleteStorage(const url::Origin& origin,
+                     DeleteStorageCallback callback) override;
+  void CleanUpStorage(CleanUpStorageCallback callback) override;
+  void Flush(FlushCallback callback) override;
+  void PurgeMemory() override;
+  void ForceKeepSessionState() override;
+
   // base::trace_event::MemoryDumpProvider implementation.
   bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
                     base::trace_event::ProcessMemoryDump* pmd) override;
@@ -129,21 +130,17 @@ class CONTENT_EXPORT LocalStorageContextMojo
   void DeleteAndRecreateDatabase(const char* histogram_name);
   void OnDBDestroyed(bool recreate_in_memory, leveldb::Status status);
 
-  // The (possibly delayed) implementation of OpenLocalStorage(). Can be called
-  // directly from that function, or through |on_database_open_callbacks_|.
-  void BindLocalStorage(
-      const url::Origin& origin,
-      mojo::PendingReceiver<blink::mojom::StorageArea> receiver);
   StorageAreaHolder* GetOrCreateStorageArea(const url::Origin& origin);
 
-  // The (possibly delayed) implementation of GetStorageUsage(). Can be called
-  // directly from that function, or through |on_database_open_callbacks_|.
-  void RetrieveStorageUsage(GetStorageUsageCallback callback);
+  // The (possibly delayed) implementation of GetUsage(). Can be called directly
+  // from that function, or through |on_database_open_callbacks_|.
+  void RetrieveStorageUsage(GetUsageCallback callback);
   void OnGotMetaData(
-      GetStorageUsageCallback callback,
+      GetUsageCallback callback,
       std::vector<storage::DomStorageDatabase::KeyValuePair> data);
 
-  void OnGotStorageUsageForShutdown(std::vector<StorageUsageInfo> usage);
+  void OnGotStorageUsageForShutdown(
+      std::vector<storage::mojom::LocalStorageUsageInfoPtr> usage);
   void OnShutdownComplete(leveldb::Status status);
 
   void GetStatistics(size_t* total_cache_size, size_t* unused_area_count);
@@ -200,6 +197,8 @@ class CONTENT_EXPORT LocalStorageContextMojo
   // Name of an extra histogram to log open results to, if not null.
   const char* open_result_histogram_ = nullptr;
 
+  mojo::Receiver<storage::mojom::LocalStorageControl> control_receiver_{this};
+
   base::WeakPtrFactory<LocalStorageContextMojo> weak_ptr_factory_{this};
 };
 

content/browser/storage_partition_impl_unittest.cc

@@ -16,7 +16,9 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "components/services/storage/dom_storage/async_dom_storage_database.h"
 #include "components/services/storage/dom_storage/dom_storage_database.h"
+#include "components/services/storage/public/cpp/constants.h"
 #include "content/browser/code_cache/generated_code_cache.h"
 #include "content/browser/code_cache/generated_code_cache_context.h"
 #include "content/browser/dom_storage/local_storage_context_mojo.h"
@@ -44,6 +46,7 @@
 #include "storage/browser/test/mock_special_storage_policy.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/features.h"
+#include "third_party/leveldatabase/env_chromium.h"
 
 #if BUILDFLAG(ENABLE_PLUGINS)
 #include "ppapi/shared_impl/ppapi_constants.h"  // nogncheck
@@ -192,12 +195,11 @@ class RemoveCookieTester {
 class RemoveLocalStorageTester {
  public:
   RemoveLocalStorageTester(content::BrowserTaskEnvironment* task_environment,
-                           TestBrowserContext* profile)
-      : task_environment_(task_environment), dom_storage_context_(nullptr) {
-    dom_storage_context_ =
-        content::BrowserContext::GetDefaultStoragePartition(profile)
-            ->GetDOMStorageContext();
-  }
+                           TestBrowserContext* browser_context)
+      : task_environment_(task_environment),
+        storage_partition_(
+            BrowserContext::GetDefaultStoragePartition(browser_context)),
+        dom_storage_context_(storage_partition_->GetDOMStorageContext()) {}
 
   ~RemoveLocalStorageTester() {
     // Tests which bring up a real Local Storage context need to shut it down
@@ -220,15 +222,35 @@ class RemoveLocalStorageTester {
   }
 
   void AddDOMStorageTestData() {
-    // Note: This test depends on details of how the dom_storage library
-    // stores data in the database.
-
-    static_cast<DOMStorageContextWrapper*>(dom_storage_context_)
-        ->SetLocalStorageDatabaseOpenCallbackForTesting(
-            base::BindLambdaForTesting([&](LocalStorageContextMojo* context) {
-              context->GetDatabaseForTesting().PostTaskWithThisObject(
-                  FROM_HERE, base::BindOnce(&PopulateDatabase));
-            }));
+    // NOTE: Tests which call this method depend on implementation details of
+    // how exactly the Local Storage subsystem stores persistent data.
+
+    base::RunLoop open_loop;
+    leveldb_env::Options options;
+    options.create_if_missing = true;
+    auto database = storage::AsyncDomStorageDatabase::OpenDirectory(
+        std::move(options),
+        storage_partition_->GetPath().Append(storage::kLocalStoragePath),
+        storage::kLocalStorageLeveldbName, base::nullopt,
+        base::ThreadTaskRunnerHandle::Get(),
+        base::BindLambdaForTesting([&](leveldb::Status status) {
+          ASSERT_TRUE(status.ok());
+          open_loop.Quit();
+        }));
+    open_loop.Run();
+
+    base::RunLoop populate_loop;
+    database->database().PostTaskWithThisObject(
+        FROM_HERE,
+        base::BindLambdaForTesting([&](const storage::DomStorageDatabase& db) {
+          PopulateDatabase(db);
+          populate_loop.Quit();
+        }));
+    populate_loop.Run();
+
+    // Ensure that this database is fully closed before returning.
+    database.reset();
+    task_environment_->RunUntilIdle();
   }
 
   static void PopulateDatabase(const storage::DomStorageDatabase& db) {
@@ -298,8 +320,9 @@ class RemoveLocalStorageTester {
   }
 
   // We don't own these pointers.
-  content::BrowserTaskEnvironment* const task_environment_;
-  content::DOMStorageContext* dom_storage_context_;
+  BrowserTaskEnvironment* const task_environment_;
+  StoragePartition* const storage_partition_;
+  DOMStorageContext* dom_storage_context_;
 
   std::vector<content::StorageUsageInfo> infos_;