Ensure show/hide are properly called on interstitial pages if present.

In my CL for sending Show/Hide to all frames on a page (https://codereview.chromium.org/298283003), I used the RWH for each RenderFrameHost in the tree. This missed the fact that an interstitial page will be overlayed and not part of the frame tree in WebContents.

BUG=381439

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

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

chrome/browser/ssl/ssl_browser_tests.cc

@@ -30,6 +30,7 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/security_style.h"
@@ -1750,6 +1751,31 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, InterstitialNotAffectedByContentSettings) {
   ASSERT_TRUE(result);
 }
 
+// Verifies that switching tabs, while showing interstitial page, will not
+// affect the visibility of the interestitial.
+// https://crbug.com/381439
+IN_PROC_BROWSER_TEST_F(SSLUITest, InterstitialNotAffectedByHideShow) {
+  ASSERT_TRUE(https_server_expired_.Start());
+  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_TRUE(tab->GetRenderWidgetHostView()->IsShowing());
+  ui_test_utils::NavigateToURL(
+      browser(), https_server_expired_.GetURL("files/ssl/google.html"));
+  CheckAuthenticationBrokenState(
+      tab, net::CERT_STATUS_DATE_INVALID, AuthState::SHOWING_INTERSTITIAL);
+  EXPECT_TRUE(tab->GetRenderWidgetHostView()->IsShowing());
+
+  AddTabAtIndex(0,
+                https_server_.GetURL("files/ssl/google.html"),
+                content::PAGE_TRANSITION_TYPED);
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+  EXPECT_EQ(tab, browser()->tab_strip_model()->GetWebContentsAt(1));
+  EXPECT_FALSE(tab->GetRenderWidgetHostView()->IsShowing());
+
+  browser()->tab_strip_model()->ActivateTabAt(1, true);
+  EXPECT_TRUE(tab->GetRenderWidgetHostView()->IsShowing());
+}
+
 // TODO(jcampan): more tests to do below.
 
 // Visit a page over https that contains a frame with a redirect.

content/browser/site_per_process_browsertest.cc

@@ -219,10 +219,10 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
   {
     // There should be only one RenderWidgetHost when there are no
     // cross-process iframes.
-    std::set<RenderWidgetHostImpl*> widgets_set =
+    std::set<RenderWidgetHostView*> views_set =
         static_cast<WebContentsImpl*>(shell()->web_contents())
-            ->GetRenderWidgetHostsInTree();
-    EXPECT_EQ(1U, widgets_set.size());
+            ->GetRenderWidgetHostViewsInTree();
+    EXPECT_EQ(1U, views_set.size());
   }
 
   // These must stay in scope with replace_host.
@@ -249,10 +249,10 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
   {
     // There should be now two RenderWidgetHosts, one for each process
     // rendering a frame.
-    std::set<RenderWidgetHostImpl*> widgets_set =
+    std::set<RenderWidgetHostView*> views_set =
         static_cast<WebContentsImpl*>(shell()->web_contents())
-            ->GetRenderWidgetHostsInTree();
-    EXPECT_EQ(2U, widgets_set.size());
+            ->GetRenderWidgetHostViewsInTree();
+    EXPECT_EQ(2U, views_set.size());
   }
 
   // Load another cross-site page into the same iframe.
@@ -279,10 +279,10 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
             child->current_frame_host()->GetProcess());
   EXPECT_NE(rph, child->current_frame_host()->GetProcess());
   {
-    std::set<RenderWidgetHostImpl*> widgets_set =
+    std::set<RenderWidgetHostView*> views_set =
         static_cast<WebContentsImpl*>(shell()->web_contents())
-            ->GetRenderWidgetHostsInTree();
-    EXPECT_EQ(2U, widgets_set.size());
+            ->GetRenderWidgetHostViewsInTree();
+    EXPECT_EQ(2U, views_set.size());
   }
 }
 

content/browser/web_contents/web_contents_impl.cc

@@ -216,9 +216,13 @@ void SendToAllFramesInternal(IPC::Message* message, RenderFrameHost* rfh) {
   rfh->Send(message_copy);
 }
 
