<webview> content scripts also need a separate URLLoaderFactory.

After r606352, default URLLoaderFactory does not make any CORB
exceptions for extensions and/or apps.  Separate URLLoaderFactories
(relaxing CORB) need to be created for content scripts.

One scenario, where separate URLLoaderFactories should be created, is
content scripts injected into <webview> tag.  Such scripts execute
fetches with |request_initiator| set to the origin of the <webview>
embedder and so permissions of the embedder need to be taken into
account (e.g. a Chrome App might be able to make cross-origin requests).

This CL adds a regression test for this scenario (without product code
changes the new test fails with --enable-features=NetworkService flag)
and fixes the product code to trigger creation of separate
URLLoaderFactory when needed.

For an overview of separate URLLoaderFactories for extensions, see
https://docs.google.com/document/d/1fuBUmLZj1H319jMkK8waUbf5WdQS0N95J_mWXVzbP7A

Bug: 846346
Change-Id: I6ac0f42df7562a01c14cbe6740417b6d94b51810
Reviewed-on: https://chromium-review.googlesource.com/c/1337822
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608859}

chrome/browser/extensions/cross_origin_read_blocking_browsertest.cc

@@ -19,6 +19,8 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
+#include "chrome/browser/ui/extensions/app_launch_params.h"
+#include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/render_frame_host.h"
@@ -70,7 +72,7 @@ class CrossOriginReadBlockingExtensionTest : public ExtensionBrowserTest {
 
     const char kManifestTemplate[] = R"(
         {
-          "name": "CrossOriginReadBlockingTest",
+          "name": "CrossOriginReadBlockingTest - Extension",
           "version": "1.0",
           "manifest_version": 2,
           "permissions": ["tabs", "*://*/*"],
@@ -93,6 +95,38 @@ class CrossOriginReadBlockingExtensionTest : public ExtensionBrowserTest {
     return extension_;
   }
 
+  const Extension* InstallApp() {
+    const char kManifest[] = R"(
+        {
+          "name": "CrossOriginReadBlockingTest - App",
+          "version": "1.0",
+          "manifest_version": 2,
+          "permissions": ["*://*/*", "webview"],
+          "app": {
+            "background": {
+              "scripts": ["background_script.js"]
+            }
+          }
+        } )";
+    dir_.WriteManifest(kManifest);
+
+    const char kBackgroungScript[] = R"(
+        chrome.app.runtime.onLaunched.addListener(function() {
+          chrome.app.window.create('page.html', {}, function () {});
+        });
+    )";
+    dir_.WriteFile(FILE_PATH_LITERAL("background_script.js"),
+                   kBackgroungScript);
+
+    const char kPage[] = R"(
+        <div id="webview-tag-container"></div>
+    )";
+    dir_.WriteFile(FILE_PATH_LITERAL("page.html"), kPage);
+
+    extension_ = LoadExtension(dir_.UnpackedPath());
+    return extension_;
+  }
+
   bool RegisterServiceWorkerForExtension(
       const std::string& service_worker_script) {
     const char kServiceWorkerPath[] = "service_worker.js";
@@ -196,6 +230,16 @@ class CrossOriginReadBlockingExtensionTest : public ExtensionBrowserTest {
 
   const Extension* extension() { return extension_; }
 
+  std::string CreateFetchScript(const GURL& resource) {
+    const char kXhrScriptTemplate[] = R"(
+      fetch($1)
+        .then(response => response.text())
+        .then(text => domAutomationController.send(text))
+        .catch(err => domAutomationController.send('error: ' + err));
+    )";
+    return content::JsReplace(kXhrScriptTemplate, resource);
+  }
+
  private:
   // Asks the test |extension_| to inject |content_script| into |web_contents|.
   //
@@ -254,16 +298,6 @@ class CrossOriginReadBlockingExtensionTest : public ExtensionBrowserTest {
     return true;
   }
 
-  std::string CreateFetchScript(const GURL& resource) {
-    const char kXhrScriptTemplate[] = R"(
-      fetch($1)
-        .then(response => response.text())
-        .then(text => domAutomationController.send(text))
-        .catch(err => domAutomationController.send('error: ' + err));
-    )";
-    return content::JsReplace(kXhrScriptTemplate, resource);
-  }
-
   // FetchCallback represents a function that executes |fetch_script|.
   //
   // |fetch_script| will include calls to |domAutomationController.send| and
@@ -869,4 +903,60 @@ IN_PROC_BROWSER_TEST_F(CrossOriginReadBlockingExtensionTest,
                             content::JsReplace(kScriptTemplate, "logo3.png")));
 }
 
