Fix Flash window in Analytics being incorrectly visible.

The bug was that we accidentally marked a plugin widget as visible if it's parent was visible, even if it was explicitly set as hidden.

BUG=8927
TEST=regression test included, also can verify that analytics doesn't display the gray rectangle per the bug
Review URL: http://codereview.chromium.org/106008

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

9 files changed, 119 insertions, 4 deletions

diff --git a/webkit/data/plugins/hidden_plugin.html b/webkit/data/plugins/hidden_plugin.html
new file mode 100644
index 0000000..db6b575
--- /dev/null
+++ b/webkit/data/plugins/hidden_plugin.html
@@ -0,0 +1,2 @@
+<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
new file mode 100644
index 0000000..ad0a994
--- /dev/null
+++ b/webkit/data/plugins/hidden_plugin_iframe.html
@@ -0,0 +1,14 @@
+<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/glue/plugins/test/npapi_test_plugin.vcproj b/webkit/glue/plugins/test/npapi_test_plugin.vcproj
index eff52e9..a8c9a3c 100644
--- a/webkit/glue/plugins/test/npapi_test_plugin.vcproj
+++ b/webkit/glue/plugins/test/npapi_test_plugin.vcproj
@@ -263,6 +263,14 @@
 			>
 		</File>
 		<File
+			RelativePath=".\plugin_windowed_test.cc"
+			>
+		</File>
+		<File
+			RelativePath=".\plugin_windowed_test.h"
+			>
+		</File>
+		<File
 			RelativePath=".\plugin_windowless_test.cc"
 			>
 		</File>
diff --git a/webkit/glue/plugins/test/plugin_client.cc b/webkit/glue/plugins/test/plugin_client.cc
index f87820e..7a92de4 100644
--- a/webkit/glue/plugins/test/plugin_client.cc
+++ b/webkit/glue/plugins/test/plugin_client.cc
@@ -17,6 +17,7 @@
 #include "webkit/glue/plugins/test/plugin_npobject_lifetime_test.h"
 #include "webkit/glue/plugins/test/plugin_npobject_proxy_test.h"
 #include "webkit/glue/plugins/test/plugin_window_size_test.h"
+#include "webkit/glue/plugins/test/plugin_windowed_test.h"
 #include "webkit/glue/plugins/test/plugin_windowless_test.h"
 #include "third_party/npapi/bindings/npapi.h"
 #include "third_party/npapi/bindings/npruntime.h"
