Consolidate Private Aggregation features in blink

Moves the existing Private Aggregation features and associated params to blink. Also adjusts the FLEDGE extensions feature to be an associated base::FeatureParam instead of a runtime-enabled feature; this aligns with the other params and avoids unnecessary integration with the Origin Trial framework. Bug: 1422776 Change-Id: If6654bab66348cacb94825909dce4a24fa97959d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4321087 Reviewed-by: Qingxin Wu <qingxinwu@google.com> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Commit-Queue: Alex Turner <alexmt@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Reviewed-by: Nan Lin <linnan@chromium.org> Cr-Commit-Position: refs/heads/main@{#1118385}
chromium · Mar 16, 2023 · 5091bfd · 5091bfd
1 parent 755ba97
commit 5091bfd
Show file tree

Hide file tree

Showing 24 changed files with 112 additions and 237 deletions.
diff --git a/content/browser/interest_group/ad_auction_service_impl_unittest.cc b/content/browser/interest_group/ad_auction_service_impl_unittest.cc
@@ -38,7 +38,6 @@
 #include "content/browser/private_aggregation/private_aggregation_test_utils.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/storage_partition_impl.h"
-#include "content/common/private_aggregation_features.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
@@ -6535,11 +6534,9 @@ class AdAuctionServiceImplPrivateAggregationEnabledTest
     : public AdAuctionServiceImplTest {
  public:
   AdAuctionServiceImplPrivateAggregationEnabledTest() {
-    feature_list_.InitWithFeatures(
-        /*enabled_features=*/{content::kPrivateAggregationApi,
-                              blink::features::
-                                  kPrivateAggregationApiFledgeExtensions},
-        /*disabled_features=*/{});
+    feature_list_.InitAndEnableFeatureWithParameters(
+        blink::features::kPrivateAggregationApi,
+        {{"fledge_extensions_enabled", "true"}});
   }
 
  protected:
@@ -7066,7 +7063,8 @@ class AdAuctionServiceImplPrivateAggregationDisabledTest
     : public AdAuctionServiceImplTest {
  public:
   AdAuctionServiceImplPrivateAggregationDisabledTest() {
-    feature_list_.InitAndDisableFeature(content::kPrivateAggregationApi);
+    feature_list_.InitAndDisableFeature(
+        blink::features::kPrivateAggregationApi);
   }
 
  protected:

diff --git a/content/browser/interest_group/auction_runner_unittest.cc b/content/browser/interest_group/auction_runner_unittest.cc
@@ -43,7 +43,6 @@
 #include "content/browser/interest_group/test_interest_group_manager_impl.h"
 #include "content/browser/interest_group/test_interest_group_private_aggregation_manager.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
-#include "content/common/private_aggregation_features.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_renderer_host.h"
@@ -1163,17 +1162,13 @@ class AuctionRunnerTest : public RenderViewHostTestHarness,
     std::vector<base::test::FeatureRef> disabled_features;
 
     if (should_enable_private_aggregation) {
-      if (should_enable_private_aggregation_fledge_extension) {
-        // Only makes sense to enable fledge extension when
-        // kPrivateAggregationApi is enabled.
-        enabled_features.push_back({content::kPrivateAggregationApi, {}});
-        enabled_features.push_back(
-            {blink::features::kPrivateAggregationApiFledgeExtensions, {}});
-      } else {
-        enabled_features.push_back({content::kPrivateAggregationApi, {}});
-      }
+      enabled_features.push_back(
+          {blink::features::kPrivateAggregationApi,
+           {{"fledge_extensions_enabled",
+             should_enable_private_aggregation_fledge_extension ? "true"
+                                                                : "false"}}});
     } else {
-      disabled_features.push_back(content::kPrivateAggregationApi);
+      disabled_features.push_back(blink::features::kPrivateAggregationApi);
     }
 
     switch (kanon_mode) {

diff --git a/content/browser/interest_group/interest_group_auction_reporter.cc b/content/browser/interest_group/interest_group_auction_reporter.cc
@@ -38,7 +38,6 @@
 #include "content/browser/interest_group/interest_group_storage.h"
 #include "content/browser/private_aggregation/private_aggregation_budget_key.h"
 #include "content/browser/private_aggregation/private_aggregation_manager.h"
-#include "content/common/private_aggregation_features.h"
 #include "content/services/auction_worklet/public/mojom/bidder_worklet.mojom.h"
 #include "content/services/auction_worklet/public/mojom/private_aggregation_request.mojom-forward.h"
 #include "content/services/auction_worklet/public/mojom/private_aggregation_request.mojom.h"
@@ -734,10 +733,9 @@ void InterestGroupAuctionReporter::SendPendingReportsIfNavigated() {
       std::move(private_aggregation_requests_reserved_));
   private_aggregation_requests_reserved_.clear();
 
-  if (base::FeatureList::IsEnabled(content::kPrivateAggregationApi) &&
-      content::kPrivateAggregationApiEnabledInFledge.Get() &&
-      base::FeatureList::IsEnabled(
-          blink::features::kPrivateAggregationApiFledgeExtensions)) {
+  if (base::FeatureList::IsEnabled(blink::features::kPrivateAggregationApi) &&
+      blink::features::kPrivateAggregationApiEnabledInFledge.Get() &&
+      blink::features::kPrivateAggregationApiFledgeExtensionsEnabled.Get()) {
     fenced_frame_reporter_->OnForEventPrivateAggregationRequestsReceived(
         std::move(private_aggregation_requests_non_reserved_));
   }

diff --git a/content/browser/interest_group/interest_group_auction_reporter_unittest.cc b/content/browser/interest_group/interest_group_auction_reporter_unittest.cc
@@ -31,7 +31,6 @@
 #include "content/browser/interest_group/test_interest_group_manager_impl.h"
 #include "content/browser/interest_group/test_interest_group_private_aggregation_manager.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
-#include "content/common/private_aggregation_features.h"
 #include "content/public/test/test_renderer_host.h"
 #include "content/services/auction_worklet/public/mojom/private_aggregation_request.mojom.h"
 #include "mojo/public/cpp/system/functions.h"
@@ -41,6 +40,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/fenced_frame/redacted_fenced_frame_config.h"
 #include "third_party/blink/public/common/interest_group/auction_config.h"
 #include "third_party/blink/public/common/interest_group/interest_group.h"
@@ -93,10 +93,9 @@ class InterestGroupAuctionReporterTest
       public AuctionWorkletManager::Delegate {
  public:
   InterestGroupAuctionReporterTest() {
-    feature_list_.InitWithFeatures(
-        {content::kPrivateAggregationApi,
-         blink::features::kPrivateAggregationApiFledgeExtensions},
-        {});
+    feature_list_.InitAndEnableFeatureWithParameters(
+        blink::features::kPrivateAggregationApi,
+        {{"fledge_extensions_enabled", "true"}});
 
     mojo::SetDefaultProcessErrorHandler(
         base::BindRepeating(&InterestGroupAuctionReporterTest::OnBadMessage,
@@ -1779,7 +1778,8 @@ class InterestGroupAuctionReporterPrivateAggregationDisabledTest
     : public InterestGroupAuctionReporterTest {
  public:
   InterestGroupAuctionReporterPrivateAggregationDisabledTest() {
-    feature_list_.InitAndDisableFeature(content::kPrivateAggregationApi);
+    feature_list_.InitAndDisableFeature(
+        blink::features::kPrivateAggregationApi);
   }
 
  protected:
@@ -1834,9 +1834,9 @@ class InterestGroupAuctionReporterPrivateAggregationFledgeExtensionDisabledTest
     : public InterestGroupAuctionReporterTest {
  public:
   InterestGroupAuctionReporterPrivateAggregationFledgeExtensionDisabledTest() {
-    feature_list_.InitWithFeatures(
-        {content::kPrivateAggregationApi},
-        {blink::features::kPrivateAggregationApiFledgeExtensions});
+    feature_list_.InitAndEnableFeatureWithParameters(
+        blink::features::kPrivateAggregationApi,
+        {{"fledge_extensions_enabled", "false"}});
   }
 
  protected:
@@ -1849,8 +1849,8 @@ TEST_F(
   // This is possible currently because we're not checking the feature flags
   // when collecting PA requests and sending to InterestGroupAuctionReporter,
   // and a compromised worklet can send PA requests to browser process when
-  // feature `blink::features::kPrivateAggregationApiFledgeExtensions` is
-  // disabled.
+  // feature param
+  // `blink::features::kPrivateAggregationApiFledgeExtensionsEnabled` is false.
   private_aggregation_event_map_["event_type"].push_back(
       kWinningBidderGenerateBidPrivateAggregationRequest.Clone());
   private_aggregation_event_map_["event_type2"].push_back(
@@ -1875,8 +1875,8 @@ TEST_F(
 
   // The non-reserved aggregation requests from the bidder's reportWin() method
   // should not be passed along neither. reportWin() could only return PA
-  // requests if the worklet is compromised when feature
-  // `blink::features::kPrivateAggregationApiFledgeExtensions` is disabled.
+  // requests if the worklet is compromised when feature param
+  // `blink::features::kPrivateAggregationApiFledgeExtensionsEnabled` is false.
   WaitForReportWinAndRunCallback(
       /*report_url=*/absl::nullopt, /*ad_beacon_map=*/{},
       MakeRequestPtrVector(

diff --git a/content/browser/interest_group/interest_group_browsertest.cc b/content/browser/interest_group/interest_group_browsertest.cc
@@ -41,7 +41,6 @@
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
-#include "content/common/private_aggregation_features.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
@@ -1353,19 +1352,19 @@ class InterestGroupFencedFrameBrowserTest
       feature_list_.InitWithFeaturesAndParameters(
           {{blink::features::kFencedFrames, {}},
            {features::kPrivacySandboxAdsAPIsOverride, {}},
-           {content::kPrivateAggregationApi, {}},
-           {blink::features::kPrivateAggregationApiFledgeExtensions, {}},
+           {blink::features::kPrivateAggregationApi,
+            {{"fledge_extensions_enabled", "true"}}},
            // This feature allows `runAdAuction()`'s promise to resolve to a
            // `FencedFrameConfig` object upon developer request.
            {blink::features::kFencedFramesAPIChanges, {}}},
-          {/* disabled_features */});
+          /*disabled_features=*/{});
     } else {
       feature_list_.InitWithFeaturesAndParameters(
           {{blink::features::kFencedFrames, {}},
            {features::kPrivacySandboxAdsAPIsOverride, {}},
-           {content::kPrivateAggregationApi, {}},
-           {blink::features::kPrivateAggregationApiFledgeExtensions, {}}},
-          {/* disabled_features */});
+           {blink::features::kPrivateAggregationApi,
+            {{"fledge_extensions_enabled", "true"}}}},
+          /*disabled_features=*/{});
     }
   }
 

diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -7806,8 +7806,9 @@ void RenderFrameHostImpl::CreateNewWindow(
 
 void RenderFrameHostImpl::SendPrivateAggregationRequestsForFencedFrameEvent(
     const std::string& event_type) {
-  if (!base::FeatureList::IsEnabled(
-          blink::features::kPrivateAggregationApiFledgeExtensions)) {
+  if (!base::FeatureList::IsEnabled(blink::features::kPrivateAggregationApi) ||
+      !blink::features::kPrivateAggregationApiEnabledInFledge.Get() ||
+      !blink::features::kPrivateAggregationApiFledgeExtensionsEnabled.Get()) {
     mojo::ReportBadMessage(
         "FLEDGE extensions must be enabled to use reportEvent() for private "
         "aggregation events.");

diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
@@ -2001,13 +2001,6 @@ class SecurityExploitBrowserTestFencedFrames
     https_server()->SetSSLConfig(net::EmbeddedTestServer::CERT_TEST_NAMES);
     SetupCrossSiteRedirector(https_server());
 
-    // Enable FLEDGE extensions for the renderer only. This is done for the
-    // "SendPrivateAggregationEventWithoutEnabling" test so that the renderer
-    // will think that the feature is allowed, while the browser will think
-    // it's disallowed.
-    command_line->AppendSwitchASCII(switches::kEnableBlinkFeatures,
-                                    "PrivateAggregationApiFledgeExtensions");
-
     // EmbeddedTestServer::InitializeAndListen() initializes its |base_url_|
     // which is required below. This cannot invoke Start() however as that kicks
     // off the "EmbeddedTestServer IO Thread" which then races with
@@ -2121,80 +2114,6 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTestFencedFrames,
   EXPECT_EQ(bad_message::RFH_SANDBOX_FLAGS, kill_waiter.Wait());
 }
 
-IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTestFencedFrames,
-                       SendPrivateAggregationEventWithoutEnabling) {
-  // Create a FencedFrameReporter and pass it reporting metadata.
-  scoped_refptr<FencedFrameReporter> fenced_frame_reporter =
-      FencedFrameReporter::CreateForFledge(
-          shell()
-              ->web_contents()
-              ->GetPrimaryMainFrame()
-              ->GetStoragePartition()
-              ->GetURLLoaderFactoryForBrowserProcess(),
-          AttributionManager::FromBrowserContext(
-              shell()->web_contents()->GetBrowserContext()),
-          /*direct_seller_is_seller=*/false,
-          PrivateAggregationManager::GetManager(
-              *shell()->web_contents()->GetBrowserContext()),
-          /*main_frame_origin=*/
-          shell()
-              ->web_contents()
-              ->GetPrimaryMainFrame()
-              ->GetLastCommittedOrigin(),
-          /*winner_origin=*/url::Origin::Create(GURL("https://a.test")));
-  GURL reporting_url(
-      https_server()->GetURL("a.test", "/_report_event_server.html"));
-  // Set valid reporting metadata for buyer.
-  fenced_frame_reporter->OnUrlMappingReady(
-      blink::FencedFrame::ReportingDestination::kBuyer,
-      {{"click", reporting_url}});
-
-  // Navigate the root frame.
-  GURL main_frame_url(https_server()->GetURL("a.test", "/simple_page.html"));
-  EXPECT_TRUE(NavigateToURL(shell(), main_frame_url));
-  RenderFrameHostImpl* root_rfh = static_cast<RenderFrameHostImpl*>(
-      shell()->web_contents()->GetPrimaryMainFrame());
-
-  // Set up the URN mapping for the fenced frame.
-  const GURL fenced_frame_url =
-      https_server()->GetURL("a.test", "/fenced_frames/empty.html");
-  FencedFrameURLMapping& url_mapping =
-      root_rfh->GetPage().fenced_frame_urls_map();
-  auto urn_uuid = test::AddAndVerifyFencedFrameURL(
-      &url_mapping, fenced_frame_url, fenced_frame_reporter);
-
-  // Add the fenced frame.
-  constexpr char kAddFencedFrameScript[] = R"({
-      var fenced_frame = document.createElement('fencedframe');
-      fenced_frame.mode = "opaque-ads";
-      // fenced_frame.src = $1;
-      document.body.appendChild(fenced_frame);
-    })";
-  EXPECT_TRUE(ExecJs(root_rfh, JsReplace(kAddFencedFrameScript, urn_uuid)));
-  std::vector<FencedFrame*> fenced_frames = root_rfh->GetFencedFrames();
-  EXPECT_EQ(fenced_frames.size(), 1u);
-  FencedFrame* new_fenced_frame = fenced_frames.back();
-  RenderFrameHostImpl* fenced_rfh = new_fenced_frame->GetInnerRoot();
-  TestFrameNavigationObserver fenced_frame_observer(fenced_rfh);
-
-  // Navigate the fenced frame.
-  EXPECT_TRUE(ExecJs(root_rfh, JsReplace("fenced_frame.src = $1;", urn_uuid)));
-  fenced_frame_observer.WaitForCommit();
-
-  RenderProcessHostBadMojoMessageWaiter kill_waiter(fenced_rfh->GetProcess());
-
-  // The renderer will think that this is an allowed call because we set the
-  // runtime enabled feature during setup (using the `--enable-blink-features`
-  // flag). Since we did not also set the base::Feature while setting up the
-  // test, the browser will think the feature is not allowed and badmessage the
-  // renderer.
-  std::ignore =
-      ExecJs(fenced_rfh, JsReplace("window.fence.reportEvent($1)", "myevent"));
-
-  absl::optional<std::string> result = kill_waiter.Wait();
-  EXPECT_THAT(result, Optional(HasSubstr("FLEDGE extensions must be enabled")));
-}
-
 INSTANTIATE_TEST_SUITE_P(
     SecurityExploitBrowserTestMojoBlobURLs,
     SecurityExploitBrowserTestMojoBlobURLsP,

diff --git a/content/browser/shared_storage/shared_storage_browsertest.cc b/content/browser/shared_storage/shared_storage_browsertest.cc
@@ -30,7 +30,6 @@
 #include "content/browser/shared_storage/shared_storage_worklet_host_manager.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
-#include "content/common/private_aggregation_features.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
@@ -4687,7 +4686,8 @@ class SharedStoragePrivateAggregationDisabledBrowserTest
     : public SharedStorageBrowserTestBase {
  public:
   SharedStoragePrivateAggregationDisabledBrowserTest() {
-    scoped_feature_list_.InitAndDisableFeature(content::kPrivateAggregationApi);
+    scoped_feature_list_.InitAndDisableFeature(
+        blink::features::kPrivateAggregationApi);
   }
 
  private:
@@ -4719,7 +4719,7 @@ class SharedStoragePrivateAggregationDisabledForSharedStorageOnlyBrowserTest
  public:
   SharedStoragePrivateAggregationDisabledForSharedStorageOnlyBrowserTest() {
     scoped_feature_list_.InitAndEnableFeatureWithParameters(
-        content::kPrivateAggregationApi,
+        blink::features::kPrivateAggregationApi,
         {{"enabled_in_shared_storage", "false"}});
   }
 
@@ -4764,7 +4764,8 @@ class SharedStoragePrivateAggregationEnabledBrowserTest
   };
 
   SharedStoragePrivateAggregationEnabledBrowserTest() {
-    scoped_feature_list_.InitAndEnableFeature(content::kPrivateAggregationApi);
+    scoped_feature_list_.InitAndEnableFeature(
+        blink::features::kPrivateAggregationApi);
   }
 
   void SetUpOnMainThread() override {

diff --git a/content/browser/shared_storage/shared_storage_worklet_host.cc b/content/browser/shared_storage/shared_storage_worklet_host.cc
@@ -21,10 +21,10 @@
 #include "content/browser/shared_storage/shared_storage_url_loader_factory_proxy.h"
 #include "content/browser/shared_storage/shared_storage_worklet_driver.h"
 #include "content/browser/shared_storage/shared_storage_worklet_host_manager.h"
-#include "content/common/private_aggregation_features.h"
 #include "content/common/renderer.mojom.h"
 #include "content/public/browser/browser_context.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/mojom/private_aggregation/private_aggregation_host.mojom.h"
 #include "third_party/blink/public/mojom/shared_storage/shared_storage_worklet_service.mojom.h"
 #include "third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom.h"
@@ -875,8 +875,8 @@ mojo::PendingRemote<blink::mojom::PrivateAggregationHost>
 SharedStorageWorkletHost::MaybeBindPrivateAggregationHost() {
   DCHECK(browser_context_);
 
-  if (!base::FeatureList::IsEnabled(content::kPrivateAggregationApi) ||
-      !content::kPrivateAggregationApiEnabledInSharedStorage.Get()) {
+  if (!base::FeatureList::IsEnabled(blink::features::kPrivateAggregationApi) ||
+      !blink::features::kPrivateAggregationApiEnabledInSharedStorage.Get()) {
     return mojo::PendingRemote<blink::mojom::PrivateAggregationHost>();
   }
 

diff --git a/content/browser/storage_partition_impl.cc b/content/browser/storage_partition_impl.cc
@@ -97,7 +97,6 @@
 #include "content/browser/ssl_private_key_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/browser/worker_host/shared_worker_service_impl.h"
-#include "content/common/private_aggregation_features.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -1442,7 +1441,7 @@ void StoragePartitionImpl::Initialize(
         shared_storage_path, special_storage_policy_);
   }
 
-  if (base::FeatureList::IsEnabled(kPrivateAggregationApi)) {
+  if (base::FeatureList::IsEnabled(blink::features::kPrivateAggregationApi)) {
     private_aggregation_manager_ =
         std::make_unique<PrivateAggregationManagerImpl>(is_in_memory(), path,
                                                         this);

diff --git a/content/common/BUILD.gn b/content/common/BUILD.gn
@@ -143,8 +143,6 @@ source_set("common") {
     "navigation_gesture.h",
     "navigation_params_utils.h",
     "origin_util.cc",
-    "private_aggregation_features.cc",
-    "private_aggregation_features.h",
     "process_type.cc",
     "process_visibility_tracker.cc",
     "process_visibility_tracker.h",