Protected Audiences: Don't name processes based on worklet origin/type.

With late binding to origins/types of preemptively created processes, we
no longer know for certain what origin a process will be associated with
at process creation time, and have no facility to rename processes. So
switch to using the same name for all Protected Audiences dedicated
processes.

The processes shouldn't be around that long, and it's not clear how
useful knowing which process is running which worklet really is. If
this ends up being a problem, we can look into other options.

Bug: 374790901
Change-Id: Ie68dfd4846ff9ec5bff45e2aaaf7ad599d215dd7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5971235
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: mmenke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1374773}

content/browser/interest_group/auction_process_manager.cc

@@ -161,10 +161,6 @@ void AuctionProcessManager::WorkletProcess::ActivateAndBindIfUnbound(
   remove_idle_process_from_manager_timer_.Stop();
 }
 
-std::string AuctionProcessManager::WorkletProcess::ComputeDisplayName() const {
-  return AuctionProcessManager::ComputeDisplayName(worklet_type_, origin_);
-}
-
 void AuctionProcessManager::WorkletProcess::SetService(
     ProcessContext service_context) {
   DCHECK(!service_);
@@ -554,21 +550,6 @@ bool AuctionProcessManager::TryToUseIdleProcessForHandle(
 
 AuctionProcessManager::AuctionProcessManager() = default;
 
-std::string AuctionProcessManager::ComputeDisplayName(
-    WorkletType worklet_type,
-    const url::Origin& origin) {
-  // Use origin and whether it's a buyer/seller in display in task manager,
-  // though admittedly, worklet processes should hopefully not be around too
-  // long.
-  std::string display_name;
-  if (worklet_type == WorkletType::kBidder) {
-    display_name = "Auction Bidder Worklet: ";
-  } else {
-    display_name = "Auction Seller Worklet: ";
-  }
-  return display_name + origin.Serialize();
-}
-
 void AuctionProcessManager::RemovePendingProcessHandle(
     ProcessHandle* process_handle) {
   DCHECK(!process_handle->worklet_process_);
@@ -711,7 +692,7 @@ DedicatedAuctionProcessManager::CreateProcessInternal(
   content::ServiceProcessHost::Launch(
       service.InitWithNewPipeAndPassReceiver(),
       ServiceProcessHost::Options()
-          .WithDisplayName(worklet_process.ComputeDisplayName())
+          .WithDisplayName("Protected Audience JavaScript Process")
 #if BUILDFLAG(IS_MAC)
           // TODO(crbug.com/40812055) add a utility helper for Jit.
           .WithChildFlags(ChildProcessHost::CHILD_RENDERER)

content/browser/interest_group/auction_process_manager.h

@@ -167,12 +167,6 @@ class CONTENT_EXPORT AuctionProcessManager {
     void ActivateAndBindIfUnbound(WorkletType worklet_type,
                                   const url::Origin& origin);
 
-    // Returns the display name to use for a process.
-    std::string ComputeDisplayName() const;
-
-    // Sets `is_bound_` to true. Only used in tests.
-    void set_is_bound_to_origin_for_testing() { is_bound_to_origin_ = true; }
-
     SiteInstance* site_instance() { return site_instance_.get(); }
 
     // Returns a weak pointer so that tests can hold onto a pointer to the
@@ -415,11 +409,6 @@ class CONTENT_EXPORT AuctionProcessManager {
   // `process_handle` will be already filled.
   virtual bool TryUseSharedProcess(ProcessHandle* process_handle) = 0;
 
-  // Returns the display name to use for a process. Separate method so it can be
-  // used in tests.
-  static std::string ComputeDisplayName(WorkletType worklet_type,
-                                        const url::Origin& origin);
-
  private:
   // Contains ProcessHandles which have not yet been assigned processes.
   // Processes requested the earliest are at the start of the list, so processes

content/browser/interest_group/auction_worklet_manager_unittest.cc

@@ -531,25 +531,30 @@ class MockAuctionProcessManager
   MockAuctionProcessManager() = default;
   ~MockAuctionProcessManager() override = default;
 
+  struct WorkletInfo {
+    bool operator==(const WorkletInfo& other) const = default;
+
+    WorkletType worklet_type;
+    url::Origin origin;
+  };
+
   // DedicatedAuctionProcessManager implementation:
   WorkletProcess::ProcessContext CreateProcessInternal(
       WorkletProcess& worklet_process) override {
     mojo::PendingRemote<auction_worklet::mojom::AuctionWorkletService> service;
 
     // Have to flush the receiver set, so that any closed receivers are removed,
-    // before searching for duplicate process names.
+    // before searching for duplicate worklets.
     receiver_set_.FlushForTesting();
 
-    // Each receiver should get a unique display name. This check serves to help
-    // ensure that processes are correctly reused.
-    std::string display_name = worklet_process.ComputeDisplayName();
+    WorkletInfo info{worklet_process.worklet_type(), worklet_process.origin()};
 
     for (auto context : receiver_set_.GetAllContexts()) {
-      EXPECT_NE(*context.second, display_name);
+      EXPECT_NE(*context.second, info);
     }
 
     receiver_set_.Add(this, service.InitWithNewPipeAndPassReceiver(),
-                      display_name);
+                      std::move(info));
 
     return WorkletProcess::ProcessContext(std::move(service));
   }
@@ -613,9 +618,10 @@ class MockAuctionProcessManager
     DCHECK(!bidder_worklet_);
 
     // Make sure this request came over the right pipe.
-    EXPECT_EQ(receiver_set_.current_context(),
-              ComputeDisplayName(AuctionProcessManager::WorkletType::kBidder,
-                                 url::Origin::Create(script_source_url)));
+    EXPECT_EQ(receiver_set_.current_context().worklet_type,
+              AuctionProcessManager::WorkletType::kBidder);
+    EXPECT_EQ(receiver_set_.current_context().origin,
+              url::Origin::Create(script_source_url));
 
     bidder_worklet_ = std::make_unique<MockBidderWorklet>(
         std::move(bidder_worklet_receiver),
@@ -650,9 +656,10 @@ class MockAuctionProcessManager
     DCHECK(!seller_worklet_);
 
     // Make sure this request came over the right pipe.
-    EXPECT_EQ(receiver_set_.current_context(),
-              ComputeDisplayName(AuctionProcessManager::WorkletType::kSeller,
-                                 url::Origin::Create(script_source_url)));
+    EXPECT_EQ(receiver_set_.current_context().worklet_type,
+              AuctionProcessManager::WorkletType::kSeller);
+    EXPECT_EQ(receiver_set_.current_context().origin,
+              url::Origin::Create(script_source_url));
 
     seller_worklet_ = std::make_unique<MockSellerWorklet>(
         std::move(seller_worklet_receiver),
@@ -713,9 +720,9 @@ class MockAuctionProcessManager
 
   // ReceiverSet is last (except for ProcessHandles) so that destroying `this`
   // while there's a pending callback over the pipe will not DCHECK. Each
-  // context is the display name associated with the WorkletProcess used to
-  // create the pipe.
-  mojo::ReceiverSet<auction_worklet::mojom::AuctionWorkletService, std::string>
+  // context is the WorkletProcess associated with the pipe, to make sure
+  // worklets are requested over the correct pipe.
+  mojo::ReceiverSet<auction_worklet::mojom::AuctionWorkletService, WorkletInfo>
       receiver_set_;
 
   bool defer_on_launched_for_handles_ = false;

content/browser/interest_group/mock_auction_process_manager.cc

@@ -557,22 +557,9 @@ MockAuctionProcessManager::~MockAuctionProcessManager() = default;
 AuctionProcessManager::WorkletProcess::ProcessContext
 MockAuctionProcessManager::CreateProcessInternal(
     WorkletProcess& worklet_process) {
-  // Each receiver should get a unique display name. This check serves to help
-  // ensure that processes are correctly reused.
-  std::string display_name = worklet_process.ComputeDisplayName();
-  for (auto receiver : receiver_set_.GetAllContexts()) {
-    EXPECT_NE(*receiver.second, display_name);
-  }
-
   mojo::PendingRemote<auction_worklet::mojom::AuctionWorkletService> service;
   receiver_set_.Add(this, service.InitWithNewPipeAndPassReceiver(),
-                    std::move(display_name));
-  if (!worklet_process.is_bound_to_origin()) {
-    // The display name check isn't compatible with late-binding, as it can
-    // change which origin a process is bound to, so bind processes immediately
-    // to avoid running into issues. See https://crbug.com/374790901.
-    worklet_process.set_is_bound_to_origin_for_testing();
-  }
+                    worklet_process.GetWeakPtrForTesting());
   return WorkletProcess::ProcessContext(std::move(service));
 }
 
@@ -603,11 +590,17 @@ void MockAuctionProcessManager::LoadBidderWorklet(
   load_bidder_worklet_count_++;
   last_load_bidder_worklet_threads_count_ = shared_storage_hosts.size();
 
-  // Make sure this request came over the right pipe.
-  url::Origin owner = url::Origin::Create(script_source_url);
-  EXPECT_EQ(receiver_set_.current_context(),
-            ComputeDisplayName(AuctionProcessManager::WorkletType::kBidder,
-                               url::Origin::Create(script_source_url)));
+  // Make sure this request came over the right pipe, if the WorkletProcess
+  // hasn't been destroyed yet. Can't grab the origin on creation, as the origin
+  // may change in the case of processes that have not yet been bound to an
+  // origin.
+  WorkletProcess* worklet_process = receiver_set_.current_context().get();
+  if (worklet_process) {
+    EXPECT_EQ(worklet_process->origin(),
+              url::Origin::Create(script_source_url));
+    EXPECT_EQ(worklet_process->worklet_type(),
+              AuctionProcessManager::WorkletType::kBidder);
+  }
 
   EXPECT_EQ(0u, bidder_worklets_.count(script_source_url));
   bidder_worklets_.emplace(
@@ -637,10 +630,17 @@ void MockAuctionProcessManager::LoadSellerWorklet(
     auction_worklet::mojom::TrustedSignalsPublicKeyPtr public_key) {
   EXPECT_EQ(0u, seller_worklets_.count(script_source_url));
 
-  // Make sure this request came over the right pipe.
-  EXPECT_EQ(receiver_set_.current_context(),
-            ComputeDisplayName(AuctionProcessManager::WorkletType::kSeller,
-                               url::Origin::Create(script_source_url)));
+  // Make sure this request came over the right pipe, if the WorkletProcess
+  // hasn't been destroyed yet. Can't grab the origin on creation, as the origin
+  // may change in the case of processes that have not yet been bound to an
+  // origin.
+  WorkletProcess* worklet_process = receiver_set_.current_context().get();
+  if (worklet_process) {
+    EXPECT_EQ(worklet_process->origin(),
+              url::Origin::Create(script_source_url));
+    EXPECT_EQ(worklet_process->worklet_type(),
+              AuctionProcessManager::WorkletType::kSeller);
+  }
 
   seller_worklets_.emplace(
       script_source_url,

content/browser/interest_group/mock_auction_process_manager.h

@@ -548,10 +548,11 @@ class MockAuctionProcessManager
   size_t last_load_bidder_worklet_threads_count_ = 0;
 
   // ReceiverSet is last so that destroying `this` while there's a pending
-  // callback over the pipe will not DCHECK. Each entry has a string context,
-  // which is the display name. Used to verify that worklets are created in the
-  // right process.
-  mojo::ReceiverSet<auction_worklet::mojom::AuctionWorkletService, std::string>
+  // callback over the pipe will not DCHECK. Keeps track of the weak pointers
+  // for each WorkletProcess to make sure each process is only used for the
+  // correct worklets.
+  mojo::ReceiverSet<auction_worklet::mojom::AuctionWorkletService,
+                    base::WeakPtr<WorkletProcess>>
       receiver_set_;
 };