Add render_process_id field to WebBundleTokenParams

WebBundleManager uses URLLoaderFactoryParams::process_id for request matching, but for subframe navigation requests it is kBrowserProcessId. This patch adds a new field to WebBundleTokenParams, render_process_id, which will be set by the browser process in a follow-up CL. WebBundleManager uses that field for request matching, if and only if URLLoaderFactoryParams::process_id is kBrowserProcessId. Bug: 1082020,1149255 Change-Id: I56edb6d52b062f50654f8a2f7e2a45f32513acab Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2644555 Reviewed-by: Hayato Ito <[email protected]> Reviewed-by: Kinuko Yasuda <[email protected]> Reviewed-by: Tsuyoshi Horo <[email protected]> Commit-Queue: Kunihiko Sakamoto <[email protected]> Cr-Commit-Position: refs/heads/master@{#846971}
yjfxfjch · Jan 26, 2021 · 3ddfa0b · 3ddfa0b
1 parent 6e99525
commit 3ddfa0b
Show file tree

Hide file tree

Showing 9 changed files with 68 additions and 17 deletions.
diff --git a/services/network/public/cpp/resource_request.cc b/services/network/public/cpp/resource_request.cc
@@ -84,6 +84,7 @@ ResourceRequest::WebBundleTokenParams::operator=(
     const WebBundleTokenParams& other) {
   token = other.token;
   handle = other.CloneHandle();
+  render_process_id = other.render_process_id;
   return *this;
 }
 
@@ -92,10 +93,16 @@ ResourceRequest::WebBundleTokenParams::WebBundleTokenParams(
     mojo::PendingRemote<mojom::WebBundleHandle> handle)
     : token(token), handle(std::move(handle)) {}
 
+ResourceRequest::WebBundleTokenParams::WebBundleTokenParams(
+    const base::UnguessableToken& token,
+    int32_t render_process_id)
+    : token(token), render_process_id(render_process_id) {}
+
 bool ResourceRequest::WebBundleTokenParams::EqualsForTesting(
     const WebBundleTokenParams& other) const {
   return token == other.token &&
-         ((handle && other.handle) || (!handle && !other.handle));
+         ((handle && other.handle) || (!handle && !other.handle)) &&
+         render_process_id == other.render_process_id;
 }
 
 mojo::PendingRemote<mojom::WebBundleHandle>

diff --git a/services/network/public/cpp/resource_request.h b/services/network/public/cpp/resource_request.h
@@ -72,6 +72,8 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceRequest {
 
     WebBundleTokenParams(const base::UnguessableToken& token,
                          mojo::PendingRemote<mojom::WebBundleHandle> handle);
+    WebBundleTokenParams(const base::UnguessableToken& token,
+                         int32_t render_process_id);
 
     // For testing. Regarding the equality of |handle|, |this| equals |other| if
     // both |handle| exists, or neither exists, because we cannot test the
@@ -82,6 +84,7 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceRequest {
 
     base::UnguessableToken token;
     mojo::PendingRemote<mojom::WebBundleHandle> handle;
+    int32_t render_process_id = -1;
   };
 
   ResourceRequest();

diff --git a/services/network/public/cpp/url_request_mojom_traits.cc b/services/network/public/cpp/url_request_mojom_traits.cc
@@ -173,6 +173,7 @@ bool StructTraits<network::mojom::WebBundleTokenParamsDataView,
   }
   out->handle = data.TakeWebBundleHandle<
       mojo::PendingRemote<network::mojom::WebBundleHandle>>();
+  out->render_process_id = data.render_process_id();
   return true;
 }
 

diff --git a/services/network/public/cpp/url_request_mojom_traits.h b/services/network/public/cpp/url_request_mojom_traits.h
@@ -103,6 +103,10 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE)
         const_cast<network::ResourceRequest::WebBundleTokenParams&>(params)
             .handle);
   }
+  static int32_t render_process_id(
+      const network::ResourceRequest::WebBundleTokenParams& params) {
+    return params.render_process_id;
+  }
 
   static bool Read(network::mojom::WebBundleTokenParamsDataView data,
                    network::ResourceRequest::WebBundleTokenParams* out);

diff --git a/services/network/public/mojom/url_loader.mojom b/services/network/public/mojom/url_loader.mojom
@@ -93,6 +93,10 @@ struct WebBundleTokenParams {
   // the WebBundle data in the network process when the renderer-side endpoint
   // is deleted.
   pending_remote<WebBundleHandle>? web_bundle_handle;
+  // Renderer process ID of the request initiator frame. Set by the browser
+  // process, for subframe navigation requests to bundled resources. Not used
+  // for subresource requests sent by renderer processes.
+  int32 render_process_id;
 };
 
 // Typemapped to network::ResourceRequest.

