Ensure that <object>/<embed> navigation bypasses Service Workers.

Step 13 of https://w3c.github.io/ServiceWorker/#on-fetch-request-algorithm
should exclude `embed` and `object` requests from Service Workers. Our
implementation handles this correctly for the initial request, but failed
to bypass the Service Worker for subsequent navigations. This patch adds
a destination check to
`ServiceWorkerMainResourceLoaderInterceptor::ShouldCreateForNavigation`,
and ensures that the `destination` for a given request is set early enough
in the lifecycle to ensure that the check succeeds.

See also https://github.com/whatwg/fetch/pull/948.

Change-Id: I21a1d37da438e1d0f185696f2b3b4058bc3911fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2209456Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773781}

content/browser/frame_host/navigation_request.cc

@@ -660,6 +660,35 @@ void EnterChildTraceEvent(const char* name,
                                     arg_value);
 }
 
+network::mojom::RequestDestination GetDestinationFromFrameTreeNode(
+    FrameTreeNode* frame_tree_node) {
+  if (frame_tree_node->IsMainFrame()) {
+    return frame_tree_node->current_frame_host()
+                   ->GetRenderViewHost()
+                   ->GetDelegate()
+                   ->IsPortal()
+               ? network::mojom::RequestDestination::kIframe
+               : network::mojom::RequestDestination::kDocument;
+  } else {
+    switch (frame_tree_node->frame_owner_element_type()) {
+      case blink::mojom::FrameOwnerElementType::kObject:
+        return network::mojom::RequestDestination::kObject;
+      case blink::mojom::FrameOwnerElementType::kEmbed:
+        return network::mojom::RequestDestination::kEmbed;
+      case blink::mojom::FrameOwnerElementType::kIframe:
+        return network::mojom::RequestDestination::kIframe;
+      case blink::mojom::FrameOwnerElementType::kFrame:
+        return network::mojom::RequestDestination::kFrame;
+      case blink::mojom::FrameOwnerElementType::kPortal:
+      case blink::mojom::FrameOwnerElementType::kNone:
+        NOTREACHED();
+        return network::mojom::RequestDestination::kDocument;
+    }
+    NOTREACHED();
+    return network::mojom::RequestDestination::kDocument;
+  }
+}
+
 }  // namespace
 
 // static
