History intervention: Do not intervene for pdfs and other inner webcontents

This is a temporary fix so that the intervention does not break back
button for pdfs. The root cause is that mouse click on pdf is not considered
a user gesture (crbug.com/934637). Once this is fixed, then this CL's changes
can be removed.

Bug: 965434
Change-Id: I15d8392e98d1578a1c30f242c847fbcf8e4bd8ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623137
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662831}

chrome/browser/chrome_navigation_browsertest.cc

@@ -1371,6 +1371,35 @@ IN_PROC_BROWSER_TEST_F(HistoryManipulationInterventionBrowserTest,
   ASSERT_EQ(GURL("about:blank"), main_contents->GetLastCommittedURL());
 }
 
+// Tests that a main frame hosting pdf does not get skipped because of history
+// manipulation intervention (crbug.com/965434).
+IN_PROC_BROWSER_TEST_F(HistoryManipulationInterventionBrowserTest,
+                       PDFDoNotSkipOnBackForward) {
+  GURL pdf_url(embedded_test_server()->GetURL("/pdf/test.pdf"));
+  ui_test_utils::NavigateToURL(browser(), pdf_url);
+
+  GURL url(embedded_test_server()->GetURL("/title2.html"));
+
+  // Navigate to a new document from the renderer without a user gesture.
+  content::WebContents* main_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::TestNavigationObserver observer(main_contents);
+  EXPECT_TRUE(ExecuteScriptWithoutUserGesture(
+      main_contents, "location = '" + url.spec() + "';"));
+  observer.Wait();
+  EXPECT_EQ(url, main_contents->GetLastCommittedURL());
+
+  // Even though pdf_url initiated a navigation without a user gesture, it will
+  // not be skipped since it is a pdf.
+  // Going back should be allowed and should navigate to pdf_url.
+  EXPECT_TRUE(chrome::IsCommandEnabled(browser(), IDC_BACK));
+
+  ASSERT_TRUE(chrome::CanGoBack(browser()));
+  chrome::GoBack(browser(), WindowOpenDisposition::CURRENT_TAB);
+  content::WaitForLoadStop(main_contents);
+  ASSERT_EQ(pdf_url, main_contents->GetLastCommittedURL());
+}
+
 // This test class turns on the mode where sites where the user enters a
 // password are dynamically added to the list of sites requiring a dedicated
 // process.  It also disables strict site isolation so that the effects of

content/browser/frame_host/navigation_controller_delegate.h

@@ -56,6 +56,10 @@ class NavigationControllerDelegate {
   virtual void ActivateAndShowRepostFormWarningDialog() = 0;
   virtual bool HasAccessedInitialDocument() = 0;
 
+  // TODO(crbug.com/934637): Remove when pdf and any inner web contents user
+  // gesture is properly propagated.
+  virtual bool HadInnerWebContents() = 0;
+
   // This method is needed, since we are no longer guaranteed that the
   // embedder for NavigationController will be a WebContents object.
   virtual WebContents* GetWebContents() = 0;

content/browser/frame_host/navigation_controller_impl.cc

@@ -3410,8 +3410,12 @@ void NavigationControllerImpl::SetShouldSkipOnBackForwardUIIfNeeded(
   // Note that for a subframe, previous_document_was_activated is true if the
   // gesture happened in any subframe (propagated to main frame) or in the main
   // frame itself.
+  // TODO(crbug.com/934637): Remove the check for HadInnerWebContents() when
+  // pdf and any inner web contents user gesture is properly propagated. This is
+  // a temporary fix for history intervention to be disabled for pdfs
+  // (crbug.com/965434).
   if (replace_entry || previous_document_was_activated ||
-      !is_renderer_initiated) {
+      !is_renderer_initiated || delegate_->HadInnerWebContents()) {
     if (last_committed_entry_index_ != -1) {
       UMA_HISTOGRAM_BOOLEAN(
           "Navigation.BackForward.SetShouldSkipOnBackForwardUI", false);

content/browser/web_contents/web_contents_impl.cc

@@ -592,6 +592,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
 #endif  // !defined(OS_ANDROID)
       is_overlay_content_(false),
       showing_context_menu_(false),
+      had_inner_webcontents_(false),
       loading_weak_factory_(this),
       weak_factory_(this) {
   frame_tree_.SetFrameRemoveListener(
@@ -3495,6 +3496,10 @@ void WebContentsImpl::DidProceedOnInterstitial() {
     LoadingStateChanged(true, true, nullptr);
 }
 
+bool WebContentsImpl::HadInnerWebContents() {
+  return had_inner_webcontents_;
+}
+
 void WebContentsImpl::DetachInterstitialPage(bool has_focus) {
   bool interstitial_pausing_throbber =
       ShowingInterstitialPage() && interstitial_page_->pause_throbber();
@@ -4409,6 +4414,9 @@ void WebContentsImpl::DidNavigateMainFramePostCommit(
   if (delegate_)
     delegate_->DidNavigateMainFramePostCommit(this);
   view_->SetOverscrollControllerEnabled(CanOverscrollContent());
+
+  if (!details.is_same_document && GetInnerWebContents().empty())
+    had_inner_webcontents_ = false;
 }
 
 void WebContentsImpl::DidNavigateAnyFramePostCommit(
@@ -5606,6 +5614,7 @@ void WebContentsImpl::FocusOuterAttachmentFrameChain() {
 }
 
 void WebContentsImpl::InnerWebContentsCreated(WebContents* inner_web_contents) {
+  had_inner_webcontents_ = true;
   for (auto& observer : observers_)
     observer.InnerWebContentsCreated(inner_web_contents);
 }

content/browser/web_contents/web_contents_impl.h

@@ -885,6 +885,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   // Unpause the throbber if it was paused.
   void DidProceedOnInterstitial() override;
 
+  bool HadInnerWebContents() override;
+
   // Forces overscroll to be disabled (used by touch emulation).
   void SetForceDisableOverscrollContent(bool force_disable);
 
@@ -1858,6 +1860,11 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
       base::BindRepeating(&WebContentsImpl::OnDarkModeChanged,
                           base::Unretained(this))};
 
+  // TODO(crbug.com/934637): Remove this field when pdf/any inner web contents
+  // user gesture is properly propagated. This is a temporary fix for history
+  // intervention to be disabled for pdfs (crbug.com/965434).
+  bool had_inner_webcontents_;
+
   base::WeakPtrFactory<WebContentsImpl> loading_weak_factory_;
   base::WeakPtrFactory<WebContentsImpl> weak_factory_;