[Merge 110] Cookies Tree Model: Make canned helpers fetch sync

Convert canned helpers from unnecessarily resolving their callbacks
async, to sync.

This does subtly alter the semantics of the TreeModelEndBatch call,
but as there is only a single production user of that function, and
the call will stop existing with the BrowsingDataModel, it is renamed
to indicate it is deprecated.

(cherry picked from commit 72359b1)

Bug: 1404110
Change-Id: Ib12d85ed46f86c95878a2b4cc3f1753d5025af84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4127301
Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Olesia Marukhno <olesiamarukhno@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1088183}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4147138
Owners-Override: Daniel Yip <danielyip@google.com>
Cr-Commit-Position: refs/branch-heads/5481@{#175}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}

chrome/browser/browsing_data/access_context_audit_browsertest.cc

@@ -161,7 +161,9 @@ class CookiesTreeObserver : public CookiesTreeModel::Observer {
     run_loop->Run();
   }
 
-  void TreeModelEndBatch(CookiesTreeModel* model) override { run_loop->Quit(); }
+  void TreeModelEndBatchDeprecated(CookiesTreeModel* model) override {
+    run_loop->Quit();
+  }
 
   void TreeNodesAdded(ui::TreeModel* model,
                       ui::TreeModelNode* parent,

chrome/browser/browsing_data/browsing_data_remover_browsertest_base.cc

@@ -100,9 +100,9 @@ class CookiesTreeObserver : public CookiesTreeModel::Observer {
       : quit_closure_(std::move(quit_closure)) {}
   ~CookiesTreeObserver() override = default;
 
-  void TreeModelBeginBatch(CookiesTreeModel* model) override {}
+  void TreeModelBeginBatchDeprecated(CookiesTreeModel* model) override {}
 
-  void TreeModelEndBatch(CookiesTreeModel* model) override {
+  void TreeModelEndBatchDeprecated(CookiesTreeModel* model) override {
     std::move(quit_closure_).Run();
   }
 

chrome/browser/browsing_data/cookies_tree_model.cc

@@ -1709,7 +1709,7 @@ void CookiesTreeModel::NotifyObserverBeginBatch() {
   // Only notify the model once if we're batching in a nested manner.
   if (batches_started_++ == 0) {
     for (Observer& observer : cookies_observer_list_)
-      observer.TreeModelBeginBatch(this);
+      observer.TreeModelBeginBatchDeprecated(this);
   }
 }
 
@@ -1724,7 +1724,7 @@ void CookiesTreeModel::MaybeNotifyBatchesEnded() {
   if (batches_ended_ == batches_started_ &&
       batches_seen_ == batches_expected_) {
     for (Observer& observer : cookies_observer_list_)
-      observer.TreeModelEndBatch(this);
+      observer.TreeModelEndBatchDeprecated(this);
     SetBatchExpectation(0, true);
   }
 }

chrome/browser/browsing_data/cookies_tree_model.h

@@ -10,6 +10,7 @@
 #include <string>
 #include <vector>
 
+#include "base/gtest_prod_util.h"
 #include "base/memory/raw_ptr.h"
 #include "base/observer_list.h"
 #include "chrome/browser/browsing_data/access_context_audit_database.h"
@@ -285,10 +286,17 @@ class CookiesTreeModel : public ui::TreeNodeModel<CookieTreeNode> {
   // observers for every item added from databases and local storage.
   // We extend the Observer interface to add notifications before and
   // after these batch inserts.
+  // DEPRECATED(crbug.com/1271155): The cookies tree model is slowly being
+  // deprecated, during this process the semantics of the model are nuanced
+  // w.r.t sync vs async operations, and should not be used in new locations.
+  // Batch operations which fetch are always sync if all helpers are canned
+  // (in-memory) versions *and* the CannedLocalStorageHelper is configured not
+  // to check for empty local storages. Batch fetch operations are always async
+  // otherwise.
   class Observer : public ui::TreeModelObserver {
    public:
-    virtual void TreeModelBeginBatch(CookiesTreeModel* model) {}
-    virtual void TreeModelEndBatch(CookiesTreeModel* model) {}
+    virtual void TreeModelBeginBatchDeprecated(CookiesTreeModel* model) {}
+    virtual void TreeModelEndBatchDeprecated(CookiesTreeModel* model) {}
   };
 
   // This class defines the scope for batch updates. It can be created as a
@@ -377,6 +385,8 @@ class CookiesTreeModel : public ui::TreeNodeModel<CookieTreeNode> {
   GetCookieDeletionDisabledCallback(Profile* profile);
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(CookiesTreeModelBrowserTest, BatchesFinishSync);
+
   // Record that one batch has been delivered.
   void RecordBatchSeen();
 

chrome/browser/browsing_data/cookies_tree_model_browsertest.cc

@@ -3,8 +3,10 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/browsing_data/cookies_tree_model.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/test/base/chrome_test_utils.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "components/browsing_data/content/local_shared_objects_container.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
@@ -57,7 +59,9 @@ class CookiesTreeObserver : public CookiesTreeModel::Observer {
     run_loop->Run();
   }
 
-  void TreeModelEndBatch(CookiesTreeModel* model) override { run_loop->Quit(); }
+  void TreeModelEndBatchDeprecated(CookiesTreeModel* model) override {
+    run_loop->Quit();
+  }
 
   void TreeNodesAdded(ui::TreeModel* model,
                       ui::TreeModelNode* parent,
@@ -154,6 +158,34 @@ IN_PROC_BROWSER_TEST_F(CookiesTreeModelBrowserTest, NoQuotaStorage) {
   EXPECT_EQ(1, node_counts[CookieTreeNode::DetailedInfo::TYPE_CACHE_STORAGES]);
 }
 
+IN_PROC_BROWSER_TEST_F(CookiesTreeModelBrowserTest, BatchesFinishSync) {
+  // Confirm that when all helpers fetch functions return synchronously, that
+  // the model has received all expected batches.
+  auto shared_objects = browsing_data::LocalSharedObjectsContainer(
+      chrome_test_utils::GetProfile(this),
+      /*ignore_empty_localstorage=*/false, {}, base::NullCallback());
+  auto local_data_container = std::make_unique<LocalDataContainer>(
+      shared_objects.cookies(), shared_objects.databases(),
+      shared_objects.local_storages(), shared_objects.session_storages(),
+      shared_objects.indexed_dbs(), shared_objects.file_systems(),
+      /*quota_helper=*/nullptr, shared_objects.service_workers(),
+      shared_objects.shared_workers(), shared_objects.cache_storages());
+
+  // Ideally we could observe TreeModelEndBatch, however in the sync case, the
+  // batch will finish during the models constructor, before we can attach an
+  // observer.
+  auto cookies_model = std::make_unique<CookiesTreeModel>(
+      std::move(local_data_container), /*special_storage_policy=*/nullptr);
+
+  // The model will clear all batch information when the batch is completed, so
+  // all 0's here implies any previous batches have been completed, and the
+  // model is not awaiting any helper to finish.
+  EXPECT_EQ(cookies_model->batches_seen_, 0);
+  EXPECT_EQ(cookies_model->batches_started_, 0);
+  EXPECT_EQ(cookies_model->batches_expected_, 0);
+  EXPECT_EQ(cookies_model->batches_seen_, 0);
+}
+
 class CookiesTreeModelBrowserTestQuotaOnly
     : public CookiesTreeModelBrowserTest {
  public:

chrome/browser/browsing_data/local_data_container.cc

@@ -42,78 +42,81 @@ void LocalDataContainer::Init(CookiesTreeModel* model) {
   DCHECK(!model_);
   model_ = model;
 
-  batches_started_ = 0;
+  int batches_started = 0;
   if (cookie_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     cookie_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnCookiesModelInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (database_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     database_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnDatabaseModelInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (local_storage_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     local_storage_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnLocalStorageModelInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (session_storage_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     session_storage_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnSessionStorageModelInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (indexed_db_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     indexed_db_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnIndexedDBModelInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (file_system_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     file_system_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnFileSystemModelInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (quota_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     quota_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnQuotaModelInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (service_worker_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     service_worker_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnServiceWorkerModelInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (shared_worker_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     shared_worker_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnSharedWorkerInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (cache_storage_helper_.get()) {
-    batches_started_++;
+    batches_started++;
     cache_storage_helper_->StartFetching(
         base::BindOnce(&LocalDataContainer::OnCacheStorageModelInfoLoaded,
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
-  model_->SetBatchExpectation(batches_started_, true);
+  // Don't reset batches, as some completions may have been reported
+  // synchronously. As this function is called during model construction, there
+  // can't have been any batches started outside this function.
+  model_->SetBatchExpectation(batches_started, /*reset=*/false);
 }
 
 void LocalDataContainer::OnCookiesModelInfoLoaded(

chrome/browser/browsing_data/local_data_container.h

@@ -137,9 +137,6 @@ class LocalDataContainer {
   // delegate to deliver the updated data to the CookieTreeModel.
   raw_ptr<CookiesTreeModel> model_ = nullptr;
 
-  // Keeps track of how many batches are expected to start.
-  int batches_started_ = 0;
-
   base::WeakPtrFactory<LocalDataContainer> weak_ptr_factory_{this};
 };
 

chrome/browser/ui/views/site_data/page_specific_site_data_dialog_unittest.cc

@@ -4,21 +4,47 @@
 
 #include "chrome/browser/ui/views/site_data/page_specific_site_data_dialog.h"
 
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/page_specific_content_settings_delegate.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "components/browsing_data/content/fake_browsing_data_model.h"
 #include "components/content_settings/browser/page_specific_content_settings.h"
+#include "components/content_settings/common/content_settings_manager.mojom.h"
+#include "components/page_info/core/features.h"
 #include "content/public/browser/cookie_access_details.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace {
 
+using StorageType =
+    content_settings::mojom::ContentSettingsManager::StorageType;
+
 const char kCurrentUrl[] = "https://google.com";
 const char kThirdPartyUrl[] = "https://youtube.com";
 const char kExampleUrl[] = "https://example.com";
 
+void ValidateAllowedUnpartitionedSites(
+    test::PageSpecificSiteDataDialogTestApi* delegate,
+    const std::vector<GURL>& expected_sites_in_order) {
+  auto sites = delegate->GetAllSites();
+  ASSERT_EQ(sites.size(), expected_sites_in_order.size());
+
+  // All sites should be allowed and not fully partitioned.
+  for (auto& site : sites) {
+    EXPECT_EQ(site.setting, CONTENT_SETTING_ALLOW);
+    EXPECT_FALSE(site.is_fully_partitioned);
+  }
+
+  // Hosts should match in order.
+  EXPECT_TRUE(
+      base::ranges::equal(sites, expected_sites_in_order,
+                          [](const auto& site, const auto& expected_site) {
+                            return site.origin.host() == expected_site.host();
+                          }));
+}
+
 }  // namespace
 
 class PageSpecificSiteDataDialogUnitTest
@@ -40,12 +66,16 @@ class PageSpecificSiteDataDialogUnitTest
 
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
+    feature_list_.InitAndEnableFeature(page_info::kPageSpecificSiteDataDialog);
     NavigateAndCommit(GURL(kCurrentUrl));
     content_settings::PageSpecificContentSettings::CreateForWebContents(
         web_contents(),
         std::make_unique<chrome::PageSpecificContentSettingsDelegate>(
             web_contents()));
   }
+
+ protected:
+  base::test::ScopedFeatureList feature_list_;
 };
 
 TEST_F(PageSpecificSiteDataDialogUnitTest, CookieAccessed) {
@@ -79,18 +109,8 @@ TEST_F(PageSpecificSiteDataDialogUnitTest, CookieAccessed) {
 
   auto delegate =
       std::make_unique<test::PageSpecificSiteDataDialogTestApi>(web_contents());
-  auto sites = delegate->GetAllSites();
-  ASSERT_EQ(sites.size(), 2u);
-
-  auto first_site = sites[0];
-  EXPECT_EQ(first_site.origin.host(), GURL(kCurrentUrl).host());
-  EXPECT_EQ(first_site.setting, CONTENT_SETTING_ALLOW);
-  EXPECT_EQ(first_site.is_fully_partitioned, false);
-
-  auto second_site = sites[1];
-  EXPECT_EQ(second_site.origin.host(), GURL(kThirdPartyUrl).host());
-  EXPECT_EQ(second_site.setting, CONTENT_SETTING_ALLOW);
-  EXPECT_EQ(second_site.is_fully_partitioned, false);
+  ValidateAllowedUnpartitionedSites(delegate.get(),
+                                    {GURL(kCurrentUrl), GURL(kThirdPartyUrl)});
 }
 
 TEST_F(PageSpecificSiteDataDialogUnitTest, TrustTokenAccessed) {
@@ -341,3 +361,49 @@ TEST_F(PageSpecificSiteDataDialogUnitTest, RemoveMixedModelData) {
     EXPECT_THAT(sites, testing::UnorderedElementsAreArray(expected_sites));
   }
 }
+
+TEST_F(PageSpecificSiteDataDialogUnitTest, ServiceWorkerAccessed) {
+  // Verify that site data access through CookiesTreeModel is correctly
+  // displayed in the dialog.
+  auto* content_settings = GetContentSettings();
+
+  content_settings->OnServiceWorkerAccessed(
+      GURL(kCurrentUrl), content::AllowServiceWorkerResult::Yes());
+  content_settings->OnServiceWorkerAccessed(
+      GURL(kThirdPartyUrl), content::AllowServiceWorkerResult::Yes());
+
+  auto delegate =
+      std::make_unique<test::PageSpecificSiteDataDialogTestApi>(web_contents());
+
+  ValidateAllowedUnpartitionedSites(delegate.get(),
+                                    {GURL(kCurrentUrl), GURL(kThirdPartyUrl)});
+}
+
+class PageSpecificSiteDataDialogStorageUnitTest
+    : public PageSpecificSiteDataDialogUnitTest,
+      public testing::WithParamInterface<StorageType> {};
+
+TEST_P(PageSpecificSiteDataDialogStorageUnitTest, StorageAccessed) {
+  // Verify that storage access through CookiesTreeModel is correctly
+  // displayed in the dialog.
+  auto* content_settings = GetContentSettings();
+
+  content_settings->OnStorageAccessed(GetParam(), GURL(kCurrentUrl),
+                                      /*blocked_by_policy=*/false);
+  content_settings->OnStorageAccessed(GetParam(), GURL(kThirdPartyUrl),
+                                      /*blocked_by_policy=*/false);
+
+  auto delegate =
+      std::make_unique<test::PageSpecificSiteDataDialogTestApi>(web_contents());
+  ValidateAllowedUnpartitionedSites(delegate.get(),
+                                    {GURL(kCurrentUrl), GURL(kThirdPartyUrl)});
+}
+
+INSTANTIATE_TEST_SUITE_P(PageSpecificSiteDataDialogStorageUnitTestInstance,
+                         PageSpecificSiteDataDialogStorageUnitTest,
+                         testing::Values(StorageType::DATABASE,
+                                         StorageType::LOCAL_STORAGE,
+                                         StorageType::SESSION_STORAGE,
+                                         StorageType::FILE_SYSTEM,
+                                         StorageType::INDEXED_DB,
+                                         StorageType::CACHE));

chrome/browser/ui/webui/settings/site_settings_handler.cc

@@ -2058,7 +2058,7 @@ void SiteSettingsHandler::TreeNodesRemoved(ui::TreeModel* model,
 void SiteSettingsHandler::TreeNodeChanged(ui::TreeModel* model,
                                           ui::TreeModelNode* node) {}
 
-void SiteSettingsHandler::TreeModelEndBatch(CookiesTreeModel* model) {
+void SiteSettingsHandler::TreeModelEndBatchDeprecated(CookiesTreeModel* model) {
   ModelBuilt();
 }
 

chrome/browser/ui/webui/settings/site_settings_handler.h

@@ -71,7 +71,7 @@ class SiteSettingsHandler
                         size_t start,
                         size_t count) override;
   void TreeNodeChanged(ui::TreeModel* model, ui::TreeModelNode* node) override;
-  void TreeModelEndBatch(CookiesTreeModel* model) override;
+  void TreeModelEndBatchDeprecated(CookiesTreeModel* model) override;
 
   // content_settings::Observer:
   void OnContentSettingChanged(const ContentSettingsPattern& primary_pattern,