Origin isolation: add mismatch console warnings and fix use counter

This adds console warnings when header-based origin isolation is
requested, but not granted, or when it is not requested, but is imposed.
Along the way, a bug with the use counter code for the Origin-Isolation
header was discovered. Fixing this is best done via similar mechanisms
as adding the warnings, namely saving some state early in the
navigation lifecycle to be reported later.

Bug: 1087562, 1066931
Change-Id: I5dce9f48c64f4dd62dcdcea60be94e6492ab9085
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2220358Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774800}

content/browser/frame_host/navigation_request.cc

@@ -1192,6 +1192,7 @@ NavigationRequest::~NavigationRequest() {
 
   if (IsNavigationStarted()) {
     GetDelegate()->DidFinishNavigation(this);
+    ProcessOriginIsolationEndResult();
     if (IsInMainFrame()) {
       TRACE_EVENT_NESTABLE_ASYNC_END2(
           "navigation", "Navigation StartToCommit",
@@ -1849,6 +1850,77 @@ NavigationRequest::IsOptInIsolationRequested(const GURL& url) {
   return OptInIsolationCheckResult::NONE;
 }
 
+void NavigationRequest::DetermineOriginIsolationEndResult(
+    OptInIsolationCheckResult check_result) {
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  const url::Origin origin = url::Origin::Create(common_params_->url);
+  const IsolationContext& isolation_context =
+      render_frame_host_->GetSiteInstance()->GetIsolationContext();
+  const bool got_isolated =
+      policy->ShouldOriginGetOptInIsolation(isolation_context, origin);
+
+  switch (check_result) {
+    case OptInIsolationCheckResult::NONE:
+      origin_isolation_end_result_ =
+          got_isolated
+              ? OptInOriginIsolationEndResult::kNotRequestedButIsolated
+              : OptInOriginIsolationEndResult::kNotRequestedAndNotIsolated;
+      break;
+    case OptInIsolationCheckResult::ORIGIN_POLICY:
+      origin_isolation_end_result_ =
+          got_isolated ? OptInOriginIsolationEndResult::
+                             kRequestedViaOriginPolicyAndIsolated
+                       : OptInOriginIsolationEndResult::
+                             kRequestedViaOriginPolicyButNotIsolated;
+      break;
+    case OptInIsolationCheckResult::HEADER:
+      origin_isolation_end_result_ =
+          got_isolated
+              ? OptInOriginIsolationEndResult::kRequestedViaHeaderAndIsolated
+              : OptInOriginIsolationEndResult::
+                    kRequestedViaHeaderButNotIsolated;
+      break;
+  }
+}
+
+void NavigationRequest::ProcessOriginIsolationEndResult() {
+  if (!HasCommitted() || IsErrorPage() || IsSameDocument())
+    return;
+
+  if (origin_isolation_end_result_ ==
+          OptInOriginIsolationEndResult::kRequestedViaHeaderAndIsolated ||
+      origin_isolation_end_result_ ==
+          OptInOriginIsolationEndResult::kRequestedViaHeaderButNotIsolated)
+    GetContentClient()->browser()->LogWebFeatureForCurrentPage(
+        render_frame_host_, blink::mojom::WebFeature::kOriginIsolationHeader);
+
+  const url::Origin origin = url::Origin::Create(GetURL());
+
+  if (origin_isolation_end_result_ ==
+          OptInOriginIsolationEndResult::kRequestedViaHeaderButNotIsolated ||
+      origin_isolation_end_result_ ==
+          OptInOriginIsolationEndResult::
+              kRequestedViaOriginPolicyButNotIsolated)
+    render_frame_host_->AddMessageToConsole(
+        blink::mojom::ConsoleMessageLevel::kWarning,
+        base::StringPrintf(
+            "The page requested origin isolation, but could not be isolated "
+            "since the origin '%s' had previously been seen with no "
+            "isolation. Update your headers to uniformly isolate all pages "
+            "on the origin.",
+            origin.Serialize().c_str()));
+
+  if (origin_isolation_end_result_ ==
+      OptInOriginIsolationEndResult::kNotRequestedButIsolated)
+    render_frame_host_->AddMessageToConsole(
+        blink::mojom::ConsoleMessageLevel::kWarning,
+        base::StringPrintf("The page did not request origin isolation, but "
+                           "was isolated anyway because the origin '%s' had "
+                           "previously been isolated. Update your headers to "
+                           "uniformly isolate all pages on the origin.",
+                           origin.Serialize().c_str()));
+}
+
 void NavigationRequest::OnResponseStarted(
     network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
     network::mojom::URLResponseHeadPtr response_head,
@@ -2097,10 +2169,7 @@ void NavigationRequest::OnResponseStarted(
   DCHECK(render_frame_host_ || !response_should_be_rendered_);
 
   if (render_frame_host_) {
-    if (opt_in_isolation == OptInIsolationCheckResult::HEADER) {
-      GetContentClient()->browser()->LogWebFeatureForCurrentPage(
-          render_frame_host_, blink::mojom::WebFeature::kOriginIsolationHeader);
-    }
+    DetermineOriginIsolationEndResult(opt_in_isolation);
 
     // TODO(pmeuleman, ahemery): Only set COOP and COEP values on
     // RenderFrameHost when the navigation commits. In the meantime, keep them

content/browser/frame_host/navigation_request.h

@@ -218,6 +218,21 @@ class CONTENT_EXPORT NavigationRequest
   };
   OptInIsolationCheckResult IsOptInIsolationRequested(const GURL& url);
 
+  // The origin isolation end result is determined early in the lifecycle of a
+  // NavigationRequest, but used late. In particular, we want to trigger use
+  // counters and console warnings once navigation has committed.
+  enum class OptInOriginIsolationEndResult {
+    kNotRequestedAndNotIsolated,
+    kNotRequestedButIsolated,
+    kRequestedViaOriginPolicyButNotIsolated,
+    kRequestedViaOriginPolicyAndIsolated,
+    kRequestedViaHeaderButNotIsolated,
+    kRequestedViaHeaderAndIsolated
+  };
+  void DetermineOriginIsolationEndResult(
+      OptInIsolationCheckResult check_result);
+  void ProcessOriginIsolationEndResult();
+
   // NavigationHandle implementation:
   int64_t GetNavigationId() override;
   const GURL& GetURL() override;
@@ -1327,6 +1342,9 @@ class CONTENT_EXPORT NavigationRequest
   // 'ReadyToCommit' time.
   base::Optional<network::mojom::WebSandboxFlags> sandbox_flags_to_commit_;
 
+  OptInOriginIsolationEndResult origin_isolation_end_result_ =
+      OptInOriginIsolationEndResult::kNotRequestedAndNotIsolated;
+
   base::WeakPtrFactory<NavigationRequest> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(NavigationRequest);

content/browser/isolated_origin_browsertest.cc

@@ -511,7 +511,15 @@ IN_PROC_BROWSER_TEST_F(OriginIsolationOptInOriginPolicyTest,
   // Change OriginPolicy manifest to stop isolating the sub-origin. It should
   // still be isolated, to remain consistent with the other frame.
   SetOriginPolicyManifest(R"({ })");
+
+  WebContentsConsoleObserver console_observer(shell()->web_contents());
+  console_observer.SetPattern(
+      "The page did not request origin isolation, but was isolated anyway*");
+
   NavigateFrameToURL(child_frame_node1, isolated_suborigin_url);
+
+  console_observer.Wait();
+
   EXPECT_NE(root->current_frame_host()->GetSiteInstance(),
             child_frame_node1->current_frame_host()->GetSiteInstance());
 
@@ -553,7 +561,15 @@ IN_PROC_BROWSER_TEST_F(OriginIsolationOptInOriginPolicyTest,
   // Change OriginPolicy manifest to start isolating the sub-origin. It should
   // still be not-isolated, to remain consistent with the other frame.
   SetOriginPolicyManifest(R"({ "ids": ["my-policy"], "isolation": true })");
+
+  WebContentsConsoleObserver console_observer(shell()->web_contents());
+  console_observer.SetPattern(
+      "The page requested origin isolation, but could not be isolated*");
+
   NavigateFrameToURL(child_frame_node1, isolated_suborigin_url);
+
+  console_observer.Wait();
+
   EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),
             child_frame_node1->current_frame_host()->GetSiteInstance());
 

services/network/public/mojom/parsed_headers.mojom

@@ -24,7 +24,7 @@ struct ParsedHeaders {
   CrossOriginOpenerPolicy cross_origin_opener_policy;
 
   // The parsed value of the Origin-Isolation header.
-  bool origin_isolation;
+  bool origin_isolation = false;
 
   // The parsed Accept-CH from response headers.
   //