diff --git a/services/network/url_loader_factory.cc b/services/network/url_loader_factory.cc
@@ -136,7 +136,7 @@ void URLLoaderFactory::CreateLoaderAndStart(
     // Load a subresource from a WebBundle.
     base::WeakPtr<WebBundleURLLoaderFactory> web_bundle_url_loader_factory =
         context_->GetWebBundleManager().GetWebBundleURLLoaderFactory(
-            url_request.web_bundle_token_params->token, params_->process_id);
+            *url_request.web_bundle_token_params, params_->process_id);
     if (web_bundle_url_loader_factory) {
       web_bundle_url_loader_factory->CreateLoaderAndStart(
           std::move(receiver), routing_id, request_id, options, url_request,

diff --git a/services/network/web_bundle_manager.cc b/services/network/web_bundle_manager.cc
@@ -49,9 +49,14 @@ WebBundleManager::CreateWebBundleURLLoaderFactory(
 
 base::WeakPtr<WebBundleURLLoaderFactory>
 WebBundleManager::GetWebBundleURLLoaderFactory(
-    const base::UnguessableToken& web_bundle_token,
+    const ResourceRequest::WebBundleTokenParams& token_params,
     int32_t process_id) {
-  auto it = factories_.find({process_id, web_bundle_token});
+  // If the request is from the browser process, use
+  // WebBundleTokenParams::render_process_id for matching.
+  if (process_id == mojom::kBrowserProcessId)
+    process_id = token_params.render_process_id;
+
+  auto it = factories_.find({process_id, token_params.token});
   if (it == factories_.end()) {
     return nullptr;
   }

diff --git a/services/network/web_bundle_manager.h b/services/network/web_bundle_manager.h
@@ -33,7 +33,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) WebBundleManager {
       const mojom::URLLoaderFactoryParamsPtr& factory_params);
 
   base::WeakPtr<WebBundleURLLoaderFactory> GetWebBundleURLLoaderFactory(
-      const base::UnguessableToken& token,
+      const ResourceRequest::WebBundleTokenParams& params,
       int32_t process_id);
 
  private:

diff --git a/services/network/web_bundle_manager_unittest.cc b/services/network/web_bundle_manager_unittest.cc
@@ -38,39 +38,66 @@ TEST_F(WebBundleManagerTest, NoFactoryExistsForDifferentProcessId) {
   mojo::PendingRemote<network::mojom::WebBundleHandle> handle;
   mojo::PendingReceiver<network::mojom::WebBundleHandle> receiver =
       handle.InitWithNewPipeAndPassReceiver();
-  auto token_params =
-      ResourceRequest::WebBundleTokenParams(token, std::move(handle));
+  ResourceRequest::WebBundleTokenParams create_params(token, std::move(handle));
 
   auto factory = manager.CreateWebBundleURLLoaderFactory(
-      GURL(kBundleUrl), token_params, factory_params);
+      GURL(kBundleUrl), create_params, factory_params);
   ASSERT_TRUE(factory);
-  ASSERT_TRUE(
-      manager.GetWebBundleURLLoaderFactory(token, factory_params->process_id));
-  ASSERT_FALSE(manager.GetWebBundleURLLoaderFactory(token, process_id2));
+
+  ResourceRequest::WebBundleTokenParams find_params(token,
+                                                    mojom::kInvalidProcessId);
+  ASSERT_TRUE(manager.GetWebBundleURLLoaderFactory(find_params,
+                                                   factory_params->process_id));
+  ASSERT_FALSE(manager.GetWebBundleURLLoaderFactory(find_params, process_id2));
+}
+
+TEST_F(WebBundleManagerTest, UseProcesIdInTokenParamsForRequestsFromBrowser) {
+  WebBundleManager manager;
+  base::UnguessableToken token = base::UnguessableToken::Create();
+  auto factory_params = mojom::URLLoaderFactoryParams::New();
+  factory_params->process_id = process_id1;
+  mojo::PendingRemote<network::mojom::WebBundleHandle> handle;
+  mojo::PendingReceiver<network::mojom::WebBundleHandle> receiver =
+      handle.InitWithNewPipeAndPassReceiver();
+  ResourceRequest::WebBundleTokenParams create_params(token, std::move(handle));
+
+  auto factory = manager.CreateWebBundleURLLoaderFactory(
+      GURL(kBundleUrl), create_params, factory_params);
+  ASSERT_TRUE(factory);
+
+  ResourceRequest::WebBundleTokenParams find_params1(token, process_id1);
+  ASSERT_TRUE(manager.GetWebBundleURLLoaderFactory(find_params1,
+                                                   mojom::kBrowserProcessId));
+  ASSERT_FALSE(manager.GetWebBundleURLLoaderFactory(find_params1, process_id2));
+  ResourceRequest::WebBundleTokenParams find_params2(token, process_id2);
+  ASSERT_FALSE(manager.GetWebBundleURLLoaderFactory(find_params2,
+                                                    mojom::kBrowserProcessId));
 }
 
 TEST_F(WebBundleManagerTest, RemoveFactoryWhenDisconnected) {
   WebBundleManager manager;
   base::UnguessableToken token = base::UnguessableToken::Create();
+  ResourceRequest::WebBundleTokenParams find_params(token,
+                                                    mojom::kInvalidProcessId);
   auto factory_params = mojom::URLLoaderFactoryParams::New();
   factory_params->process_id = 123;
   {
     mojo::PendingRemote<network::mojom::WebBundleHandle> handle;
     mojo::PendingReceiver<network::mojom::WebBundleHandle> receiver =
         handle.InitWithNewPipeAndPassReceiver();
-    auto token_params =
-        ResourceRequest::WebBundleTokenParams(token, std::move(handle));
+    ResourceRequest::WebBundleTokenParams create_params(token,
+                                                        std::move(handle));
 
     auto factory = manager.CreateWebBundleURLLoaderFactory(
-        GURL(kBundleUrl), token_params, factory_params);
+        GURL(kBundleUrl), create_params, factory_params);
     ASSERT_TRUE(factory);
     ASSERT_TRUE(manager.GetWebBundleURLLoaderFactory(
-        token, factory_params->process_id));
+        find_params, factory_params->process_id));
     // Getting out of scope to delete |receiver|.
   }
   base::RunLoop().RunUntilIdle();
-  EXPECT_FALSE(
-      manager.GetWebBundleURLLoaderFactory(token, factory_params->process_id))
+  EXPECT_FALSE(manager.GetWebBundleURLLoaderFactory(find_params,
+                                                    factory_params->process_id))
       << "The manager should remove a factory when the handle is disconnected.";
 }