Ensure fullscreen mode is exited for same-site navigations.

Resolves a fullscreen-within-tab bug where navigation using the Back button doesn't exit fullscreen.  This would result in the browser (WebContentsDelegate) erroneously showing a page with the "fullscreen in your tab" UI layout.

Furthermore, this change will cause the previous renderer to be notified that it has been force-exited from fullscreen.  Should the user revisit using the Back/Forward/History actions, the renderer will therefore be in the correct non-fullscreen state.

Added a new unit test to web_contents_impl_unittest.cc to make sure this problem doesn't regress.

BUG=356951,347232
TEST=Repro steps in bug 356951 result in fullscreen exit.

Review URL: https://codereview.chromium.org/218093002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@261087 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 89 insertions, 21 deletions

diff --git a/content/browser/frame_host/navigator_delegate.h b/content/browser/frame_host/navigator_delegate.h
index ab70aac..e92dbee 100644
--- a/content/browser/frame_host/navigator_delegate.h
+++ b/content/browser/frame_host/navigator_delegate.h
@@ -59,6 +59,11 @@ class CONTENT_EXPORT NavigatorDelegate {
       const GURL& url,
       PageTransition transition_type) {}
 
+  // Handles post-navigation tasks in navigation BEFORE the entry has been
+  // committed to the NavigationController.
+  virtual void DidNavigateMainFramePreCommit(
+      const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {}
+
   // Handles post-navigation tasks in navigation AFTER the entry has been
   // committed to the NavigationController. Note that the NavigationEntry is
   // not provided since it may be invalid/changed after being committed. The
diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc
index a983608..7be5f8c 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -396,14 +396,19 @@ void NavigatorImpl::DidNavigate(
   }
 
   if (PageTransitionIsMainFrame(params.transition)) {
-    // When overscroll navigation gesture is enabled, a screenshot of the page
-    // in its current state is taken so that it can be used during the
-    // nav-gesture. It is necessary to take the screenshot here, before calling
-    // RenderFrameHostManager::DidNavigateMainFrame, because that can change
-    // WebContents::GetRenderViewHost to return the new host, instead of the one
-    // that may have just been swapped out.
-    if (delegate_ && delegate_->CanOverscrollContent())
-      controller_->TakeScreenshot();
+    if (delegate_) {
+      // When overscroll navigation gesture is enabled, a screenshot of the page
+      // in its current state is taken so that it can be used during the
+      // nav-gesture. It is necessary to take the screenshot here, before
+      // calling RenderFrameHostManager::DidNavigateMainFrame, because that can
+      // change WebContents::GetRenderViewHost to return the new host, instead
+      // of the one that may have just been swapped out.
+      if (delegate_->CanOverscrollContent())
+        controller_->TakeScreenshot();
+
+      // Run tasks that must execute just before the commit.
+      delegate_->DidNavigateMainFramePreCommit(params);
+    }
 
     if (!use_site_per_process)
       frame_tree->root()->render_manager()->DidNavigateFrame(render_frame_host);
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 56e1ffd..e00498e 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -2237,6 +2237,21 @@ void WebContentsImpl::DidCommitProvisionalLoad(
                                        render_view_host));
 }
 