+IN_PROC_BROWSER_TEST_F(CrossOriginReadBlockingExtensionTest,
+                       WebViewContentScript) {
+  // Load the test app.
+  const Extension* app = InstallApp();
+  ASSERT_TRUE(app);
+
+  // Launch the test app and grab its WebContents.
+  content::WebContents* app_contents = nullptr;
+  {
+    content::WebContentsAddedObserver new_contents_observer;
+    OpenApplication(AppLaunchParams(
+        browser()->profile(), app, LAUNCH_CONTAINER_NONE,
+        WindowOpenDisposition::NEW_WINDOW, extensions::SOURCE_TEST));
+    app_contents = new_contents_observer.GetWebContents();
+  }
+  ASSERT_TRUE(content::WaitForLoadStop(app_contents));
+
+  // Inject a <webview> script and declare desire to inject
+  // cross-origin-fetching content scripts into the guest.
+  const char kWebViewInjectionScriptTemplate[] = R"(
+      document.querySelector('#webview-tag-container').innerHTML =
+          '<webview style="width: 100px; height: 100px;"></webview>';
+      var webview = document.querySelector('webview');
+      webview.addContentScripts([{
+          name: 'rule',
+          matches: ['*://*/*'],
+          js: { code: $1 },
+          run_at: 'document_start'}]);
+  )";
+  GURL cross_site_resource(
+      embedded_test_server()->GetURL("bar.com", "/nosniff.xml"));
+  std::string web_view_injection_script = content::JsReplace(
+      kWebViewInjectionScriptTemplate, CreateFetchScript(cross_site_resource));
+  ASSERT_TRUE(ExecuteScript(app_contents, web_view_injection_script));
+
+  // Navigate <webview>, which should trigger content script execution.
+  GURL guest_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
+  const char kWebViewNavigationScriptTemplate[] = R"(
+      var webview = document.querySelector('webview');
+      webview.src = $1;
+  )";
+  std::string web_view_navigation_script =
+      content::JsReplace(kWebViewNavigationScriptTemplate, guest_url);
+  {
+    content::DOMMessageQueue queue;
+    base::HistogramTester histograms;
+    content::ExecuteScriptAsync(app_contents, web_view_navigation_script);
+    std::string fetch_result = PopString(&queue);
+
+    // Verify that no CORB blocking occurred.
+    EXPECT_EQ("nosniff.xml - body\n", fetch_result);
+    EXPECT_THAT(histograms.GetAllSamples("SiteIsolation.XSD.Browser.Blocked"),
+                testing::IsEmpty());
+  }
+}
+
 }  // namespace extensions

extensions/browser/guest_view/web_view/web_view_guest.cc

@@ -56,6 +56,8 @@
 #include "extensions/browser/guest_view/web_view/web_view_permission_helper.h"
 #include "extensions/browser/guest_view/web_view/web_view_permission_types.h"
 #include "extensions/browser/guest_view/web_view/web_view_renderer_state.h"
+#include "extensions/browser/process_manager.h"
+#include "extensions/browser/url_loader_factory_manager.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/manifest_constants.h"
@@ -820,6 +822,39 @@ WebViewGuest::WebViewGuest(WebContents* owner_web_contents)
 WebViewGuest::~WebViewGuest() {
 }
 
+void WebViewGuest::ReadyToCommitNavigation(
+    content::NavigationHandle* navigation_handle) {
+  // Return early if no declarative content scripts are present.
+  //
+  // Note - more granular checks (e.g. against URL patterns) are desirable for
+  // performance (to avoid creating unnecessary URLLoaderFactory), but not
+  // necessarily for security (because there are anyway no OOPIFs inside the
+  // GuestView process - https://crbug.com/614463).  At the same time, more
+  // granular checks are difficult to achieve, because the UserScript objects
+  // are not retained (i.e. only UserScriptIDs are available) by
+  // WebViewContentScriptManager.
+  WebViewContentScriptManager* script_manager =
+      WebViewContentScriptManager::Get(browser_context());
+  int embedder_process_id =
+      owner_web_contents()->GetMainFrame()->GetProcess()->GetID();
+  std::set<int> script_ids = script_manager->GetContentScriptIDSet(
+      embedder_process_id, view_instance_id());
+  if (script_ids.empty())
+    return;
+
+  // At ReadyToCommitNavigation time there is no need to trigger an explicit
+  // push of URLLoaderFactoryBundle to the renderer - it is sufficient if the
+  // factories are pushed slightly later - during the commit.
+  constexpr bool kPushToRendererNow = false;
+
+  // |request_initiator| in fetches initiated by content scripts should use the
+  // origin of the <webview> embedder.
+  navigation_handle->GetRenderFrameHost()
+      ->MarkInitiatorsAsRequiringSeparateURLLoaderFactory(
+          {owner_web_contents()->GetMainFrame()->GetLastCommittedOrigin()},
+          kPushToRendererNow);
+}
+
 void WebViewGuest::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->IsErrorPage() || !navigation_handle->HasCommitted()) {

extensions/browser/guest_view/web_view/web_view_guest.h

@@ -270,6 +270,8 @@ class WebViewGuest : public guest_view::GuestView<WebViewGuest> {
   void DidStartNavigation(content::NavigationHandle* navigation_handle) final;
   void DidRedirectNavigation(
       content::NavigationHandle* navigation_handle) final;
+  void ReadyToCommitNavigation(
+      content::NavigationHandle* navigation_handle) final;
   void DidFinishNavigation(content::NavigationHandle* navigation_handle) final;
   void DocumentOnLoadCompletedInMainFrame() final;
   void RenderProcessGone(base::TerminationStatus status) final;

extensions/browser/url_loader_factory_manager.cc

@@ -231,7 +231,7 @@ void URLLoaderFactoryManager::ReadyToCommitNavigation(
   if (!initiators_requiring_separate_factory.empty()) {
     // At ReadyToCommitNavigation time there is no need to trigger an explicit
     // push of URLLoaderFactoryBundle to the renderer - it is sufficient if the
-    // factories are pushed during the commit.
+    // factories are pushed slightly later - during the commit.
     constexpr bool kPushToRendererNow = false;
 
     MarkInitiatorsAsRequiringSeparateURLLoaderFactory(