Don't post delayed task after SharedProtoDatabase::Shutdown()

Bug: 546640
Change-Id: Id2685a95d05e621012bb0e9aab6b129e88e57cd7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391144Reviewed-by: ssid <ssid@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Tomasz Moniuszko <tmoniuszko@opera.com>
Cr-Commit-Position: refs/heads/master@{#808273}

chrome/browser/tab/state/tab_state_db_factory_unittest.cc

@@ -16,9 +16,6 @@ class TabStateDBFactoryTest : public testing::Test {
   TabStateDBFactoryTest() = default;
 
   void SetUp() override {
-    // TODO(crbug.com/546640): This test seems to leak data across tests (on
-    // Windows, even temp files) because SharedProtoDatabase doesn't get
-    // reliably deleted upon steardown.
     ASSERT_TRUE(profile_dir_.CreateUniqueTempDir());
     ASSERT_TRUE(different_profile_dir_.CreateUniqueTempDir());
 

chrome/test/DEPS

@@ -29,6 +29,7 @@ include_rules = [
   "+components/keep_alive_registry",
   "+components/keyed_service/content",
   "+components/keyed_service/core",
+  "+components/leveldb_proto/public",
   "+components/nacl/browser",
   "+components/nacl/common",
   "+components/nacl/renderer",

chrome/test/base/testing_profile.cc

@@ -70,6 +70,7 @@
 #include "components/keyed_service/core/simple_dependency_manager.h"
 #include "components/keyed_service/core/simple_factory_key.h"
 #include "components/keyed_service/core/simple_key_map.h"
+#include "components/leveldb_proto/public/proto_database_provider.h"
 #include "components/omnibox/browser/autocomplete_classifier.h"
 #include "components/omnibox/browser/history_index_restore_observer.h"
 #include "components/omnibox/browser/in_memory_url_index.h"
@@ -493,6 +494,16 @@ TestingProfile::~TestingProfile() {
   if (host_content_settings_map_.get())
     host_content_settings_map_->ShutdownOnUIThread();
 
+  // Make sure SharedProtoDatabase doesn't post delayed tasks anymore.
+  ForEachStoragePartition(
+      this,
+      base::BindRepeating([](content::StoragePartition* storage_partition) {
+        if (auto* provider =
+                storage_partition->GetProtoDatabaseProviderForTesting()) {
+          provider->SetSharedDBDeleteObsoleteDelayForTesting(base::TimeDelta());
+        }
+      }));
+
   // Shutdown storage partitions before we post a task to delete
   // the resource context.
   ShutdownStoragePartitions();

components/leveldb_proto/public/proto_database_provider.cc

@@ -52,4 +52,10 @@ void ProtoDatabaseProvider::GetSharedDBInstance(
                                  base::BindOnce(std::move(callback), db_));
 }
 
+void ProtoDatabaseProvider::SetSharedDBDeleteObsoleteDelayForTesting(
+    base::TimeDelta delay) {
+  if (db_)
+    db_->set_delete_obsolete_delay_for_testing(delay);  // IN-TEST
+}
+
 }  // namespace leveldb_proto

components/leveldb_proto/public/proto_database_provider.h

@@ -11,6 +11,7 @@
 #include "base/files/file_path.h"
 #include "base/memory/ptr_util.h"
 #include "base/sequenced_task_runner.h"
+#include "base/time/time.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/leveldb_proto/internal/proto_database_impl.h"
 #include "components/leveldb_proto/internal/shared_proto_database_provider.h"
@@ -60,9 +61,10 @@ class COMPONENT_EXPORT(LEVELDB_PROTO) ProtoDatabaseProvider {
       const base::FilePath& unique_db_dir,
       const scoped_refptr<base::SequencedTaskRunner>& task_runner);
 
-
   virtual ~ProtoDatabaseProvider();
 
+  void SetSharedDBDeleteObsoleteDelayForTesting(base::TimeDelta delay);
+
  private:
   friend class TestProtoDatabaseProvider;
   template <typename T_>

content/browser/storage_partition_impl.cc

@@ -1662,6 +1662,11 @@ void StoragePartitionImpl::SetProtoDatabaseProvider(
   proto_database_provider_ = std::move(proto_db_provider);
 }
 
+leveldb_proto::ProtoDatabaseProvider*
+StoragePartitionImpl::GetProtoDatabaseProviderForTesting() {
+  return proto_database_provider_.get();
+}
+
 void StoragePartitionImpl::OpenLocalStorage(
     const url::Origin& origin,
     mojo::PendingReceiver<blink::mojom::StorageArea> receiver) {

content/browser/storage_partition_impl.h

@@ -155,6 +155,8 @@ class CONTENT_EXPORT StoragePartitionImpl
   void SetProtoDatabaseProvider(
       std::unique_ptr<leveldb_proto::ProtoDatabaseProvider> proto_db_provider)
       override;
+  leveldb_proto::ProtoDatabaseProvider* GetProtoDatabaseProviderForTesting()
+      override;
   void ClearDataForOrigin(uint32_t remove_mask,
                           uint32_t quota_storage_remove_mask,
                           const GURL& storage_origin) override;

content/public/browser/storage_partition.h

@@ -285,6 +285,11 @@ class CONTENT_EXPORT StoragePartition {
       mojo::PendingRemote<network::mojom::NetworkContext>
           network_context_remote) = 0;
 
+  // Returns the same provider as GetProtoDatabaseProvider() but doesn't create
+  // a new instance and returns nullptr instead.
+  virtual leveldb_proto::ProtoDatabaseProvider*
+  GetProtoDatabaseProviderForTesting() = 0;
+
   // The value pointed to by |settings| should remain valid until the
   // the function is called again with a new value or a nullptr.
   static void SetDefaultQuotaSettingsForTesting(

content/public/test/test_storage_partition.cc

@@ -127,6 +127,11 @@ TestStoragePartition::GetProtoDatabaseProvider() {
 void TestStoragePartition::SetProtoDatabaseProvider(
     std::unique_ptr<leveldb_proto::ProtoDatabaseProvider> proto_db_provider) {}
 
+leveldb_proto::ProtoDatabaseProvider*
+TestStoragePartition::GetProtoDatabaseProviderForTesting() {
+  return nullptr;
+}
+
 #if !defined(OS_ANDROID)
 HostZoomMap* TestStoragePartition::GetHostZoomMap() {
   return host_zoom_map_;

content/public/test/test_storage_partition.h

@@ -142,6 +142,8 @@ class TestStoragePartition : public StoragePartition {
   void SetProtoDatabaseProvider(
       std::unique_ptr<leveldb_proto::ProtoDatabaseProvider> proto_db_provider)
       override;
+  leveldb_proto::ProtoDatabaseProvider* GetProtoDatabaseProviderForTesting()
+      override;
 
   void set_content_index_context(ContentIndexContext* context) {
     content_index_context_ = context;