From 65c2955546bb3d4cca0d647c2511b9475b01dce7 Mon Sep 17 00:00:00 2001
From: Kevin McNee <mcnee@chromium.org>
Date: Mon, 11 May 2026 02:37:29 -0700
Subject: [PATCH] Prevent id collision in AppView pending embed requests

The requests are stored in a global map, but the guest instance ids are
scoped to a profile. We now include the profile's unique token as part
of the key to prevent collisions across profiles.

Fixed: 497488593
Change-Id: Ib3b54d15d0ecba1733c53ffed1bdbb4082f59b12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7829969
Auto-Submit: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Giovanni Pezzino <giovax@google.com>
Commit-Queue: Giovanni Pezzino <giovax@google.com>
Cr-Commit-Position: refs/heads/main@{#1628447}
---

diff --git a/chrome/browser/apps/guest_view/app_view_browsertest.cc b/chrome/browser/apps/guest_view/app_view_browsertest.cc
index 9ce2ae4e..045d6f3 100644
--- a/chrome/browser/apps/guest_view/app_view_browsertest.cc
+++ b/chrome/browser/apps/guest_view/app_view_browsertest.cc
@@ -300,6 +300,46 @@
   ContinueEmbedding(guest_app, true);
 }
 
+IN_PROC_BROWSER_TEST_P(AppViewTest, NoCrossProfilePendingRequestCollision) {
+  // Existing test utilities don't handle opening apps in other profiles. For
+  // ease of testing, we simulate an app making an embed request in another
+  // profile and leaving it pending.
+  const base::UnguessableToken other_profile_token =
+      base::UnguessableToken::Create();
+  const int colliding_id = 1;
+  extensions::AppViewGuest::AddFakePendingRequestForTesting(other_profile_token,
+                                                            colliding_id);
+
+  // Now in the original profile, embed an app normally.
+  const extensions::Extension* host_app =
+      LoadAndLaunchPlatformApp("app_view/host_app", "AppViewTest.LAUNCHED");
+  const extensions::Extension* guest_app =
+      InstallPlatformApp("app_view/guest_app");
+
+  ExtensionTestMessageListener on_embed_requested_listener(
+      "AppViewTest.EmbedRequested");
+  ASSERT_TRUE(content::ExecJs(
+      extensions::AppWindowRegistry::Get(browser()->profile())
+          ->GetCurrentAppWindowForApp(host_app->id())
+          ->web_contents(),
+      content::JsReplace("onAppCommand($1, $2);", "EMBED", guest_app->id())));
+  ASSERT_TRUE(on_embed_requested_listener.WaitUntilSatisfied());
+  EXPECT_EQ(
+      2u,
+      extensions::AppViewGuest::GetAllRegisteredInstanceIdsForTesting().size());
+
+  ContinueEmbedding(guest_app, true);
+
+  guest_view::GuestViewBase* guest =
+      test_guest_view_manager()->WaitForSingleGuestViewCreated();
+  EXPECT_EQ(browser()->profile(), guest->browser_context());
+  EXPECT_TRUE(test_guest_view_manager()->WaitUntilAttachedAndLoaded(guest));
+
+  EXPECT_EQ(
+      1u,
+      extensions::AppViewGuest::GetAllRegisteredInstanceIdsForTesting().size());
+}
+
 // Load an AppView which loads a WebView with a text field. The embedding app
 // calls `focus()` on the AppView. The AppView calls `focus()` on the WebView.
 // This should be enough to focus the content of the WebView without further
diff --git a/extensions/browser/guest_view/app_view/app_view_guest.cc b/extensions/browser/guest_view/app_view/app_view_guest.cc
index f51ac6c64..4a610e6 100644
--- a/extensions/browser/guest_view/app_view/app_view_guest.cc
+++ b/extensions/browser/guest_view/app_view/app_view_guest.cc
@@ -10,6 +10,7 @@
 #include "base/functional/bind.h"
 #include "base/lazy_instance.h"
 #include "components/guest_view/browser/guest_view_manager.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/content_features.h"
 #include "extensions/browser/api/app_runtime/app_runtime_api.h"
@@ -53,7 +54,9 @@
   ~ResponseInfo() = default;
 };
 
-using PendingResponseMap = std::map<int, std::unique_ptr<ResponseInfo>>;
+using PendingResponseKey = std::pair<int, base::UnguessableToken>;
+using PendingResponseMap =
+    std::map<PendingResponseKey, std::unique_ptr<ResponseInfo>>;
 base::LazyInstance<PendingResponseMap>::DestructorAtExit
     g_pending_response_map = LAZY_INSTANCE_INITIALIZER;
 