-void AddRenderWidgetHostToSet(std::set<RenderWidgetHostImpl*>* set,
-                              RenderFrameHost* rfh) {
-  set->insert(static_cast<RenderFrameHostImpl*>(rfh)->GetRenderWidgetHost());
+void AddRenderWidgetHostViewToSet(std::set<RenderWidgetHostView*>* set,
+                                  RenderFrameHost* rfh) {
+  RenderWidgetHostView* rwhv = static_cast<RenderFrameHostImpl*>(rfh)
+                                   ->frame_tree_node()
+                                   ->render_manager()
+                                   ->GetRenderWidgetHostView();
+  set->insert(rwhv);
 }
 
 }  // namespace
@@ -941,15 +945,14 @@ base::TimeTicks WebContentsImpl::GetLastActiveTime() const {
 void WebContentsImpl::WasShown() {
   controller_.SetActive(true);
 
-  std::set<RenderWidgetHostImpl*> widgets = GetRenderWidgetHostsInTree();
-  for (std::set<RenderWidgetHostImpl*>::iterator iter = widgets.begin();
+  std::set<RenderWidgetHostView*> widgets = GetRenderWidgetHostViewsInTree();
+  for (std::set<RenderWidgetHostView*>::iterator iter = widgets.begin();
        iter != widgets.end();
        iter++) {
-    RenderWidgetHostView* rwhv = (*iter)->GetView();
-    if (rwhv) {
-      rwhv->Show();
+    if (*iter) {
+      (*iter)->Show();
 #if defined(OS_MACOSX)
-      rwhv->SetActive(true);
+      (*iter)->SetActive(true);
 #endif
     }
   }
@@ -979,13 +982,12 @@ void WebContentsImpl::WasHidden() {
     // removes the |GetRenderViewHost()|; then when we actually destroy the
     // window, OnWindowPosChanged() notices and calls WasHidden() (which
     // calls us).
-    std::set<RenderWidgetHostImpl*> widgets = GetRenderWidgetHostsInTree();
-    for (std::set<RenderWidgetHostImpl*>::iterator iter = widgets.begin();
+    std::set<RenderWidgetHostView*> widgets = GetRenderWidgetHostViewsInTree();
+    for (std::set<RenderWidgetHostView*>::iterator iter = widgets.begin();
          iter != widgets.end();
          iter++) {
-      RenderWidgetHostView* rwhv = (*iter)->GetView();
-      if (rwhv)
-        rwhv->Hide();
+      if (*iter)
+        (*iter)->Hide();
     }
   }
 
@@ -1142,9 +1144,15 @@ void WebContentsImpl::RemoveObserver(WebContentsObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
-std::set<RenderWidgetHostImpl*> WebContentsImpl::GetRenderWidgetHostsInTree() {
-  std::set<RenderWidgetHostImpl*> set;
-  ForEachFrame(base::Bind(&AddRenderWidgetHostToSet, base::Unretained(&set)));
+std::set<RenderWidgetHostView*>
+WebContentsImpl::GetRenderWidgetHostViewsInTree() {
+  std::set<RenderWidgetHostView*> set;
+  if (ShowingInterstitialPage()) {
+    set.insert(GetRenderWidgetHostView());
+  } else {
+    ForEachFrame(
+        base::Bind(&AddRenderWidgetHostViewToSet, base::Unretained(&set)));
+  }
   return set;
 }
 

content/browser/web_contents/web_contents_impl.h

@@ -673,8 +673,8 @@ class CONTENT_EXPORT WebContentsImpl
   void RemoveDestructionObserver(WebContentsImpl* web_contents);
 
   // Traverses all the RenderFrameHosts in the FrameTree and creates a set
-  // all the unique RenderWidgetHosts.
-  std::set<RenderWidgetHostImpl*> GetRenderWidgetHostsInTree();
+  // all the unique RenderWidgetHostViews.
+  std::set<RenderWidgetHostView*> GetRenderWidgetHostViewsInTree();
 
   // Callback function when showing JavaScript dialogs.  Takes in a routing ID
   // pair to identify the RenderFrameHost that opened the dialog, because it's