@@ -153,6 +154,12 @@ NPError NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode,
   } else if (test_name == "private") {
     new_test = new NPAPIClient::PrivateTest(instance,
       NPAPIClient::PluginClient::HostFunctions());
+#if defined(OS_WIN)
+  // TODO(port): plugin_windowed_test.*.
+  } else if (test_name == "hidden_plugin") {
+    new_test = new NPAPIClient::WindowedPluginTest(instance,
+        NPAPIClient::PluginClient::HostFunctions());
+#endif
   } else {
     // If we don't have a test case for this, create a
     // generic one which basically never fails.
diff --git a/webkit/glue/plugins/test/plugin_test.h b/webkit/glue/plugins/test/plugin_test.h
index f763acc..5914fef 100644
--- a/webkit/glue/plugins/test/plugin_test.h
+++ b/webkit/glue/plugins/test/plugin_test.h
@@ -111,6 +111,7 @@ class PluginTest {
   // The NPP Identifier for this plugin instance.
   NPP id() { return id_; }
   std::string test_id() { return test_id_; }
+  std::string test_name() { return test_name_; }
 
  private:
   NPP                       id_;
diff --git a/webkit/glue/plugins/test/plugin_windowed_test.cc b/webkit/glue/plugins/test/plugin_windowed_test.cc
new file mode 100644
index 0000000..20215ef
--- /dev/null
+++ b/webkit/glue/plugins/test/plugin_windowed_test.cc
@@ -0,0 +1,38 @@
+// Copyright (c) 2009 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 "webkit/glue/plugins/test/plugin_windowed_test.h"
+#include "webkit/glue/plugins/test/plugin_client.h"
+
+namespace NPAPIClient {
+
+WindowedPluginTest::WindowedPluginTest(NPP id, NPNetscapeFuncs *host_functions)
+    : PluginTest(id, host_functions) {
+}
+
+NPError WindowedPluginTest::SetWindow(NPWindow* pNPWindow) {
+  HWND window = reinterpret_cast<HWND>(pNPWindow->window);
+  if (!pNPWindow || !::IsWindow(window)) {
+    SetError("Invalid arguments passed in");
+    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;
+}
+
+} // namespace NPAPIClient
diff --git a/webkit/glue/plugins/test/plugin_windowed_test.h b/webkit/glue/plugins/test/plugin_windowed_test.h
new file mode 100644
index 0000000..547b079
--- /dev/null
+++ b/webkit/glue/plugins/test/plugin_windowed_test.h
@@ -0,0 +1,22 @@
+// Copyright (c) 2009 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.
+
+#ifndef WEBKIT_GLUE_PLUGINS_TEST_PLUGIN_WINDOWED_TEST_H
+#define WEBKIT_GLUE_PLUGINS_TEST_PLUGIN_WINDOWED_TEST_H
+
+#include "webkit/glue/plugins/test/plugin_test.h"
+
+namespace NPAPIClient {
+
+// This class contains a list of windowed plugin tests. Please add additional
+// tests to this class.
+class WindowedPluginTest : public PluginTest {
+ public:
+  WindowedPluginTest(NPP id, NPNetscapeFuncs *host_functions);
+  virtual NPError SetWindow(NPWindow* pNPWindow);
+};
+
+} // namespace NPAPIClient
+
+#endif  // WEBKIT_GLUE_PLUGINS_TEST_PLUGIN_WINDOWED_TEST_H
diff --git a/webkit/glue/webplugin_impl.cc b/webkit/glue/webplugin_impl.cc
index c1e63af..306a9a9 100644
--- a/webkit/glue/webplugin_impl.cc
+++ b/webkit/glue/webplugin_impl.cc
@@ -202,7 +202,13 @@ void WebPluginContainer::frameRectsChanged() {
 // webkit is ignored. This function is called when the plugin eventually
 // gets a parent.
 void WebPluginContainer::setParentVisible(bool visible) {
+  if (isParentVisible() == visible)
+    return;  // No change.
+
   WebCore::Widget::setParentVisible(visible);
+  if (!isSelfVisible())
+    return;  // This widget has explicitely been marked as not visible.
+
   if (visible)
     show();
   else
@@ -646,10 +652,6 @@ void WebPluginImpl::setFrameRect(const WebCore::IntRect& rect) {
   std::vector<gfx::Rect> cutout_rects;
   CalculateBounds(rect, &window_rect, &clip_rect, &cutout_rects);
 
-  // Notify the plugin that its parameters have changed.
-  delegate_->UpdateGeometry(webkit_glue::FromIntRect(window_rect),
-                            webkit_glue::FromIntRect(clip_rect));
-
   if (window_) {
     // Notify the window hosting the plugin (the WebViewDelegate) that
     // it needs to adjust the plugin, so that all the HWNDs can be moved
@@ -664,6 +666,10 @@ void WebPluginImpl::setFrameRect(const WebCore::IntRect& rect) {
     webview->delegate()->DidMove(webview, move);
   }
 
+  // Notify the plugin that its parameters have changed.
+  delegate_->UpdateGeometry(webkit_glue::FromIntRect(window_rect),
+                            webkit_glue::FromIntRect(clip_rect));
+
   // Initiate a download on the plugin url. This should be done for the
   // first update geometry sequence.
   if (first_geometry_update_) {
diff --git a/webkit/tools/test_shell/plugin_tests.cc b/webkit/tools/test_shell/plugin_tests.cc
index b501b36..eae50e0 100644
--- a/webkit/tools/test_shell/plugin_tests.cc
+++ b/webkit/tools/test_shell/plugin_tests.cc
@@ -156,3 +156,20 @@ TEST_F(PluginTest, DeleteFrameDuringEvent) {
 
   // No crash means we passed.
 }
+
+#if defined(OS_WIN)
+// Tests that a hidden plugin is not shown.  See http://crbug.com/8927
+TEST_F(PluginTest, HiddenPlugin) {
+  CopyTestPlugin();
+
+  FilePath test_html = data_dir_;
+  test_html = test_html.AppendASCII("plugins");
+  test_html = test_html.AppendASCII("hidden_plugin.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));
+}
+#endif