+void WebContentsImpl::DidNavigateMainFramePreCommit(
+    const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
+  // Ensure fullscreen mode is exited before committing the navigation to a
+  // different page.  The next page will not start out assuming it is in
+  // fullscreen mode.
+  if (params.was_within_same_page) {
+    // No document change?  Then, the renderer shall decide whether to exit
+    // fullscreen.
+    return;
+  }
+  if (IsFullscreenForCurrentTab())
+    GetRenderViewHost()->ExitFullscreen();
+  DCHECK(!IsFullscreenForCurrentTab());
+}
+
 void WebContentsImpl::DidNavigateMainFramePostCommit(
     const LoadCommittedDetails& details,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
@@ -3052,19 +3067,8 @@ void WebContentsImpl::Close(RenderViewHost* rvh) {
 }
 
 void WebContentsImpl::SwappedOut(RenderFrameHost* rfh) {
-  // TODO(creis): Handle subframes that go fullscreen.
-  if (rfh->GetRenderViewHost() == GetRenderViewHost()) {
-    // Exit fullscreen mode before the current RVH is swapped out.  For numerous
-    // cases, there is no guarantee the renderer would/could initiate an exit.
-    // Example: http://crbug.com/347232
-    if (IsFullscreenForCurrentTab()) {
-      rfh->GetRenderViewHost()->ExitFullscreen();
-      DCHECK(!IsFullscreenForCurrentTab());
-    }
-
-    if (delegate_)
-      delegate_->SwappedOut(this);
-  }
+  if (delegate_ && rfh->GetRenderViewHost() == GetRenderViewHost())
+    delegate_->SwappedOut(this);
 }
 
 void WebContentsImpl::RequestMove(const gfx::Rect& new_bounds) {
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index aa439a1..f7a6159 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -455,6 +455,8 @@ class CONTENT_EXPORT WebContentsImpl
       bool is_main_frame,
       const GURL& url,
       PageTransition transition_type) OVERRIDE;
+  virtual void DidNavigateMainFramePreCommit(
+      const FrameHostMsg_DidCommitProvisionalLoad_Params& params) OVERRIDE;
   virtual void DidNavigateMainFramePostCommit(
       const LoadCommittedDetails& details,
       const FrameHostMsg_DidCommitProvisionalLoad_Params& params) OVERRIDE;
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index a6a66bb..051e011 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -1299,6 +1299,58 @@ TEST_F(WebContentsImplTest, NavigationExitsFullscreen) {
   contents()->SetDelegate(NULL);
 }
 
+// Tests that fullscreen is exited throughout the object hierarchy when
+// instructing NavigationController to GoBack() or GoForward().
+TEST_F(WebContentsImplTest, HistoryNavigationExitsFullscreen) {
+  FakeFullscreenDelegate fake_delegate;
+  contents()->SetDelegate(&fake_delegate);
+  TestRenderViewHost* const orig_rvh = test_rvh();
+
+  // Navigate to a site.
+  const GURL url("http://www.google.com");
+  controller().LoadURL(url, Referrer(), PAGE_TRANSITION_TYPED, std::string());
+  contents()->TestDidNavigate(orig_rvh, 1, url, PAGE_TRANSITION_TYPED);
+  EXPECT_EQ(orig_rvh, contents()->GetRenderViewHost());
+
+  // Now, navigate to another page on the same site.
+  const GURL url2("http://www.google.com/search?q=kittens");
+  controller().LoadURL(url2, Referrer(), PAGE_TRANSITION_TYPED, std::string());
+  EXPECT_FALSE(contents()->cross_navigation_pending());
+  contents()->TestDidNavigate(orig_rvh, 2, url2, PAGE_TRANSITION_TYPED);
+  EXPECT_EQ(orig_rvh, contents()->GetRenderViewHost());
+
+  // Sanity-check: Confirm we're not starting out in fullscreen mode.
+  EXPECT_FALSE(orig_rvh->IsFullscreen());
+  EXPECT_FALSE(contents()->IsFullscreenForCurrentTab());
+  EXPECT_FALSE(fake_delegate.IsFullscreenForTabOrPending(contents()));
+
+  for (int i = 0; i < 2; ++i) {
+    // Toggle fullscreen mode on (as if initiated via IPC from renderer).
+    orig_rvh->OnMessageReceived(
+        ViewHostMsg_ToggleFullscreen(orig_rvh->GetRoutingID(), true));
+    EXPECT_TRUE(orig_rvh->IsFullscreen());
+    EXPECT_TRUE(contents()->IsFullscreenForCurrentTab());
+    EXPECT_TRUE(fake_delegate.IsFullscreenForTabOrPending(contents()));
+
+    // Navigate backward (or forward).
+    if (i == 0)
+      controller().GoBack();
+    else
+      controller().GoForward();
+    EXPECT_FALSE(contents()->cross_navigation_pending());
+    EXPECT_EQ(orig_rvh, contents()->GetRenderViewHost());
+    contents()->TestDidNavigate(
+        orig_rvh, i + 1, url, PAGE_TRANSITION_FORWARD_BACK);
+
+    // Confirm fullscreen has exited.
+    EXPECT_FALSE(orig_rvh->IsFullscreen());
+    EXPECT_FALSE(contents()->IsFullscreenForCurrentTab());
+    EXPECT_FALSE(fake_delegate.IsFullscreenForTabOrPending(contents()));
+  }
+
+  contents()->SetDelegate(NULL);
+}
+
 // Tests that fullscreen is exited throughout the object hierarchy on a renderer
 // crash.
 TEST_F(WebContentsImplTest, CrashExitsFullscreen) {