Implement NavigationThrottle::BLOCK_REQUEST_AND_COLLAPSE.

content/browser/frame_host/navigation_handle_impl.cc

Index: content/browser/frame_host/navigation_handle_impl.cc
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc
index 46866467c1b8cdbff04a4724cd1a56569396b7e6..bde20dbda9394cd0a29c40d6028a5777656d15c1 100644
--- a/content/browser/frame_host/navigation_handle_impl.cc
+++ b/content/browser/frame_host/navigation_handle_impl.cc
@@ -355,7 +355,15 @@ void NavigationHandleImpl::CancelDeferredNavigation(
          state_ == DEFERRING_REDIRECT ||
          state_ == DEFERRING_RESPONSE);
   DCHECK(result == NavigationThrottle::CANCEL_AND_IGNORE ||
-         result == NavigationThrottle::CANCEL);
+         result == NavigationThrottle::CANCEL ||
+         result == NavigationThrottle::BLOCK_REQUEST_AND_COLLAPSE);
+  DCHECK(result != NavigationThrottle::BLOCK_REQUEST_AND_COLLAPSE ||
+         state_ == DEFERRING_START ||
+         (state_ == DEFERRING_REDIRECT && IsBrowserSideNavigationEnabled()));
+
+  if (result == NavigationThrottle::BLOCK_REQUEST_AND_COLLAPSE)
+    frame_tree_node_->SetCollapsed(true);
+
   state_ = CANCELING;
   RunCompleteCallback(result);
 }
@@ -680,6 +688,13 @@ void NavigationHandleImpl::DidCommitNavigation(
     state_ = DID_COMMIT_ERROR_PAGE;
   } else {
     state_ = DID_COMMIT;
+
+    // Getting this far means that the navigation was not blocked, and neither
+    // is this the error page navigation following a blocked navigation. Ensure
+    // the frame owner element is no longer collapsed as a result of a prior
+    // navigation having been blocked with BLOCK_REQUEST_AND_COLLAPSE.
+    if (!frame_tree_node()->IsMainFrame())
+      frame_tree_node()->SetCollapsed(false);

[clamy, 2017/04/24 13:15:18]

What about SameDocument navigations? We should check that we don't get them if
the frame is collapsed, and I don't think they should uncollapse the frame
right?

[engedy, 2017/04/28 13:43:59]

If the frame is currently collapsed, it will be showing an error page with
|kUnreachableWebDataURL|, so I don't think that a same page navigation is
technically possible here unless we need to worry about weird race conditions.
(Do we need to?)

   }
 
   if (url_.SchemeIs(url::kDataScheme) && IsInMainFrame() &&
@@ -722,6 +737,11 @@ NavigationHandleImpl::CheckWillStartRequest() {
         state_ = CANCELING;
         return result;
 
+      case NavigationThrottle::BLOCK_REQUEST_AND_COLLAPSE:

[clamy, 2017/04/24 13:15:18]

nit: I'd prefer if we were consistent when using fall through with what we're
doing below: either fall through in both cases or not fall through in both
cases.

[engedy, 2017/04/28 13:43:58]

Done.

+        frame_tree_node_->SetCollapsed(true);
+        state_ = CANCELING;
+        return result;
+
       case NavigationThrottle::DEFER:
         state_ = DEFERRING_START;
         next_index_ = i + 1;
@@ -748,9 +768,13 @@ NavigationHandleImpl::CheckWillRedirectRequest() {
       case NavigationThrottle::PROCEED:
         continue;
 
+      case NavigationThrottle::BLOCK_REQUEST_AND_COLLAPSE:
+        frame_tree_node_->SetCollapsed(true);
+      // Fall through.

[nasko, 2017/04/20 15:53:40]

nit: Indent this to match the call above.

[engedy, 2017/04/28 13:43:59]

I moved it after the call, clang-format does not let me indent that.

       case NavigationThrottle::BLOCK_REQUEST:
         CHECK(IsBrowserSideNavigationEnabled())
-            << "BLOCK_REQUEST must not be used on redirect without PlzNavigate";
+            << "BLOCK_REQUEST and BLOCK_REQUEST_AND_COLLAPSE must not be used "
+               "on redirect without PlzNavigate";
       case NavigationThrottle::CANCEL:
       case NavigationThrottle::CANCEL_AND_IGNORE:
         state_ = CANCELING;
@@ -799,6 +823,7 @@ NavigationHandleImpl::CheckWillProcessResponse() {
         return result;
 
       case NavigationThrottle::BLOCK_REQUEST:
+      case NavigationThrottle::BLOCK_REQUEST_AND_COLLAPSE:
         NOTREACHED();
     }
   }