Fix plugin window sticking around when parent became invisible. Added test for all different scenarios.

Note I grabbed the plugin hwnd and manually checked it instead of looking for callbacks from the plugin's SetWindow since the latter isn't called if the visibility changes.

BUG=1842096
TEST=regression test included, test http://chromedashboard per bug
Review URL: http://codereview.chromium.org/115169

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

8 files changed, 66 insertions, 65 deletions

diff --git a/webkit/data/plugins/hidden_plugin.html b/webkit/data/plugins/hidden_plugin.html
deleted file mode 100644
index db6b575..0000000
--- a/webkit/data/plugins/hidden_plugin.html
+++ /dev/null
@@ -1,2 +0,0 @@
-<div id='result'>FAIL</div>

-<iframe src='hidden_plugin_iframe.html'></iframe>
\ No newline at end of file
diff --git a/webkit/data/plugins/hidden_plugin_iframe.html b/webkit/data/plugins/hidden_plugin_iframe.html
deleted file mode 100644
index ad0a994..0000000
--- a/webkit/data/plugins/hidden_plugin_iframe.html
+++ /dev/null
@@ -1,14 +0,0 @@
-<script>

-var gotVisible = false;

-function windowHidden() {

-  if (!gotVisible)

-    parent.document.getElementById('result').innerHTML = 'DONE';

-}

-

-function windowVisible() {

-  gotVisible = true;

-  parent.document.getElementById('result').innerHTML = 'FAIL';

-}

-

-</script>

-<embed style="visibility:hidden" id='1' type='application/vnd.npapi-test' src='foo' name='hidden_plugin'>
\ No newline at end of file
diff --git a/webkit/data/plugins/plugin_visibility.html b/webkit/data/plugins/plugin_visibility.html
new file mode 100644
index 0000000..bda8a9d
--- /dev/null
+++ b/webkit/data/plugins/plugin_visibility.html
@@ -0,0 +1,12 @@
+<script>

+function showFrame(show) {

+  frame = document.getElementById("the_iframe");

+  frame.style.visibility = show ? "" : "hidden";

+}

+

+function showPlugin(show) {

+  iframe = document.getElementById("the_iframe").contentDocument;

+  iframe.getElementById("1").style.visibility = show ? "" : "hidden";

+}

+</script>

+<iframe id='the_iframe' src='plugin_visibility_iframe.html'></iframe>
\ No newline at end of file
diff --git a/webkit/data/plugins/plugin_visibility_iframe.html b/webkit/data/plugins/plugin_visibility_iframe.html
new file mode 100644
index 0000000..447a8fe
--- /dev/null
+++ b/webkit/data/plugins/plugin_visibility_iframe.html
@@ -0,0 +1 @@
+<embed style="visibility:hidden" id='1' type='application/vnd.npapi-test' src='foo' name='hidden_plugin'>
\ No newline at end of file
diff --git a/webkit/glue/plugins/test/plugin_windowed_test.cc b/webkit/glue/plugins/test/plugin_windowed_test.cc
index 20215ef..4001ba2 100644
--- a/webkit/glue/plugins/test/plugin_windowed_test.cc
+++ b/webkit/glue/plugins/test/plugin_windowed_test.cc
@@ -18,20 +18,6 @@ NPError WindowedPluginTest::SetWindow(NPWindow* pNPWindow) {
     return NPERR_INVALID_PARAM;
   }
 
-  if (test_name() == "hidden_plugin") {
-    NPIdentifier function_id;
-    if (IsWindowVisible(window)) {
-      function_id = HostFunctions()->getstringidentifier("windowVisible");
-    } else {
-      function_id = HostFunctions()->getstringidentifier("windowHidden");
-    }
-
-    NPVariant rv;
-    NPObject *window_obj = NULL;
-    HostFunctions()->getvalue(id(), NPNVWindowNPObject, &window_obj);
-    HostFunctions()->invoke(id(), window_obj, function_id, NULL, 0, &rv);
-  }
-
   return NPERR_NO_ERROR;
 }
 
