Partitioned Storage: Use Quota nodes for Quota managed storage

CL adjusts the CookiesTreeModel to only consider quota nodes for quota managed storage types when the Storage Partitioning feature is enabled. When the feature is disabled, quota nodes are excluded from the tree model static factory function. When the feature is enabled, quota nodes are used to represent all quota managed storage types, and their more granular nodes are excluded. This is achieved by selectively including the relevant helpers (the tree model correctly supporting null helpers) Quota node functionality is also adjusted to actually delete affected hosts on deletion. Previously when removing a quota node, quota was "revoked", which does not remove any data - only resets the available quota (which is immediately re-granted when the page is reloaded) Previously Quota nodes, while included in the Cookies Tree, were not surfaced in the UI anywhere. Even in deprecated detailed control surfaces in settings. Bug: 1321120 Change-Id: I15dffc60444345d43d876b3aa84fa0a25e738b5f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3654190 Reviewed-by: Martin Šrámek <msramek@chromium.org> Reviewed-by: Marijn Kruisselbrink <mek@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com> Cr-Commit-Position: refs/heads/main@{#1018256}
Denger-Network · Jun 27, 2022 · 837b3c8 · 837b3c8
1 parent 489e854
commit 837b3c8
Show file tree

Hide file tree

Showing 21 changed files with 400 additions and 65 deletions.
diff --git a/chrome/app/settings_strings.grdp b/chrome/app/settings_strings.grdp
@@ -3422,6 +3422,9 @@
   <message name="IDS_SETTINGS_COOKIES_CACHE_STORAGE" desc="The text shown when there is Cache Storage (name of a Web standard) in the Cookies table">
     Cache Storage
   </message>
+  <message name="IDS_SETTINGS_COOKIES_QUOTA_STORAGE" desc="The text shown when there is Quota Managed Storage in the Cookies table">
+    Quota-managed storage
+  </message>
 
   <!-- Sync / People Page -->
   <message name="IDS_SETTINGS_PEOPLE" desc="Name of the settings page which manages the user's relationship to Google.">

diff --git a/chrome/app/settings_strings_grdp/IDS_SETTINGS_COOKIES_QUOTA_STORAGE.png.sha1 b/chrome/app/settings_strings_grdp/IDS_SETTINGS_COOKIES_QUOTA_STORAGE.png.sha1
@@ -0,0 +1 @@
+a15ec5fbec4e9cbe4f170edbd44d5e79d0857de5
diff --git a/chrome/browser/browsing_data/access_context_audit_browsertest.cc b/chrome/browser/browsing_data/access_context_audit_browsertest.cc
@@ -474,8 +474,8 @@ IN_PROC_BROWSER_TEST_F(AccessContextAuditBrowserTest, TreeModelDeletion) {
   EXPECT_EQ(cookies.size(),
             kEmbeddedPageCookieCount + kTopLevelPageCookieCount);
 
-  auto tree_model =
-      CookiesTreeModel::CreateForProfile(chrome_test_utils::GetProfile(this));
+  auto tree_model = CookiesTreeModel::CreateForProfileDeprecated(
+      chrome_test_utils::GetProfile(this));
   {
     CookiesTreeObserver observer;
     tree_model->AddCookiesTreeObserver(&observer);

diff --git a/chrome/browser/browsing_data/browsing_data_quota_helper.h b/chrome/browser/browsing_data/browsing_data_quota_helper.h
@@ -13,6 +13,7 @@
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/task/sequenced_task_runner_helpers.h"
+#include "third_party/blink/public/mojom/quota/quota_types.mojom.h"
 
 class BrowsingDataQuotaHelper;
 class Profile;
@@ -69,6 +70,9 @@ class BrowsingDataQuotaHelper
 
   virtual void RevokeHostQuota(const std::string& host) = 0;
 
+  virtual void DeleteHostData(const std::string& host,
+                              blink::mojom::StorageType type) = 0;
+
  protected:
   BrowsingDataQuotaHelper();
   virtual ~BrowsingDataQuotaHelper();

diff --git a/chrome/browser/browsing_data/browsing_data_quota_helper_impl.cc b/chrome/browser/browsing_data/browsing_data_quota_helper_impl.cc
@@ -52,6 +52,15 @@ void BrowsingDataQuotaHelperImpl::RevokeHostQuota(const std::string& host) {
                      this, host));
 }
 
+void BrowsingDataQuotaHelperImpl::DeleteHostData(const std::string& host,
+                                                 StorageType type) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  content::GetIOThreadTaskRunner({})->PostTask(
+      FROM_HERE,
+      base::BindOnce(&BrowsingDataQuotaHelperImpl::DeleteHostDataOnIOThread,
+                     this, host, type));
+}
+
 BrowsingDataQuotaHelperImpl::BrowsingDataQuotaHelperImpl(
     storage::QuotaManager* quota_manager)
     : BrowsingDataQuotaHelper(), quota_manager_(quota_manager) {
@@ -162,6 +171,13 @@ void BrowsingDataQuotaHelperImpl::RevokeHostQuotaOnIOThread(
                      weak_factory_.GetWeakPtr()));
 }
 