@@ -72,7 +75,8 @@
     const std::string& guest_extension_id,
     content::RenderProcessHost* guest_render_process_host) {
   PendingResponseMap* response_map = g_pending_response_map.Pointer();
-  auto it = response_map->find(guest_instance_id);
+  auto it = response_map->find(
+      std::make_pair(guest_instance_id, browser_context->UniqueToken()));
   // Kill the requesting process if it is not the real guest.
   if (it == response_map->end()) {
     // The requester used an invalid |guest_instance_id|.
@@ -355,10 +359,13 @@
           ->enabled_extensions()
           .GetByID(context_info->extension_id);
 
-  g_pending_response_map.Get().insert(std::make_pair(
-      guest_instance_id(),
+  auto [it, inserted] = g_pending_response_map.Get().insert(std::make_pair(
+      std::make_pair(guest_instance_id(), browser_context()->UniqueToken()),
       std::make_unique<ResponseInfo>(extension, std::move(owned_this),
                                      std::move(callback))));
+  // If there was a conflicting element, then it's unsafe to proceed as `this`
+  // would be destroyed.
+  CHECK(inserted);
 
   base::DictValue embed_request;
   embed_request.Set(appview::kGuestInstanceID, guest_instance_id());
@@ -384,9 +391,18 @@
 std::vector<int> AppViewGuest::GetAllRegisteredInstanceIdsForTesting() {
   std::vector<int> instances;
   for (const auto& key_value : g_pending_response_map.Get()) {
-    instances.push_back(key_value.first);
+    instances.push_back(key_value.first.first);
   }
   return instances;
 }
 
+void AppViewGuest::AddFakePendingRequestForTesting(
+    const base::UnguessableToken& profile_token,
+    int guest_instance_id) {
+  auto [it, inserted] = g_pending_response_map.Get().insert(std::make_pair(
+      std::make_pair(guest_instance_id, profile_token),
+      std::make_unique<ResponseInfo>(nullptr, nullptr, base::NullCallback())));
+  CHECK(inserted);
+}
+
 }  // namespace extensions
diff --git a/extensions/browser/guest_view/app_view/app_view_guest.h b/extensions/browser/guest_view/app_view/app_view_guest.h
index ab01e95..2128e6a 100644
--- a/extensions/browser/guest_view/app_view/app_view_guest.h
+++ b/extensions/browser/guest_view/app_view/app_view_guest.h
@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "base/containers/id_map.h"
+#include "base/unguessable_token.h"
 #include "base/values.h"
 #include "components/guest_view/browser/guest_view.h"
 #include "extensions/browser/guest_view/app_view/app_view_guest_delegate.h"
@@ -53,6 +54,10 @@
   // Sets the AppDelegate for this guest.
   void SetAppDelegateForTest(AppDelegate* delegate);
 
+  static void AddFakePendingRequestForTesting(
+      const base::UnguessableToken& profile_token,
+      int guest_instance_id);
+
  private:
   explicit AppViewGuest(content::RenderFrameHost* owner_rfh);
 

diff --git a/chrome/browser/apps/guest_view/app_view_browsertest.cc b/chrome/browser/apps/guest_view/app_view_browsertest.cc
index 9ce2ae4e..045d6f3 100644
--- a/chrome/browser/apps/guest_view/app_view_browsertest.cc
+++ b/chrome/browser/apps/guest_view/app_view_browsertest.cc
@@ -300,6 +300,46 @@
   ContinueEmbedding(guest_app, true);
 }
 
+IN_PROC_BROWSER_TEST_P(AppViewTest, NoCrossProfilePendingRequestCollision) {
+  // Existing test utilities don't handle opening apps in other profiles. For
+  // ease of testing, we simulate an app making an embed request in another
+  // profile and leaving it pending.
+  const base::UnguessableToken other_profile_token =
+      base::UnguessableToken::Create();
+  const int colliding_id = 1;
+  extensions::AppViewGuest::AddFakePendingRequestForTesting(other_profile_token,
+                                                            colliding_id);
+
+  // Now in the original profile, embed an app normally.
+  const extensions::Extension* host_app =
+      LoadAndLaunchPlatformApp("app_view/host_app", "AppViewTest.LAUNCHED");
+  const extensions::Extension* guest_app =
+      InstallPlatformApp("app_view/guest_app");
+
+  ExtensionTestMessageListener on_embed_requested_listener(
+      "AppViewTest.EmbedRequested");
+  ASSERT_TRUE(content::ExecJs(
+      extensions::AppWindowRegistry::Get(browser()->profile())
+          ->GetCurrentAppWindowForApp(host_app->id())
+          ->web_contents(),
+      content::JsReplace("onAppCommand($1, $2);", "EMBED", guest_app->id())));
+  ASSERT_TRUE(on_embed_requested_listener.WaitUntilSatisfied());
+  EXPECT_EQ(
+      2u,
+      extensions::AppViewGuest::GetAllRegisteredInstanceIdsForTesting().size());
+
+  ContinueEmbedding(guest_app, true);
+
+  guest_view::GuestViewBase* guest =
+      test_guest_view_manager()->WaitForSingleGuestViewCreated();
+  EXPECT_EQ(browser()->profile(), guest->browser_context());
+  EXPECT_TRUE(test_guest_view_manager()->WaitUntilAttachedAndLoaded(guest));
+
+  EXPECT_EQ(
+      1u,
+      extensions::AppViewGuest::GetAllRegisteredInstanceIdsForTesting().size());
+}
+
 // Load an AppView which loads a WebView with a text field. The embedding app
 // calls `focus()` on the AppView. The AppView calls `focus()` on the WebView.
 // This should be enough to focus the content of the WebView without further