diff --git a/webkit/glue/webplugin_impl.cc b/webkit/glue/webplugin_impl.cc
index a065e59..0ed9188 100644
--- a/webkit/glue/webplugin_impl.cc
+++ b/webkit/glue/webplugin_impl.cc
@@ -163,26 +163,30 @@ void WebPluginContainer::setFocus() {
 }
 
 void WebPluginContainer::show() {
+  bool old_visible = isVisible();
+  setSelfVisible(true);
   // We don't want to force a geometry update when the plugin widget is
   // already visible as this involves a geometry update which may lead
   // to unnecessary window moves in the plugin process.
-  if (!impl_->visible_) {
-    impl_->show();
-    WebCore::Widget::show();
+  if (old_visible != isVisible()) {
     // This is to force an updategeometry call to the plugin process
     // where the plugin window can be hidden or shown.
     frameRectsChanged();
   }
+
+  WebCore::Widget::show();
 }
 
 void WebPluginContainer::hide() {
-  if (impl_->visible_) {
-    impl_->hide();
-    WebCore::Widget::hide();
+  bool old_visible = isVisible();
+  setSelfVisible(false);
+  if (old_visible != isVisible()) {
     // This is to force an updategeometry call to the plugin process
     // where the plugin window can be hidden or shown.
     frameRectsChanged();
   }
+
+  WebCore::Widget::hide();
 }
 
 void WebPluginContainer::handleEvent(WebCore::Event* event) {
@@ -209,10 +213,9 @@ void WebPluginContainer::setParentVisible(bool visible) {
   if (!isSelfVisible())
     return;  // This widget has explicitely been marked as not visible.
 
-  if (visible)
-    show();
-  else
-    hide();
+  // This is to force an updategeometry call to the plugin process
+  // where the plugin window can be hidden or shown.
+  frameRectsChanged();
 }
 
 // We override this function so that if the plugin is windowed, we can call
@@ -325,7 +328,6 @@ WebPluginImpl::WebPluginImpl(WebCore::HTMLPlugInElement* element,
       element_(element),
       webframe_(webframe),
       delegate_(delegate),
-      visible_(false),
       widget_(NULL),
       plugin_url_(plugin_url),
       load_manually_(load_manually),
@@ -661,7 +663,7 @@ void WebPluginImpl::setFrameRect(const WebCore::IntRect& rect) {
     move.window_rect = webkit_glue::FromIntRect(window_rect);
     move.clip_rect = webkit_glue::FromIntRect(clip_rect);
     move.cutout_rects = cutout_rects;
-    move.visible = visible_;
+    move.visible = widget_->isVisible();
 
     webview->delegate()->DidMove(webview, move);
   }
@@ -750,14 +752,6 @@ void WebPluginImpl::setFocus() {
     delegate_->SetFocus();
 }
 
-void WebPluginImpl::show() {
-  visible_ = true;
-}
-
-void WebPluginImpl::hide() {
-  visible_ = false;
-}
-
 void WebPluginImpl::handleEvent(WebCore::Event* event) {
   if (!windowless_)
     return;
diff --git a/webkit/glue/webplugin_impl.h b/webkit/glue/webplugin_impl.h
index 64f964e..8d14792 100644
--- a/webkit/glue/webplugin_impl.h
+++ b/webkit/glue/webplugin_impl.h
@@ -229,11 +229,6 @@ class WebPluginImpl : public WebPlugin,
   // Notifies the plugin about focus changes.
   void setFocus();
 
-  // Called by WebPluginContainer::show/hide, which overrides Widget show/hide.
-  // This allows us to control the visible state of the plugin window.
-  void show();
-  void hide();
-
   // Handle widget events.
   void handleEvent(WebCore::Event* event);
   void handleMouseEvent(WebCore::MouseEvent* event);
@@ -338,7 +333,6 @@ class WebPluginImpl : public WebPlugin,
   WebFrameImpl* webframe_;
 
   WebPluginDelegate* delegate_;
-  bool visible_;
 
   WebPluginContainer* widget_;
 
diff --git a/webkit/tools/test_shell/plugin_tests.cc b/webkit/tools/test_shell/plugin_tests.cc
index eae50e0..1fa32d4 100644
--- a/webkit/tools/test_shell/plugin_tests.cc
+++ b/webkit/tools/test_shell/plugin_tests.cc
@@ -51,6 +51,16 @@ class PluginTest : public TestShellTest {
     file_util::Delete(plugin_file_path_, true);
   }
 
+  virtual void SetUp() {
+    CopyTestPlugin();
+    TestShellTest::SetUp();
+  }
+
+  virtual void TearDown() {
+    DeleteTestPlugin();
+    TestShellTest::TearDown();
+  }
+
   FilePath plugin_src_;
   FilePath plugin_file_path_;
 };
@@ -103,8 +113,6 @@ TEST_F(PluginTest, Refresh) {
   test_shell_->webView()->GetMainFrame()->ExecuteScript(call_check);
   test_shell_->webView()->GetMainFrame()->GetContentAsPlainText(10000, &text);
   ASSERT_EQ(text, L"DONE");
-
-  DeleteTestPlugin();
 }
 
 #if defined(OS_WIN)
@@ -158,18 +166,40 @@ TEST_F(PluginTest, DeleteFrameDuringEvent) {
 }
 
 #if defined(OS_WIN)
-// Tests that a hidden plugin is not shown.  See http://crbug.com/8927
-TEST_F(PluginTest, HiddenPlugin) {
-  CopyTestPlugin();
+BOOL CALLBACK EnumChildProc(HWND hwnd, LPARAM lparam) {
+  HWND* plugin_hwnd = reinterpret_cast<HWND*>(lparam);
+  if (*plugin_hwnd) {
+    // More than one child window found, unexpected.
+    plugin_hwnd = NULL;
+    return FALSE;
+  }
+  *plugin_hwnd = hwnd;
+  return TRUE;
+}
 
+// Tests that hiding/showing the parent frame hides/shows the plugin.
+TEST_F(PluginTest, PluginVisibilty) {
   FilePath test_html = data_dir_;
   test_html = test_html.AppendASCII("plugins");
-  test_html = test_html.AppendASCII("hidden_plugin.html");
+  test_html = test_html.AppendASCII("plugin_visibility.html");
   test_shell_->LoadURL(test_html.ToWStringHack().c_str());
   test_shell_->WaitTestFinished();
 
-  std::wstring text;
-  test_shell_->webView()->GetMainFrame()->GetContentAsPlainText(10000, &text);
-  ASSERT_EQ(true, StartsWith(text, L"DONE", true));
+  WebFrame* main_frame = test_shell_->webView()->GetMainFrame();
+  HWND frame_hwnd = test_shell_->webViewWnd();
+  HWND plugin_hwnd = NULL;
+  EnumChildWindows(frame_hwnd, EnumChildProc,
+      reinterpret_cast<LPARAM>(&plugin_hwnd));
+  ASSERT_TRUE(plugin_hwnd != NULL);
+  ASSERT_FALSE(IsWindowVisible(plugin_hwnd));
+
+  main_frame->ExecuteScript(WebString::fromUTF8("showPlugin(true)"));
+  ASSERT_TRUE(IsWindowVisible(plugin_hwnd));
+
+  main_frame->ExecuteScript(WebString::fromUTF8("showFrame(false)"));
+  ASSERT_FALSE(IsWindowVisible(plugin_hwnd));
+
+  main_frame->ExecuteScript(WebString::fromUTF8("showFrame(true)"));
+  ASSERT_TRUE(IsWindowVisible(plugin_hwnd));
 }
 #endif