@@ -679,11 +708,13 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated(
   // This is not currently handled here.
   bool is_form_submission = !!post_body;
 
+  network::mojom::RequestDestination destination =
+      GetDestinationFromFrameTreeNode(frame_tree_node);
+
   auto navigation_params = mojom::BeginNavigationParams::New(
       initiator_routing_id.frame_routing_id /* initiator_routing_id */,
       extra_headers, net::LOAD_NORMAL, false /* skip_service_worker */,
-      blink::mojom::RequestContextType::LOCATION,
-      network::mojom::RequestDestination::kDocument,
+      blink::mojom::RequestContextType::LOCATION, destination,
       blink::WebMixedContentContextType::kBlockable, is_form_submission,
       false /* was_initiated_by_link_click */, GURL() /* searchable_form_url */,
       std::string() /* searchable_form_encoding */,
@@ -766,6 +797,9 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateRendererInitiated(
          common_params->navigation_type ==
              mojom::NavigationType::DIFFERENT_DOCUMENT);
 
+  begin_params->request_destination =
+      GetDestinationFromFrameTreeNode(frame_tree_node);
+
   // TODO(clamy): See if the navigation start time should be measured in the
   // renderer and sent to the browser instead of being measured here.
   mojom::CommitNavigationParamsPtr commit_params =
@@ -804,6 +838,7 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateRendererInitiated(
           GURL() /* base_url_override_for_web_bundle */,
           frame_tree_node->pending_frame_policy(),
           std::vector<std::string>() /* force_enabled_origin_trials */);
+
   std::unique_ptr<NavigationRequest> navigation_request(new NavigationRequest(
       frame_tree_node, std::move(common_params), std::move(begin_params),
       std::move(commit_params),

content/browser/loader/navigation_url_loader_impl.cc

@@ -247,41 +247,7 @@ std::unique_ptr<network::ResourceRequest> CreateResourceRequest(
   new_request->has_user_gesture = request_info->common_params->has_user_gesture;
   new_request->enable_load_timing = true;
   new_request->mode = network::mojom::RequestMode::kNavigate;
-
-  FrameTreeNode* frame_tree_node =
-      FrameTreeNode::GloballyFindByID(frame_tree_node_id);
-  if (request_info->is_main_frame) {
-    new_request->destination =
-        frame_tree_node &&
-                WebContentsImpl::FromFrameTreeNode(frame_tree_node)->IsPortal()
-            ? network::mojom::RequestDestination::kIframe
-            : network::mojom::RequestDestination::kDocument;
-  } else {
-    if (frame_tree_node) {
-      switch (frame_tree_node->frame_owner_element_type()) {
-        case blink::mojom::FrameOwnerElementType::kObject:
-          new_request->destination =
-              network::mojom::RequestDestination::kObject;
-          break;
-        case blink::mojom::FrameOwnerElementType::kEmbed:
-          new_request->destination = network::mojom::RequestDestination::kEmbed;
-          break;
-        case blink::mojom::FrameOwnerElementType::kIframe:
-          new_request->destination =
-              network::mojom::RequestDestination::kIframe;
-          break;
-        case blink::mojom::FrameOwnerElementType::kFrame:
-          new_request->destination = network::mojom::RequestDestination::kFrame;
-          break;
-        case blink::mojom::FrameOwnerElementType::kPortal:
-        case blink::mojom::FrameOwnerElementType::kNone:
-          NOTREACHED();
-          break;
-      }
-    } else {
-      new_request->destination = network::mojom::RequestDestination::kDocument;
-    }
-  }
+  new_request->destination = request_info->begin_params->request_destination;
 
   if (ui::PageTransitionIsWebTriggerable(
           request_info->common_params->transition)) {

content/browser/service_worker/service_worker_main_resource_loader_interceptor.cc

@@ -202,8 +202,10 @@ ServiceWorkerMainResourceLoaderInterceptor::CreateForNavigation(
     const NavigationRequestInfo& request_info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  if (!ShouldCreateForNavigation(url))
+  if (!ShouldCreateForNavigation(
+          url, request_info.begin_params->request_destination)) {
     return nullptr;
+  }
 
   return base::WrapUnique(new ServiceWorkerMainResourceLoaderInterceptor(
       std::move(navigation_handle),
@@ -380,7 +382,15 @@ ServiceWorkerMainResourceLoaderInterceptor::
 
 // static
 bool ServiceWorkerMainResourceLoaderInterceptor::ShouldCreateForNavigation(
-    const GURL& url) {
+    const GURL& url,
+    network::mojom::RequestDestination request_destination) {
+  // <embed> and <object> navigations must bypass the service worker, per the
+  // discussion in https://w3c.github.io/ServiceWorker/#implementer-concerns.
+  if (request_destination == network::mojom::RequestDestination::kEmbed ||
+      request_destination == network::mojom::RequestDestination::kObject) {
+    return false;
+  }
+
   // Create the handler even for insecure HTTP since it's used in the
   // case of redirect to HTTPS.
   return url.SchemeIsHTTPOrHTTPS() || OriginCanAccessServiceWorkers(url) ||

content/browser/service_worker/service_worker_main_resource_loader_interceptor.h

@@ -95,7 +95,9 @@ class CONTENT_EXPORT ServiceWorkerMainResourceLoaderInterceptor final
 
   // Returns true if a ServiceWorkerMainResourceLoaderInterceptor should be
   // created for a navigation to |url|.
-  static bool ShouldCreateForNavigation(const GURL& url);
+  static bool ShouldCreateForNavigation(
+      const GURL& url,
+      network::mojom::RequestDestination request_destination);
 
   // Given as a callback to NavigationURLLoaderImpl.
   void RequestHandlerWrapper(

content/browser/service_worker/service_worker_main_resource_loader_interceptor_unittest.cc

@@ -11,25 +11,51 @@ namespace content {
 
 class ServiceWorkerMainResourceLoaderInterceptorTest : public testing::Test {
  public:
-  bool ShouldCreateForNavigation(const GURL& url) {
+  bool ShouldCreateForNavigation(
+      const GURL& url,
+      network::mojom::RequestDestination request_destination) {
     return ServiceWorkerMainResourceLoaderInterceptor::
-        ShouldCreateForNavigation(url);
+        ShouldCreateForNavigation(url, request_destination);
   }
 };
 
 TEST_F(ServiceWorkerMainResourceLoaderInterceptorTest,
        ShouldCreateForNavigation_HTTP) {
-  EXPECT_TRUE(ShouldCreateForNavigation(GURL("http://host/scope/doc")));
+  EXPECT_TRUE(
+      ShouldCreateForNavigation(GURL("http://host/scope/doc"),
+                                network::mojom::RequestDestination::kDocument));
+  EXPECT_FALSE(
+      ShouldCreateForNavigation(GURL("http://host/scope/doc"),
+                                network::mojom::RequestDestination::kEmbed));
+  EXPECT_FALSE(
+      ShouldCreateForNavigation(GURL("http://host/scope/doc"),
+                                network::mojom::RequestDestination::kObject));
 }
 
 TEST_F(ServiceWorkerMainResourceLoaderInterceptorTest,
        ShouldCreateForNavigation_HTTPS) {
-  EXPECT_TRUE(ShouldCreateForNavigation(GURL("https://host/scope/doc")));
+  EXPECT_TRUE(
+      ShouldCreateForNavigation(GURL("https://host/scope/doc"),
+                                network::mojom::RequestDestination::kDocument));
+  EXPECT_FALSE(
+      ShouldCreateForNavigation(GURL("https://host/scope/doc"),
+                                network::mojom::RequestDestination::kEmbed));
+  EXPECT_FALSE(
+      ShouldCreateForNavigation(GURL("https://host/scope/doc"),
+                                network::mojom::RequestDestination::kObject));
 }
 
 TEST_F(ServiceWorkerMainResourceLoaderInterceptorTest,
        ShouldCreateForNavigation_FTP) {
-  EXPECT_FALSE(ShouldCreateForNavigation(GURL("ftp://host/scope/doc")));
+  EXPECT_FALSE(
+      ShouldCreateForNavigation(GURL("ftp://host/scope/doc"),
+                                network::mojom::RequestDestination::kDocument));
+  EXPECT_FALSE(
+      ShouldCreateForNavigation(GURL("ftp://host/scope/doc"),
+                                network::mojom::RequestDestination::kEmbed));
+  EXPECT_FALSE(
+      ShouldCreateForNavigation(GURL("ftp://host/scope/doc"),
+                                network::mojom::RequestDestination::kObject));
 }
 
 TEST_F(ServiceWorkerMainResourceLoaderInterceptorTest,
@@ -38,8 +64,16 @@ TEST_F(ServiceWorkerMainResourceLoaderInterceptorTest,
 #if defined(OS_CHROMEOS)
   expected_handler_created = true;
 #endif  // OS_CHROMEOS
-  EXPECT_EQ(expected_handler_created,
-            ShouldCreateForNavigation(GURL("externalfile:drive/doc")));
+  EXPECT_EQ(
+      expected_handler_created,
+      ShouldCreateForNavigation(GURL("externalfile:drive/doc"),
+                                network::mojom::RequestDestination::kDocument));
+  EXPECT_FALSE(
+      ShouldCreateForNavigation(GURL("externalfile:drive/doc"),
+                                network::mojom::RequestDestination::kEmbed));
+  EXPECT_FALSE(
+      ShouldCreateForNavigation(GURL("externalfile:drive/doc"),
+                                network::mojom::RequestDestination::kObject));
 }
 
 }  // namespace content

third_party/blink/web_tests/external/wpt/service-workers/service-worker/embed-and-object-are-not-intercepted.https.html

@@ -75,4 +75,30 @@ promise_test(t => {
         });
   }, 'requests for OBJECT elements of an image should not be intercepted by service workers');
 
+promise_test(t => {
+    let frame;
+    return with_iframe('resources/object-navigation-is-not-intercepted-iframe.html')
+      .then(f => {
+          frame = f;
+          t.add_cleanup(() => { frame.remove(); });
+          return frame.contentWindow.test_promise;
+        })
+      .then(result => {
+          assert_equals(result, 'request for embedded content was not intercepted');
+        });
+  }, 'post-load navigation of OBJECT elements should not be intercepted by service workers');
+
+
+promise_test(t => {
+    let frame;
+    return with_iframe('resources/embed-navigation-is-not-intercepted-iframe.html')
+      .then(f => {
+          frame = f;
+          t.add_cleanup(() => { frame.remove(); });
+          return frame.contentWindow.test_promise;
+        })
+      .then(result => {
+          assert_equals(result, 'request for embedded content was not intercepted');
+        });
+  }, 'post-load navigation of EMBED elements should not be intercepted by service workers');
 </script>

third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/embed-navigation-is-not-intercepted-iframe.html

+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>iframe for embed-and-object-are-not-intercepted test</title>
+<body>
+<script>
+// The EMBED element will call this with the result about whether the EMBED
+// request was intercepted by the service worker.
+var report_result;
+
+// Our parent (the root frame of the test) will examine this to get the result.
+var test_promise = new Promise(resolve => {
+    report_result = resolve;
+  });
+
+let el = document.createElement('embed');
+el.src = "/common/blank.html";
+el.addEventListener('load', _ => {
+  window[0].location = "/service-workers/service-worker/resources/embedded-content-from-server.html";
+}, { once: true });
+document.body.appendChild(el);
+</script>
+
+</body>

third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/object-navigation-is-not-intercepted-iframe.html

+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>iframe for embed-and-object-are-not-intercepted test</title>
+<body>
+<script>
+// The OBJECT element will call this with the result about whether the OBJECT
+// request was intercepted by the service worker.
+var report_result;
+
+// Our parent (the root frame of the test) will examine this to get the result.
+var test_promise = new Promise(resolve => {
+    report_result = resolve;
+  });
+
+let el = document.createElement('object');
+el.data = "/common/blank.html";
+el.addEventListener('load', _ => {
+  window[0].location = "/service-workers/service-worker/resources/embedded-content-from-server.html";
+}, { once: true });
+document.body.appendChild(el);
+</script>
+
+</body>
+