Browsing Data Model: Shared Worker Integration

This CL integrates SharedWorker with the BrowsingDataModel - Move `SharedworkerInfo` to a separate file for easy use. - Create a new data type for kSharedWorker. - Update DataKey variant - Update visitors to implement how to manage the new data key. - Delete SharedWorker by calling into its service to terminate. - Report access is gated by `kMigrateStorageToBDM` feature. Change-Id: I5d475c75ba1a5db364a5a77012e3635e323a08e5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4727721 Auto-Submit: Mariam Ali <alimariam@google.com> Reviewed-by: Christian Dullweber <dullweber@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Mariam Ali <alimariam@google.com> Cr-Commit-Position: refs/heads/main@{#1185065}
chromium · Aug 18, 2023 · 38b03c9 · 38b03c9
1 parent d7e419a
commit 38b03c9
Show file tree

Hide file tree

Showing 15 changed files with 204 additions and 147 deletions.
diff --git a/chrome/browser/browsing_data/browsing_data_model_browsertest.cc b/chrome/browser/browsing_data/browsing_data_model_browsertest.cc
@@ -29,6 +29,7 @@
 #include "components/browsing_data/content/browsing_data_model.h"
 #include "components/browsing_data/content/browsing_data_model_test_util.h"
 #include "components/browsing_data/content/browsing_data_test_util.h"
+#include "components/browsing_data/content/shared_worker_info.h"
 #include "components/browsing_data/core/features.h"
 #include "components/content_settings/browser/page_specific_content_settings.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
@@ -212,6 +213,13 @@ void WaitForModelUpdate(BrowsingDataModel* model, size_t expected_size) {
   }
 }
 
+void RemoveBrowsingDataForDataOwner(BrowsingDataModel* model,
+                                    BrowsingDataModel::DataOwner data_owner) {
+  base::RunLoop run_loop;
+  model->RemoveBrowsingData(data_owner, run_loop.QuitClosure());
+  run_loop.Run();
+}
+
 // Calls the accessStorage javascript function and awaits its completion for
 // each frame in the active web contents for |browser|.
 void EnsurePageAccessedStorage(content::WebContents* web_contents) {
@@ -380,12 +388,7 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
          /*cookie_count=*/0}}});
 
   // Remove origin.
-  {
-    base::RunLoop run_loop;
-    browsing_data_model.get()->RemoveBrowsingData(kTestHost,
-                                                  run_loop.QuitClosure());
-    run_loop.Run();
-  }
+  RemoveBrowsingDataForDataOwner(browsing_data_model.get(), kTestHost);
 
   // Rebuild Browsing Data Model and verify entries are empty.
   browsing_data_model = BuildBrowsingDataModel();
@@ -472,13 +475,7 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest, TrustTokenIssuance) {
         {{BrowsingDataModel::StorageType::kTrustTokens}, 100, 0}}});
 
   // Remove data for the host, and confirm the model updates appropriately.
-  {
-    base::RunLoop run_loop;
-    browsing_data_model->RemoveBrowsingData(kTestHost,
-                                            run_loop.QuitWhenIdleClosure());
-    run_loop.Run();
-  }
-
+  RemoveBrowsingDataForDataOwner(browsing_data_model.get(), kTestHost);
   ValidateBrowsingDataEntries(browsing_data_model.get(), {});
 
   // Build another model from disk, ensuring the data is no longer present.
@@ -516,13 +513,9 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
         {{BrowsingDataModel::StorageType::kInterestGroup},
          /*storage_size=*/1024,
          /*cookie_count=*/0}}});
+
   // Remove Interest Group.
-  {
-    base::RunLoop run_loop;
-    browsing_data_model.get()->RemoveBrowsingData(kTestHost,
-                                                  run_loop.QuitClosure());
-    run_loop.Run();
-  }
+  RemoveBrowsingDataForDataOwner(browsing_data_model.get(), kTestHost);
 
   // Rebuild Browsing Data Model and verify entries are empty.
   browsing_data_model = BuildBrowsingDataModel();
