When we switch tabs in chrome, the tab being switched away from gets hidden/shown/hidden.

This occurs in the NativeViewHostAura::NativeViewDetaching code path where we first remove the
clipping window which is the intermediate parent of the web contents view. The clipping window
is hidden which causes the RWHVA::Hide function to get called which initiates the hiding sequence.
Then the web contents view is reparented to the main view which is still visible. Now the RWHVA::Show
function is called which initiates the show sequence. Eventually the main view is hidden, which then
initiates the hide sequence.

Addressed this with the following changes.
1. WebView::AttachWebContents and WebView::DetachWebContents
   now show and hide the webcontents native view. The
   WebContents is shown and hidden as before in
   WebContentsNativeViewAura::OnWindowVisibilityChanged.

2. Removed the WebContentsNativeViewAura::OnWindowParentChanged function.
   This function was present to show and hide the webcontents if the window was visible.
   This should not be needed with the change in #1 above.

3. Added a new file webview_unittest.cc. This contains the unittest WebViewUnitTest.TestWebViewAttachDetachWebContents
   This is run as part of unit_tests.exe.

BUG=412989
R=sky

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

Cr-Commit-Position: refs/heads/master@{#294962}

chrome/chrome_tests_unit.gypi

@@ -2130,10 +2130,14 @@
           'dependencies': [
             '../ui/wm/wm.gyp:wm',
             '../ui/aura/aura.gyp:aura_test_support',
+            '../ui/views/views.gyp:views_test_support',
           ],
           'sources/': [
             ['exclude', '^browser/ui/views/extensions/browser_action_drag_data_unittest.cc'],
           ],
+          'sources': [
+            '../ui/views/controls/webview/webview_unittest.cc',
+          ],
         }],
         ['use_aura==1 and use_ash==0 and use_ozone==0 and OS=="linux"', {
           'dependencies': [

content/browser/web_contents/web_contents_view_aura.cc

@@ -1578,19 +1578,6 @@ int WebContentsViewAura::OnPerformDrop(const ui::DropTargetEvent& event) {
   return ConvertFromWeb(current_drag_op_);
 }
 
-void WebContentsViewAura::OnWindowParentChanged(aura::Window* window,
-                                                aura::Window* parent) {
-  // Ignore any visibility changes in the hierarchy below.
-  if (window != window_.get() && window_->Contains(window))
-    return;
-
-  // On Windows we will get called with a parent of NULL as part of the shut
-  // down process. As such we do only change the visibility when a parent gets
-  // set.
-  if (parent)
-    UpdateWebContentsVisibility(window->IsVisible());
-}
-
 void WebContentsViewAura::OnWindowVisibilityChanged(aura::Window* window,
                                                     bool visible) {
   // Ignore any visibility changes in the hierarchy below.

content/browser/web_contents/web_contents_view_aura.h

@@ -178,8 +178,6 @@ class WebContentsViewAura
   virtual int OnPerformDrop(const ui::DropTargetEvent& event) OVERRIDE;
 
   // Overridden from aura::WindowObserver:
-  virtual void OnWindowParentChanged(aura::Window* window,
-                                     aura::Window* parent) OVERRIDE;
   virtual void OnWindowVisibilityChanged(aura::Window* window,
                                          bool visible) OVERRIDE;
 

ui/views/controls/webview/DEPS

@@ -2,4 +2,6 @@ include_rules = [
   "+content/public",
   "+ui/views",
   "+ui/web_dialogs",
+  "+content/browser/web_contents/web_contents_impl.h",
+  "+content/test/test_content_browser_client.h",
 ]

ui/views/controls/webview/webview.cc

@@ -13,6 +13,7 @@
 #include "ipc/ipc_message.h"
 #include "ui/accessibility/ax_enums.h"
 #include "ui/accessibility/ax_view_state.h"
+#include "ui/aura/window.h"
 #include "ui/base/ui_base_switches_util.h"
 #include "ui/events/event.h"
 #include "ui/views/accessibility/native_view_accessibility.h"
@@ -301,6 +302,12 @@ void WebView::AttachWebContents() {
   OnBoundsChanged(bounds());
   if (holder_->native_view() == view_to_attach)
     return;
+
+  // Fullscreen widgets are not parented by a WebContentsView. Their visibility
+  // is controlled by content i.e. (RenderWidgetHost)
+  if (!is_embedding_fullscreen_widget_)
+    view_to_attach->Show();
+
   holder_->Attach(view_to_attach);
 
   // The view will not be focused automatically when it is attached, so we need
@@ -320,6 +327,11 @@ void WebView::AttachWebContents() {
 
 void WebView::DetachWebContents() {
   if (web_contents()) {
+    // Fullscreen widgets are not parented by a WebContentsView. Their
+    // visibility is controlled by content i.e. (RenderWidgetHost).
+    if (!is_embedding_fullscreen_widget_)
+      web_contents()->GetNativeView()->Hide();
+
     holder_->Detach();
 #if defined(OS_WIN)
     if (!is_embedding_fullscreen_widget_)

ui/views/controls/webview/webview.gyp

@@ -12,6 +12,7 @@
       'target_name': 'webview',
       'type': '<(component)',
       'dependencies': [
+        '../../../aura/aura.gyp:aura',
         '../../../../base/base.gyp:base',
         '../../../../base/base.gyp:base_i18n',
         '../../../../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',

ui/views/controls/webview/webview_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/views/controls/webview/webview.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_browser_thread.h"
+#include "content/public/test/web_contents_tester.h"
+#include "content/test/test_content_browser_client.h"
+#include "ui/aura/window.h"
+#include "ui/views/test/test_views_delegate.h"
+#include "ui/views/test/widget_test.h"
+
+namespace {
+
+// Provides functionality to create a test WebContents.
+class WebViewTestViewsDelegate : public views::TestViewsDelegate {
+ public:
+  WebViewTestViewsDelegate() {}
+  virtual ~WebViewTestViewsDelegate() {}
+
+  // Overriden from TestViewsDelegate.
+  virtual content::WebContents* CreateWebContents(
+      content::BrowserContext* browser_context,
+      content::SiteInstance* site_instance) OVERRIDE {
+    return content::WebContentsTester::CreateTestWebContents(browser_context,
+                                                             site_instance);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(WebViewTestViewsDelegate);
+};
+
+// Provides functionality to test a WebView.
+class WebViewUnitTest : public views::test::WidgetTest {
+ public:
+  WebViewUnitTest()
+      : ui_thread_(content::BrowserThread::UI, base::MessageLoop::current()) {}
+
+  virtual ~WebViewUnitTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    // The ViewsDelegate is deleted when the ViewsTestBase class is torn down.
+    WidgetTest::set_views_delegate(new WebViewTestViewsDelegate);
+    WidgetTest::SetUp();
+    // Set the test content browser client to avoid pulling in needless
+    // dependencies from content.
+    SetBrowserClientForTesting(&test_browser_client_);
+  }
+
+ protected:
+  content::BrowserContext* browser_context() { return &browser_context_; }
+
+ private:
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserContext browser_context_;
+  scoped_ptr<WebViewTestViewsDelegate> views_delegate_;
+  content::TestContentBrowserClient test_browser_client_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebViewUnitTest);
+};
+
+// Provides functionaity to observe events on a WebContents like WasShown/
+// WasHidden/WebContentsDestroyed.
+class WebViewTestWebContentsObserver : public content::WebContentsObserver {
+ public:
+  WebViewTestWebContentsObserver(content::WebContents* web_contents)
+      : web_contents_(static_cast<content::WebContentsImpl*>(web_contents)),
+        was_shown_(false),
+        shown_count_(0),
+        hidden_count_(0) {
+    content::WebContentsObserver::Observe(web_contents);
+  }
+
+  virtual ~WebViewTestWebContentsObserver() {
+    if (web_contents_)
+      content::WebContentsObserver::Observe(NULL);
+  }
+
+  virtual void WebContentsDestroyed() OVERRIDE {
+    DCHECK(web_contents_);
+    content::WebContentsObserver::Observe(NULL);
+    web_contents_ = NULL;
+  }
+
+  virtual void WasShown() OVERRIDE {
+    was_shown_ = true;
+    ++shown_count_;
+  }
+
+  virtual void WasHidden() OVERRIDE {
+    was_shown_ = false;
+    ++hidden_count_;
+  }
+
+  bool was_shown() const { return was_shown_; }
+
+  int shown_count() const { return shown_count_; }
+
+  int hidden_count() const { return hidden_count_; }
+
+ private:
+  content::WebContentsImpl* web_contents_;
+  bool was_shown_;
+  int32 shown_count_;
+  int32 hidden_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebViewTestWebContentsObserver);
+};
+
+// Tests that attaching and detaching a WebContents to a WebView makes the
+// WebContents visible and hidden respectively.
+TEST_F(WebViewUnitTest, TestWebViewAttachDetachWebContents) {
+  // Create a top level widget and a webview as its content.
+  views::Widget* widget = CreateTopLevelFramelessPlatformWidget();
+  widget->SetBounds(gfx::Rect(0, 10, 100, 100));
+  views::WebView* webview = new views::WebView(browser_context());
+  widget->SetContentsView(webview);
+  widget->Show();
+
+  // Case 1: Create a new WebContents and set it in the webview via
+  // SetWebContents. This should make the WebContents visible.
+  content::WebContents::CreateParams params(browser_context());
+  scoped_ptr<content::WebContents> web_contents1(
+      content::WebContents::Create(params));
+  WebViewTestWebContentsObserver observer1(web_contents1.get());
+  EXPECT_FALSE(observer1.was_shown());
+
+  webview->SetWebContents(web_contents1.get());
+  EXPECT_TRUE(observer1.was_shown());
+  EXPECT_TRUE(web_contents1->GetNativeView()->IsVisible());
+  EXPECT_EQ(observer1.shown_count(), 1);
+  EXPECT_EQ(observer1.hidden_count(), 0);
+
+  // Case 2: Create another WebContents and replace the current WebContents
+  // via SetWebContents(). This should hide the current WebContents and show
+  // the new one.
+  content::WebContents::CreateParams params2(browser_context());
+  scoped_ptr<content::WebContents> web_contents2(
+      content::WebContents::Create(params2));
+  WebViewTestWebContentsObserver observer2(web_contents2.get());
+  EXPECT_FALSE(observer2.was_shown());
+
+  // Setting the new WebContents should hide the existing one.
+  webview->SetWebContents(web_contents2.get());
+  EXPECT_FALSE(observer1.was_shown());
+  EXPECT_TRUE(observer2.was_shown());
+
+  // WebContents1 should not get stray show calls when WebContents2 is set.
+  EXPECT_EQ(observer1.shown_count(), 1);
+  EXPECT_EQ(observer1.hidden_count(), 1);
+  EXPECT_EQ(observer2.shown_count(), 1);
+  EXPECT_EQ(observer2.hidden_count(), 0);
+
+  widget->Close();
+  RunPendingMessages();
+}
+
+}  // namespace