+void BrowsingDataQuotaHelperImpl::DeleteHostDataOnIOThread(
+    const std::string& host,
+    blink::mojom::StorageType type) {
+  quota_manager_->DeleteHostData(
+      host, type, base::DoNothingAs<void(blink::mojom::QuotaStatusCode)>());
+}
+
 void BrowsingDataQuotaHelperImpl::DidRevokeHostQuota(
     blink::mojom::QuotaStatusCode /*status*/,
     int64_t /*quota*/) {}
diff --git a/chrome/browser/browsing_data/browsing_data_quota_helper_impl.h b/chrome/browser/browsing_data/browsing_data_quota_helper_impl.h
@@ -34,6 +34,8 @@ class BrowsingDataQuotaHelperImpl : public BrowsingDataQuotaHelper {
  public:
   void StartFetching(FetchResultCallback callback) override;
   void RevokeHostQuota(const std::string& host) override;
+  void DeleteHostData(const std::string& host,
+                      blink::mojom::StorageType type) override;
 
   explicit BrowsingDataQuotaHelperImpl(storage::QuotaManager* quota_manager);
 
@@ -71,6 +73,9 @@ class BrowsingDataQuotaHelperImpl : public BrowsingDataQuotaHelper {
   void RevokeHostQuotaOnIOThread(const std::string& host);
   void DidRevokeHostQuota(blink::mojom::QuotaStatusCode status, int64_t quota);
 
+  void DeleteHostDataOnIOThread(const std::string& host,
+                                blink::mojom::StorageType type);
+
   scoped_refptr<storage::QuotaManager> quota_manager_;
 
   base::WeakPtrFactory<BrowsingDataQuotaHelperImpl> weak_factory_{this};

diff --git a/chrome/browser/browsing_data/browsing_data_quota_helper_unittest.cc b/chrome/browser/browsing_data/browsing_data_quota_helper_unittest.cc
@@ -147,6 +147,10 @@ class BrowsingDataQuotaHelperTest : public testing::Test {
     helper_->RevokeHostQuota(host);
   }
 
+  void DeleteHostData(const std::string& host, blink::mojom::StorageType type) {
+    helper_->DeleteHostData(host, type);
+  }
+
   int64_t quota() { return quota_; }
 
  private:
@@ -245,3 +249,38 @@ TEST_F(BrowsingDataQuotaHelperTest, RevokeHostQuota) {
   content::RunAllTasksUntilIdle();
   EXPECT_EQ(10, quota());
 }
+
+TEST_F(BrowsingDataQuotaHelperTest, DeleteHostData) {
+  static const ClientDefaultBucketData kStorageKeys[] = {
+      {"http://example.com/", StorageType::kTemporary, 1},
+      {"https://example.com/", StorageType::kTemporary, 10},
+      {"http://example.com/", StorageType::kPersistent, 100},
+      {"https://example.com/", StorageType::kSyncable, 1},
+      {"http://example2.com/", StorageType::kTemporary, 1000},
+  };
+  RegisterClient(kStorageKeys);
+
+  DeleteHostData("example.com", StorageType::kPersistent);
+
+  StartFetching();
+  content::RunAllTasksUntilIdle();
+  EXPECT_TRUE(fetching_completed());
+
+  std::set<QuotaInfo> expected, actual;
+  actual.insert(quota_info().begin(), quota_info().end());
+  expected.insert(QuotaInfo("example.com", 11, 0, 1));
+  expected.insert(QuotaInfo("example2.com", 1000, 0, 0));
+  EXPECT_TRUE(expected == actual);
+
+  DeleteHostData("example2.com", StorageType::kTemporary);
+
+  StartFetching();
+  content::RunAllTasksUntilIdle();
+  EXPECT_TRUE(fetching_completed());
+
+  expected.clear();
+  actual.clear();
+  actual.insert(quota_info().begin(), quota_info().end());
+  expected.insert(QuotaInfo("example.com", 11, 0, 1));
+  EXPECT_TRUE(expected == actual);
+}
diff --git a/chrome/browser/browsing_data/cookies_tree_model.cc b/chrome/browser/browsing_data/cookies_tree_model.cc
@@ -51,6 +51,7 @@
 #include "net/cookies/canonical_cookie.h"
 #include "net/cookies/cookie_constants.h"
 #include "net/cookies/cookie_util.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/storage_key/storage_key.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/image_model.h"
@@ -635,12 +636,21 @@ class CookieTreeQuotaNode : public CookieTreeNode {
   ~CookieTreeQuotaNode() override = default;
 
   void DeleteStoredObjects() override {
-    // Calling this function may cause unexpected over-quota state of origin.
-    // However, it'll caused no problem, just prevent usage growth of the
-    // origin.
     LocalDataContainer* container = GetModel()->data_container();
 
     if (container) {
+      if (quota_info_->temporary_usage > 0) {
+        container->quota_helper_->DeleteHostData(
+            quota_info_->host, blink::mojom::StorageType::kTemporary);
+      }
+      if (quota_info_->persistent_usage > 0) {
+        container->quota_helper_->DeleteHostData(
+            quota_info_->host, blink::mojom::StorageType::kPersistent);
+      }
+      if (quota_info_->syncable_usage > 0) {
+        container->quota_helper_->DeleteHostData(
+            quota_info_->host, blink::mojom::StorageType::kSyncable);
+      }
       container->quota_helper_->RevokeHostQuota(quota_info_->host);
       container->quota_info_list_.erase(quota_info_);
     }
@@ -1876,30 +1886,57 @@ CookiesTreeModel::GetCookieDeletionDisabledCallback(Profile* profile) {
 }
 
 // static
-std::unique_ptr<CookiesTreeModel> CookiesTreeModel::CreateForProfile(
+std::unique_ptr<CookiesTreeModel> CookiesTreeModel::CreateForProfileDeprecated(
     Profile* profile) {
   auto* storage_partition = profile->GetDefaultStoragePartition();
   auto* file_system_context = storage_partition->GetFileSystemContext();
   auto* native_io_context = storage_partition->GetNativeIOContext();
 
+  // If partitioned storage is enabled, the quota node is used to represent all
+  // types of quota managed storage. If not, the quota node type is excluded as
+  // it is represented by other types.
+  bool use_quota_only = base::FeatureList::IsEnabled(
+      blink::features::kThirdPartyStoragePartitioning);
+
+  // Types managed by Quota:
+  auto database_helper =
+      use_quota_only
+          ? nullptr
+          : base::MakeRefCounted<browsing_data::DatabaseHelper>(profile);
+  auto cache_helper =
+      use_quota_only ? nullptr
+                     : base::MakeRefCounted<browsing_data::CacheStorageHelper>(
+                           storage_partition);
+  auto indexed_db_helper =
+      use_quota_only ? nullptr
+                     : base::MakeRefCounted<browsing_data::IndexedDBHelper>(
+                           storage_partition);
+  auto file_system_helper =
+      use_quota_only
+          ? nullptr
+          : base::MakeRefCounted<browsing_data::FileSystemHelper>(
+                file_system_context,
+                browsing_data_file_system_util::GetAdditionalFileSystemTypes(),
+                native_io_context);
+  auto service_worker_helper =
+      use_quota_only ? nullptr
+                     : base::MakeRefCounted<browsing_data::ServiceWorkerHelper>(
+                           storage_partition->GetServiceWorkerContext());
+
+  // Quota type itself:
+  auto quota_helper =
+      use_quota_only ? BrowsingDataQuotaHelper::Create(profile) : nullptr;
+
   auto container = std::make_unique<LocalDataContainer>(
       base::MakeRefCounted<browsing_data::CookieHelper>(
           storage_partition, GetCookieDeletionDisabledCallback(profile)),
-      base::MakeRefCounted<browsing_data::DatabaseHelper>(profile),
+      database_helper,
       base::MakeRefCounted<browsing_data::LocalStorageHelper>(profile),
-      /*session_storage_helper=*/nullptr,
-      base::MakeRefCounted<browsing_data::IndexedDBHelper>(storage_partition),
-      base::MakeRefCounted<browsing_data::FileSystemHelper>(
-          file_system_context,
-          browsing_data_file_system_util::GetAdditionalFileSystemTypes(),
-          native_io_context),
-      BrowsingDataQuotaHelper::Create(profile),
-      base::MakeRefCounted<browsing_data::ServiceWorkerHelper>(
-          storage_partition->GetServiceWorkerContext()),
+      /*session_storage_helper=*/nullptr, indexed_db_helper, file_system_helper,
+      quota_helper, service_worker_helper,
       base::MakeRefCounted<browsing_data::SharedWorkerHelper>(
           storage_partition),
-      base::MakeRefCounted<browsing_data::CacheStorageHelper>(
-          storage_partition),
+      cache_helper,
       BrowsingDataMediaLicenseHelper::Create(file_system_context));
 
   return std::make_unique<CookiesTreeModel>(

diff --git a/chrome/browser/browsing_data/cookies_tree_model.h b/chrome/browser/browsing_data/cookies_tree_model.h
@@ -380,7 +380,11 @@ class CookiesTreeModel : public ui::TreeNodeModel<CookieTreeNode> {
   void SetBatchExpectation(int batches_expected, bool reset);
 
   // Create CookiesTreeModel by profile info.
-  static std::unique_ptr<CookiesTreeModel> CreateForProfile(Profile* profile);
+  // 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 partitioned storage, and should not be used in new locations.
+  static std::unique_ptr<CookiesTreeModel> CreateForProfileDeprecated(
+      Profile* profile);
 
   static browsing_data::CookieHelper::IsDeletionDisabledCallback
   GetCookieDeletionDisabledCallback(Profile* profile);