@@ -673,11 +666,7 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
          /*cookie_count=*/0}}});
 
   // Remove datakey from aggregation service and private budgeter.
-  {
-    base::RunLoop run_loop;
-    browsing_data_model->RemoveBrowsingData(kTestHost, run_loop.QuitClosure());
-    run_loop.Run();
-  }
+  RemoveBrowsingDataForDataOwner(browsing_data_model.get(), kTestHost);
 
   // Rebuild Browsing Data Model and verify entries are empty.
   browsing_data_model = BuildBrowsingDataModel();
@@ -717,12 +706,7 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
   ASSERT_EQ(allowed_browsing_data_model->size(), 1u);
 
   // Clear Topic via BDM
-  {
-    base::RunLoop run_loop;
-    allowed_browsing_data_model->RemoveBrowsingData(kTestHost,
-                                                    run_loop.QuitClosure());
-    run_loop.Run();
-  }
+  RemoveBrowsingDataForDataOwner(allowed_browsing_data_model, kTestHost);
 
   // Validate that the allowed browsing data model is cleared.
   ValidateBrowsingDataEntries(allowed_browsing_data_model, {});
@@ -822,12 +806,7 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
       ASSERT_EQ(browsing_data_model->size(), 1u);
 
       // Remove quota entry.
-      {
-        base::RunLoop run_loop;
-        browsing_data_model.get()->RemoveBrowsingData(kTestHost,
-                                                      run_loop.QuitClosure());
-        run_loop.Run();
-      }
+      RemoveBrowsingDataForDataOwner(browsing_data_model.get(), kTestHost);
 
       // Rebuild Browsing Data Model and verify entries are empty.
       browsing_data_model = BuildBrowsingDataModel();
@@ -891,12 +870,7 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
   ASSERT_EQ(browsing_data_model->size(), 1u);
 
   // Remove local storage entry.
-  {
-    base::RunLoop run_loop;
-    browsing_data_model.get()->RemoveBrowsingData(kTestHost,
-                                                  run_loop.QuitClosure());
-    run_loop.Run();
-  }
+  RemoveBrowsingDataForDataOwner(browsing_data_model.get(), kTestHost);
 
   // Rebuild Browsing Data Model and verify entries are empty.
   browsing_data_model = BuildBrowsingDataModel();
@@ -937,14 +911,8 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
     ASSERT_EQ(allowed_browsing_data_model->size(), 1u);
 
     // Delete Local Storage
-    {
-      base::RunLoop run_loop;
-      allowed_browsing_data_model->RemoveBrowsingData(kTestHost,
-                                                      run_loop.QuitClosure());
-      run_loop.Run();
-    }
+    RemoveBrowsingDataForDataOwner(allowed_browsing_data_model, kTestHost);
   }
-
   // Validate that the allowed browsing data model is empty.
   ValidateBrowsingDataEntries(allowed_browsing_data_model, {});
   ASSERT_EQ(allowed_browsing_data_model->size(), 0u);
@@ -983,14 +951,8 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
     ASSERT_EQ(allowed_browsing_data_model->size(), 1u);
 
     // Delete Session Storage
-    {
-      base::RunLoop run_loop;
-      allowed_browsing_data_model->RemoveBrowsingData(kTestHost,
-                                                      run_loop.QuitClosure());
-      run_loop.Run();
-    }
+    RemoveBrowsingDataForDataOwner(allowed_browsing_data_model, kTestHost);
   }
-
   // Validate that the allowed browsing data model is empty.
   ValidateBrowsingDataEntries(allowed_browsing_data_model, {});
   ASSERT_EQ(allowed_browsing_data_model->size(), 0u);
@@ -1041,20 +1003,58 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
       ASSERT_EQ(allowed_browsing_data_model->size(), 1u);
 
       // Delete quota data
