Fix <webview> WebRequest + NetworkService

Plumbs through some additional WebRequestInfo fields in the Network
Service path to enable <webview> WebRequest (and declarative WebRequest)
APIs to work as expected.

Bug: 721414
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I78eda1c4f286623f1d82e3e5be60dd9beff517bc
Reviewed-on: https://chromium-review.googlesource.com/938675Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539449}

chrome/browser/extensions/chrome_extensions_browser_client.cc

@@ -484,6 +484,21 @@ ChromeExtensionsBrowserClient::GetExtensionNavigationUIData(
   return navigation_data->GetExtensionNavigationUIData();
 }
 
+void ChromeExtensionsBrowserClient::GetTabAndWindowIdForWebContents(
+    content::WebContents* web_contents,
+    int* tab_id,
+    int* window_id) {
+  SessionTabHelper* session_tab_helper =
+      SessionTabHelper::FromWebContents(web_contents);
+  if (session_tab_helper) {
+    *tab_id = session_tab_helper->session_id().id();
+    *window_id = session_tab_helper->window_id().id();
+  } else {
+    *tab_id = -1;
+    *window_id = -1;
+  }
+}
+
 KioskDelegate* ChromeExtensionsBrowserClient::GetKioskDelegate() {
   if (!kiosk_delegate_)
     kiosk_delegate_.reset(new ChromeKioskDelegate());

chrome/browser/extensions/chrome_extensions_browser_client.h

@@ -142,6 +142,9 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {
   bool IsActivityLoggingEnabled(content::BrowserContext* context) override;
   extensions::ExtensionNavigationUIData* GetExtensionNavigationUIData(
       net::URLRequest* request) override;
+  void GetTabAndWindowIdForWebContents(content::WebContents* web_contents,
+                                       int* tab_id,
+                                       int* window_id) override;
   KioskDelegate* GetKioskDelegate() override;
   bool IsLockScreenContext(content::BrowserContext* context) override;
   std::string GetApplicationLocale() override;

extensions/browser/api/web_request/web_request_api.cc

@@ -31,6 +31,7 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_request_info.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/resource_type.h"
@@ -430,6 +431,16 @@ bool WebRequestAPI::MaybeProxyURLLoaderFactory(
   network::mojom::URLLoaderFactoryPtrInfo target_factory_info;
   *factory_request = mojo::MakeRequest(&target_factory_info);
 
+  std::unique_ptr<ExtensionNavigationUIData> navigation_ui_data;
+  if (is_navigation) {
+    int tab_id;
+    int window_id;
+    ExtensionsBrowserClient::Get()->GetTabAndWindowIdForWebContents(
+        content::WebContents::FromRenderFrameHost(frame), &tab_id, &window_id);
+    navigation_ui_data =
+        std::make_unique<ExtensionNavigationUIData>(frame, tab_id, window_id);
+  }
+
   auto proxy = base::MakeRefCounted<WebRequestProxyingURLLoaderFactory>(
       frame->GetProcess()->GetBrowserContext(), info_map_);
   proxies_.emplace(proxy.get(), proxy);
@@ -437,7 +448,7 @@ bool WebRequestAPI::MaybeProxyURLLoaderFactory(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&WebRequestProxyingURLLoaderFactory::StartProxying, proxy,
                      frame->GetProcess()->GetID(), frame->GetRoutingID(),
-                     is_navigation, std::move(proxied_request),
+                     std::move(navigation_ui_data), std::move(proxied_request),
                      std::move(target_factory_info),
                      base::BindOnce(&WebRequestAPI::RemoveProxyThreadSafe,
                                     weak_ptr_factory_.GetWeakPtr(),

extensions/browser/api/web_request/web_request_info.cc

@@ -168,43 +168,19 @@ WebRequestInfo::WebRequestInfo(net::URLRequest* url_request)
   ExtensionNavigationUIData* navigation_ui_data =
       browser_client ? browser_client->GetExtensionNavigationUIData(url_request)
                      : nullptr;
-  if (navigation_ui_data) {
+  if (navigation_ui_data)
     is_browser_side_navigation = true;
-    is_web_view = navigation_ui_data->is_web_view();
-    web_view_instance_id = navigation_ui_data->web_view_instance_id();
-    web_view_rules_registry_id =
-        navigation_ui_data->web_view_rules_registry_id();
 
-    // PlzNavigate: if this request corresponds to a navigation, we always have
-    // FrameData available from the ExtensionNavigationUIData. Use that.
-    frame_data = navigation_ui_data->frame_data();
-  } else if (frame_id >= 0) {
-    // Grab any WebView-related information if relevant.
-    WebViewRendererState::WebViewInfo web_view_info;
-    if (WebViewRendererState::GetInstance()->GetInfo(
-            render_process_id, routing_id, &web_view_info)) {
-      is_web_view = true;
-      web_view_instance_id = web_view_info.instance_id;
-      web_view_rules_registry_id = web_view_info.rules_registry_id;
-      web_view_embedder_process_id = web_view_info.embedder_process_id;
-    }
-
-    // For subresource loads or non-browser-side navigation requests, attempt to
-    // resolve the FrameData immediately anyway using cached information.
-    ExtensionApiFrameIdMap::FrameData data;
-    bool was_cached = ExtensionApiFrameIdMap::Get()->GetCachedFrameDataOnIO(
-        render_process_id, frame_id, &data);
-    if (was_cached)
-      frame_data = data;
-  }
+  InitializeWebViewAndFrameData(navigation_ui_data);
 }
 
-WebRequestInfo::WebRequestInfo(uint64_t request_id,
-                               int render_process_id,
-                               int render_frame_id,
-                               bool is_navigation,
-                               int32_t routing_id,
-                               const network::ResourceRequest& request)
+WebRequestInfo::WebRequestInfo(
+    uint64_t request_id,
+    int render_process_id,
+    int render_frame_id,
+    std::unique_ptr<ExtensionNavigationUIData> navigation_ui_data,
+    int32_t routing_id,
+    const network::ResourceRequest& request)
     : id(request_id),
       url(request.url),
       site_for_cookies(request.site_for_cookies),
@@ -212,7 +188,7 @@ WebRequestInfo::WebRequestInfo(uint64_t request_id,
       routing_id(routing_id),
       frame_id(render_frame_id),
       method(request.method),
-      is_browser_side_navigation(is_navigation),
+      is_browser_side_navigation(!!navigation_ui_data),
       initiator(request.request_initiator),
       type(static_cast<content::ResourceType>(request.resource_type)),
       extra_request_headers(request.headers),
@@ -222,10 +198,11 @@ WebRequestInfo::WebRequestInfo(uint64_t request_id,
   else
     web_request_type = ToWebRequestResourceType(type.value());
 
+  InitializeWebViewAndFrameData(navigation_ui_data.get());
+
   // TODO(https://crbug.com/721414): For this constructor (i.e. the Network
-  // Service case), we are still missing information for |frame_data|,
-  // |is_async|, |request_body_data|, |is_pac_request|, and all things related
-  // to <webview> requests.
+  // Service case), we are still missing information for |is_async|,
+  // |request_body_data|, and |is_pac_request|.
 }
 
 WebRequestInfo::~WebRequestInfo() = default;
@@ -251,4 +228,33 @@ void WebRequestInfo::AddResponseInfoFromResourceResponse(
   response_from_cache = false;
 }
 
+void WebRequestInfo::InitializeWebViewAndFrameData(
+    const ExtensionNavigationUIData* navigation_ui_data) {
+  if (navigation_ui_data) {
+    is_web_view = navigation_ui_data->is_web_view();
+    web_view_instance_id = navigation_ui_data->web_view_instance_id();
+    web_view_rules_registry_id =
+        navigation_ui_data->web_view_rules_registry_id();
+    frame_data = navigation_ui_data->frame_data();
+  } else if (frame_id >= 0) {
+    // Grab any WebView-related information if relevant.
+    WebViewRendererState::WebViewInfo web_view_info;
+    if (WebViewRendererState::GetInstance()->GetInfo(
+            render_process_id, routing_id, &web_view_info)) {
+      is_web_view = true;
+      web_view_instance_id = web_view_info.instance_id;
+      web_view_rules_registry_id = web_view_info.rules_registry_id;
+      web_view_embedder_process_id = web_view_info.embedder_process_id;
+    }
+
+    // For subresource loads we attempt to resolve the FrameData immediately
+    // anyway using cached information.
+    ExtensionApiFrameIdMap::FrameData data;
+    bool was_cached = ExtensionApiFrameIdMap::Get()->GetCachedFrameDataOnIO(
+        render_process_id, frame_id, &data);
+    if (was_cached)
+      frame_data = data;
+  }
+}
+
 }  // namespace extensions

extensions/browser/api/web_request/web_request_info.h

@@ -35,6 +35,8 @@ struct ResourceResponseHead;
 
 namespace extensions {
 
+class ExtensionNavigationUIData;
+
 // A URL request representation used by WebRequest API internals. This structure
 // carries information about an in-progress request.
 struct WebRequestInfo {
@@ -66,7 +68,7 @@ struct WebRequestInfo {
   WebRequestInfo(uint64_t request_id,
                  int render_process_id,
                  int render_frame_id,
-                 bool is_navigation,
+                 std::unique_ptr<ExtensionNavigationUIData> navigation_ui_data,
                  int32_t routing_id,
                  const network::ResourceRequest& request);
 
@@ -164,6 +166,9 @@ struct WebRequestInfo {
   std::unique_ptr<Logger> logger;
 
  private:
+  void InitializeWebViewAndFrameData(
+      const ExtensionNavigationUIData* navigation_ui_data);
+
   DISALLOW_COPY_AND_ASSIGN(WebRequestInfo);
 };
 

extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc

@@ -9,6 +9,7 @@
 #include "base/strings/stringprintf.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/api/web_request/web_request_api.h"
+#include "extensions/browser/extension_navigation_ui_data.h"
 #include "net/http/http_util.h"
 
 namespace extensions {
@@ -47,9 +48,11 @@ WebRequestProxyingURLLoaderFactory::InProgressRequest::~InProgressRequest() {
 void WebRequestProxyingURLLoaderFactory::InProgressRequest::Restart() {
   // Derive a new WebRequestInfo value any time |Restart()| is called, because
   // the details in |request_| may have changed e.g. if we've been redirected.
-  info_.emplace(request_id_, factory_->render_process_id_,
-                factory_->render_frame_id_, factory_->is_navigation_,
-                routing_id_, request_);
+  info_.emplace(
+      request_id_, factory_->render_process_id_, factory_->render_frame_id_,
+      factory_->navigation_ui_data_ ? factory_->navigation_ui_data_->DeepCopy()
+                                    : nullptr,
+      routing_id_, request_);
 
   auto continuation =
       base::BindRepeating(&InProgressRequest::ContinueToBeforeSendHeaders,
@@ -412,7 +415,7 @@ WebRequestProxyingURLLoaderFactory::WebRequestProxyingURLLoaderFactory(
 void WebRequestProxyingURLLoaderFactory::StartProxying(
     int render_process_id,
     int render_frame_id,
-    bool is_navigation,
+    std::unique_ptr<ExtensionNavigationUIData> navigation_ui_data,
     network::mojom::URLLoaderFactoryRequest loader_request,
     network::mojom::URLLoaderFactoryPtrInfo target_factory_info,
     base::OnceClosure on_disconnect) {
@@ -421,7 +424,7 @@ void WebRequestProxyingURLLoaderFactory::StartProxying(
   on_disconnect_ = std::move(on_disconnect);
   render_process_id_ = render_process_id;
   render_frame_id_ = render_frame_id;
-  is_navigation_ = is_navigation;
+  navigation_ui_data_ = std::move(navigation_ui_data);
 
   target_factory_.Bind(std::move(target_factory_info));
   target_factory_.set_connection_error_handler(

extensions/browser/api/web_request/web_request_proxying_url_loader_factory.h

@@ -27,6 +27,7 @@
 
 namespace extensions {
 
+class ExtensionNavigationUIData;
 class InfoMap;
 
 // Owns URLLoaderFactory bindings for WebRequest proxies with the Network
@@ -137,7 +138,7 @@ class WebRequestProxyingURLLoaderFactory
   void StartProxying(
       int render_process_id,
       int render_frame_id,
-      bool is_navigation,
+      std::unique_ptr<ExtensionNavigationUIData> navigation_ui_data,
       network::mojom::URLLoaderFactoryRequest loader_request,
       network::mojom::URLLoaderFactoryPtrInfo target_factory_info,
       base::OnceClosure on_disconnect);
@@ -167,7 +168,7 @@ class WebRequestProxyingURLLoaderFactory
   void* const browser_context_;
   int render_process_id_ = -1;
   int render_frame_id_ = -1;
-  bool is_navigation_ = false;
+  std::unique_ptr<ExtensionNavigationUIData> navigation_ui_data_;
   InfoMap* const info_map_;
   mojo::BindingSet<network::mojom::URLLoaderFactory> proxy_bindings_;
   network::mojom::URLLoaderFactoryPtr target_factory_;

extensions/browser/extension_navigation_ui_data.cc

@@ -25,6 +25,17 @@ ExtensionNavigationUIData::ExtensionNavigationUIData(
   // FrameData that works both for navigations and subresources loads.
 }
 
+ExtensionNavigationUIData::ExtensionNavigationUIData(
+    content::RenderFrameHost* frame_host,
+    int tab_id,
+    int window_id)
+    : ExtensionNavigationUIData(
+          content::WebContents::FromRenderFrameHost(frame_host),
+          tab_id,
+          window_id,
+          ExtensionApiFrameIdMap::GetFrameId(frame_host),
+          ExtensionApiFrameIdMap::GetParentFrameId(frame_host)) {}
+
 // static
 std::unique_ptr<ExtensionNavigationUIData>
 ExtensionNavigationUIData::CreateForMainFrameNavigation(

extensions/browser/extension_navigation_ui_data.h

@@ -24,6 +24,9 @@ class ExtensionNavigationUIData {
   ExtensionNavigationUIData(content::NavigationHandle* navigation_handle,
                             int tab_id,
                             int window_id);
+  ExtensionNavigationUIData(content::RenderFrameHost* frame_host,
+                            int tab_id,
+                            int window_id);
 
   static std::unique_ptr<ExtensionNavigationUIData>
   CreateForMainFrameNavigation(content::WebContents* web_contents,

extensions/browser/extensions_browser_client.cc

@@ -52,6 +52,14 @@ ExtensionsBrowserClient::GetExtensionNavigationUIData(
   return nullptr;
 }
 
+void ExtensionsBrowserClient::GetTabAndWindowIdForWebContents(
+    content::WebContents* web_contents,
+    int* tab_id,
+    int* window_id) {
+  *tab_id = -1;
+  *window_id = -1;
+}
+
 bool ExtensionsBrowserClient::IsExtensionEnabled(
     const std::string& extension_id,
     content::BrowserContext* context) const {

extensions/browser/extensions_browser_client.h

@@ -296,6 +296,15 @@ class ExtensionsBrowserClient {
   virtual ExtensionNavigationUIData* GetExtensionNavigationUIData(
       net::URLRequest* request);
 
+  // Retrives the embedder's notion of tab and window ID for a given
+  // WebContents. May return -1 for either or both values if the embedder does
+  // not implement any such concepts. This is used to support the WebRequest API
+  // exposing such numbers to callers.
+  virtual void GetTabAndWindowIdForWebContents(
+      content::WebContents* web_contents,
+      int* tab_id,
+      int* window_id);
+
   // Returns a delegate that provides kiosk mode functionality.
   virtual KioskDelegate* GetKioskDelegate() = 0;
 

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -272,24 +272,6 @@
 -ExtensionWebRequestApiTest.WebRequestDiceHeaderProtection
 -ExtensionWebRequestApiTest.WebRequestTypes
 -ExtensionWebRequestApiTest.WebRequestWithWithheldPermissions
--WebViewTests/WebViewTest.Shim_TestDeclarativeWebRequestAPI/0
--WebViewTests/WebViewTest.Shim_TestDeclarativeWebRequestAPI/1
--WebViewTests/WebViewTest.Shim_TestDeclarativeWebRequestAPISendMessage/0
--WebViewTests/WebViewTest.Shim_TestDeclarativeWebRequestAPISendMessage/1
--WebViewTests/WebViewTest.Shim_TestDeclarativeWebRequestAPISendMessageSecondWebView/0
--WebViewTests/WebViewTest.Shim_TestDeclarativeWebRequestAPISendMessageSecondWebView/1
--WebViewTests/WebViewTest.Shim_TestWebRequestAPI/0
--WebViewTests/WebViewTest.Shim_TestWebRequestAPI/1
--WebViewTests/WebViewTest.Shim_TestWebRequestAPIErrorOccurred/0
--WebViewTests/WebViewTest.Shim_TestWebRequestAPIErrorOccurred/1
--WebViewTests/WebViewTest.Shim_TestWebRequestAPIGoogleProperty/0
--WebViewTests/WebViewTest.Shim_TestWebRequestAPIGoogleProperty/1
--WebViewTests/WebViewTest.Shim_TestWebRequestAPIOnlyForInstance/0
--WebViewTests/WebViewTest.Shim_TestWebRequestAPIOnlyForInstance/1
--WebViewTests/WebViewTest.Shim_TestWebRequestAPIWithHeaders/0
--WebViewTests/WebViewTest.Shim_TestWebRequestAPIWithHeaders/1
--WebViewTests/WebViewTest.Shim_TestWebRequestListenerSurvivesReparenting/0
--WebViewTests/WebViewTest.Shim_TestWebRequestListenerSurvivesReparenting/1
 
 # http://crbug.com/705114
 # Remove streams concept from code and replace with data pipe passing.