-      {
-        base::RunLoop run_loop;
-        allowed_browsing_data_model->RemoveBrowsingData(kTestHost,
-                                                        run_loop.QuitClosure());
-        run_loop.Run();
-      }
+      RemoveBrowsingDataForDataOwner(allowed_browsing_data_model, kTestHost);
     }
-
     // Validate that the allowed browsing data model is empty.
     ValidateBrowsingDataEntries(allowed_browsing_data_model, {});
     ASSERT_EQ(allowed_browsing_data_model->size(), 0u);
   }
 }
 
+IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
+                       SharedWorkerAccessReportedCorrectly) {
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(
+      browser(),
+      https_test_server()->GetURL(kTestHost, "/browsing_data/site_data.html")));
+
+  auto* content_settings =
+      content_settings::PageSpecificContentSettings::GetForFrame(
+          web_contents()->GetPrimaryMainFrame());
+
+  // Validate that the allowed browsing data model is empty.
+  auto* allowed_browsing_data_model =
+      content_settings->allowed_browsing_data_model();
+  ValidateBrowsingDataEntries(allowed_browsing_data_model, {});
+  ASSERT_EQ(allowed_browsing_data_model->size(), 0u);
+
+  SetDataForType("SharedWorker", web_contents());
+  if (GetParam()) {
+    WaitForModelUpdate(allowed_browsing_data_model, /*expected_size=*/1);
+
+    // Validate Shared Worker is reported.
+    url::Origin testOrigin = https_test_server()->GetOrigin(kTestHost);
+    GURL::Replacements replacements;
+    replacements.SetPathStr("browsing_data/shared_worker.js");
+    GURL worker = testOrigin.GetURL().ReplaceComponents(replacements);
+    browsing_data::SharedWorkerInfo data_key(
+        worker, /*name=*/"", blink::StorageKey::CreateFirstParty(testOrigin));
+    ValidateBrowsingDataEntries(
+        allowed_browsing_data_model,
+        {{kTestHost,
+          data_key,
+          {{BrowsingDataModel::StorageType::kSharedWorker},
+           /*storage_size=*/0,
+           /*cookie_count=*/0}}});
+    ASSERT_EQ(allowed_browsing_data_model->size(), 1u);
+
+    // Delete Shared Worker
+    RemoveBrowsingDataForDataOwner(allowed_browsing_data_model, kTestHost);
+  }
+  // Validate that the allowed browsing data model is empty.
+  ValidateBrowsingDataEntries(allowed_browsing_data_model, {});
+  ASSERT_EQ(allowed_browsing_data_model->size(), 0u);
+}
+
 IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
                        PartitionedLocalStorageRemoved) {
   // Build BDM from disk.
@@ -1207,12 +1207,7 @@ IN_PROC_BROWSER_TEST_P(BrowsingDataModelBrowserTest,
   ASSERT_EQ(browsing_data_model->size(), 1u);
 
   // Remove shared dictionary entry.
-  {
-    base::RunLoop run_loop;
-    browsing_data_model.get()->RemoveBrowsingData(kTestHost,
-                                                  run_loop.QuitClosure());
-    run_loop.Run();
-  }
+  RemoveBrowsingDataForDataOwner(browsing_data_model.get(), kTestHost);
 
   // Shared dictionary must have been removed.
   EXPECT_FALSE(HasDataForType("SharedDictionary", web_contents()));
@@ -1419,4 +1414,5 @@ IN_PROC_BROWSER_TEST_P(
          /*cookie_count=*/0}}});
 }
 
+// Boolean parameter used to enable/disable the feature `kMigrateStorageToBDM`.
 INSTANTIATE_TEST_SUITE_P(All, BrowsingDataModelBrowserTest, ::testing::Bool());
diff --git a/chrome/browser/browsing_data/cookies_tree_model.cc b/chrome/browser/browsing_data/cookies_tree_model.cc
@@ -268,7 +268,7 @@ CookieTreeNode::DetailedInfo& CookieTreeNode::DetailedInfo::InitServiceWorker(
 }
 
 CookieTreeNode::DetailedInfo& CookieTreeNode::DetailedInfo::InitSharedWorker(
-    const browsing_data::SharedWorkerHelper::SharedWorkerInfo* shared_worker) {
+    const browsing_data::SharedWorkerInfo* shared_worker) {
   Init(TYPE_SHARED_WORKER);
   shared_worker_info = shared_worker;
   origin = url::Origin::Create(
@@ -677,8 +677,7 @@ class CookieTreeSharedWorkerNode : public CookieTreeNode {
   // |shared_worker_info| should remain valid at least as long as the
   // CookieTreeSharedWorkerNode is valid.
   explicit CookieTreeSharedWorkerNode(
-      std::list<browsing_data::SharedWorkerHelper::SharedWorkerInfo>::iterator
-          shared_worker_info)
+      std::list<browsing_data::SharedWorkerInfo>::iterator shared_worker_info)
       : CookieTreeNode(base::UTF8ToUTF16(shared_worker_info->worker.spec())),
         shared_worker_info_(shared_worker_info) {}
 
@@ -707,8 +706,7 @@ class CookieTreeSharedWorkerNode : public CookieTreeNode {
  private:
   // |shared_worker_info_| is expected to remain valid as long as the
   // CookieTreeSharedWorkerNode is valid.
-  std::list<browsing_data::SharedWorkerHelper::SharedWorkerInfo>::iterator
-      shared_worker_info_;
+  std::list<browsing_data::SharedWorkerInfo>::iterator shared_worker_info_;
 };
 
 ///////////////////////////////////////////////////////////////////////////////

diff --git a/chrome/browser/browsing_data/cookies_tree_model.h b/chrome/browser/browsing_data/cookies_tree_model.h
@@ -111,8 +111,7 @@ class CookieTreeNode : public ui::TreeNode<CookieTreeNode> {
     DetailedInfo& InitServiceWorker(
         const content::StorageUsageInfo* storage_usage_info);
     DetailedInfo& InitSharedWorker(
-        const browsing_data::SharedWorkerHelper::SharedWorkerInfo*
-            shared_worker);
+        const browsing_data::SharedWorkerInfo* shared_worker);
     DetailedInfo& InitCacheStorage(
         const content::StorageUsageInfo* storage_usage_info);
 
@@ -125,8 +124,7 @@ class CookieTreeNode : public ui::TreeNode<CookieTreeNode> {
     raw_ptr<const browsing_data::FileSystemHelper::FileSystemInfo>
         file_system_info = nullptr;
     raw_ptr<const BrowsingDataQuotaHelper::QuotaInfo> quota_info = nullptr;
-    raw_ptr<const browsing_data::SharedWorkerHelper::SharedWorkerInfo>
-        shared_worker_info = nullptr;
+    raw_ptr<const browsing_data::SharedWorkerInfo> shared_worker_info = nullptr;
   };
 
   CookieTreeNode() {}

diff --git a/chrome/browser/browsing_data/local_data_container.cc b/chrome/browser/browsing_data/local_data_container.cc
@@ -50,8 +50,7 @@ LocalDataContainer::CreateFromStoragePartition(
         /*file_system_helper=*/nullptr,
         /*quota_helper=*/nullptr,
         /*service_worker_helper=*/nullptr,
-        base::MakeRefCounted<browsing_data::SharedWorkerHelper>(
-            storage_partition),
+        /*shared_worker_helper=*/nullptr,
         /*cache_storage_helper=*/nullptr);
   }
 

diff --git a/chrome/browser/browsing_data/local_data_container.h b/chrome/browser/browsing_data/local_data_container.h
@@ -48,8 +48,7 @@ class LocalDataContainer {
       std::list<browsing_data::FileSystemHelper::FileSystemInfo>;
   using QuotaInfoList = std::list<BrowsingDataQuotaHelper::QuotaInfo>;
   using ServiceWorkerUsageInfoList = std::list<content::StorageUsageInfo>;
-  using SharedWorkerInfoList =
-      std::list<browsing_data::SharedWorkerHelper::SharedWorkerInfo>;
+  using SharedWorkerInfoList = std::list<browsing_data::SharedWorkerInfo>;
   using CacheStorageUsageInfoList = std::list<content::StorageUsageInfo>;
 
   static std::unique_ptr<LocalDataContainer>

diff --git a/chrome/browser/ui/webui/cookies_tree_model_util.cc b/chrome/browser/ui/webui/cookies_tree_model_util.cc
@@ -197,8 +197,8 @@ CookiesTreeModelUtil::GetCookieTreeNodeDictionary(const CookieTreeNode& node) {
     case CookieTreeNode::DetailedInfo::TYPE_SHARED_WORKER: {
       dict.Set(kKeyType, "shared_worker");
 
-      const browsing_data::SharedWorkerHelper::SharedWorkerInfo&
-          shared_worker_info = *node.GetDetailedInfo().shared_worker_info;
+      const browsing_data::SharedWorkerInfo& shared_worker_info =
+          *node.GetDetailedInfo().shared_worker_info;
 
       dict.Set(kKeyOrigin, shared_worker_info.worker.spec());
       dict.Set(kKeyName, shared_worker_info.name);

diff --git a/components/browsing_data/content/BUILD.gn b/components/browsing_data/content/BUILD.gn
@@ -34,6 +34,8 @@ static_library("content") {
     "service_worker_helper.h",
     "shared_worker_helper.cc",
     "shared_worker_helper.h",
+    "shared_worker_info.cc",
+    "shared_worker_info.h",
   ]
 
   deps = [

diff --git a/components/browsing_data/content/browsing_data_model.cc b/components/browsing_data/content/browsing_data_model.cc
@@ -17,13 +17,15 @@
 #include "base/functional/overloaded.h"
 #include "base/memory/weak_ptr.h"
 #include "components/browsing_data/content/browsing_data_quota_helper.h"
+#include "components/browsing_data/content/shared_worker_info.h"
 #include "components/browsing_data/core/features.h"
 #include "components/services/storage/public/mojom/storage_usage_info.mojom.h"
 #include "components/services/storage/shared_storage/shared_storage_manager.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/dom_storage_context.h"
 #include "content/public/browser/private_aggregation_data_model.h"
+#include "content/public/browser/shared_worker_service.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/storage_partition_config.h"
 #include "content/public/browser/storage_usage_info.h"
@@ -138,6 +140,13 @@ std::string GetDataOwner::GetOwningHost<net::SharedDictionaryIsolationKey>(
   return isolation_key.frame_origin().host();
 }
 
+template <>
+std::string GetDataOwner::GetOwningHost<browsing_data::SharedWorkerInfo>(
+    const browsing_data::SharedWorkerInfo& shared_worker_info) const {
+  DCHECK_EQ(BrowsingDataModel::StorageType::kSharedWorker, storage_type_);
+  return shared_worker_info.storage_key.origin().host();
+}
+
 // Helper which allows the lifetime management of a deletion action to occur
 // separately from the BrowsingDataModel itself.
 struct StorageRemoverHelper {
@@ -252,6 +261,16 @@ void StorageRemoverHelper::Visitor::operator()<blink::StorageKey>(
   }
 }
 
+template <>
+void StorageRemoverHelper::Visitor::operator()<browsing_data::SharedWorkerInfo>(
+    const browsing_data::SharedWorkerInfo& shared_worker_info) {
+  if (types.Has(BrowsingDataModel::StorageType::kSharedWorker)) {
+    helper->storage_partition_->GetSharedWorkerService()->TerminateWorker(
+        shared_worker_info.worker, shared_worker_info.name,
+        shared_worker_info.storage_key);
+  }
+}
+
 template <>
 void StorageRemoverHelper::Visitor::operator()<
     content::InterestGroupManager::InterestGroupDataKey>(