Fix hang seen in plugin process because plugin creation ended up having to wait on UI thread. Instead of using sync messages, the plugin hwnd is initially parented to the RenderWidgetHost's HWND. It's then lazily reparented to an intermediate HWND on the UI thread when it comes time to move it.

BUG=10711
TEST=added regression tests, but testers please confirm plugins on top video sites are placed correctly.
Review URL: http://codereview.chromium.org/67285

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

7 files changed, 134 insertions, 16 deletions

diff --git a/webkit/glue/plugins/test/npapi_test_plugin.vcproj b/webkit/glue/plugins/test/npapi_test_plugin.vcproj
index 5c5d95a..eff52e9 100644
--- a/webkit/glue/plugins/test/npapi_test_plugin.vcproj
+++ b/webkit/glue/plugins/test/npapi_test_plugin.vcproj
@@ -175,6 +175,14 @@
 			>
 		</File>
 		<File
+			RelativePath=".\plugin_create_instance_in_paint.cc"
+			>
+		</File>
+		<File
+			RelativePath=".\plugin_create_instance_in_paint.h"
+			>
+		</File>
+		<File
 			RelativePath=".\plugin_delete_plugin_in_stream_test.cc"
 			>
 		</File>
diff --git a/webkit/glue/plugins/test/plugin_client.cc b/webkit/glue/plugins/test/plugin_client.cc
index 9837f9e..f87820e 100644
--- a/webkit/glue/plugins/test/plugin_client.cc
+++ b/webkit/glue/plugins/test/plugin_client.cc
@@ -5,6 +5,9 @@
 #include "base/string_util.h"
 #include "webkit/glue/plugins/test/plugin_client.h"
 #include "webkit/glue/plugins/test/plugin_arguments_test.h"
+#if defined(OS_WIN)
+#include "webkit/glue/plugins/test/plugin_create_instance_in_paint.h"
+#endif
 #include "webkit/glue/plugins/test/plugin_delete_plugin_in_stream_test.h"
 #include "webkit/glue/plugins/test/plugin_get_javascript_url_test.h"
 #include "webkit/glue/plugins/test/plugin_geturl_test.h"
@@ -91,7 +94,7 @@ NPError NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode,
 
   NPError ret = NPERR_GENERIC_ERROR;
   bool windowless_plugin = false;
-  
+
   NPAPIClient::PluginTest *new_test = NULL;
   if (test_name == "arguments") {
     new_test = new NPAPIClient::PluginArgumentsTest(instance,
@@ -119,6 +122,9 @@ NPError NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode,
   } else if (test_name == "checkwindowrect") {
     new_test = new NPAPIClient::PluginWindowSizeTest(instance,
       NPAPIClient::PluginClient::HostFunctions());
+  } else if (test_name == "create_instance_in_paint") {
+    new_test = new NPAPIClient::CreateInstanceInPaintTest(instance,
+      NPAPIClient::PluginClient::HostFunctions());
 #endif
   } else if (test_name == "self_delete_plugin_stream") {
     new_test = new NPAPIClient::DeletePluginInStreamTest(instance,
@@ -138,8 +144,7 @@ NPError NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode,
     new_test = new NPAPIClient::NPObjectDeletePluginInNPN_Evaluate(instance,
       NPAPIClient::PluginClient::HostFunctions());
 #endif
-  } else if (test_name ==
-             "plugin_javascript_open_popup_with_plugin") {
+  } else if (test_name == "plugin_javascript_open_popup_with_plugin") {
     new_test = new NPAPIClient::ExecuteJavascriptOpenPopupWithPluginTest(
         instance, NPAPIClient::PluginClient::HostFunctions());
   } else if (test_name == "plugin_popup_with_plugin_target") {
diff --git a/webkit/glue/plugins/test/plugin_create_instance_in_paint.cc b/webkit/glue/plugins/test/plugin_create_instance_in_paint.cc
new file mode 100644
index 0000000..00f41da
--- /dev/null
+++ b/webkit/glue/plugins/test/plugin_create_instance_in_paint.cc
@@ -0,0 +1,73 @@
+// 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_create_instance_in_paint.h"
+
+#include "webkit/glue/plugins/test/plugin_client.h"
+
+namespace NPAPIClient {
+
+CreateInstanceInPaintTest::CreateInstanceInPaintTest(
+    NPP id, NPNetscapeFuncs *host_functions)
+    : PluginTest(id, host_functions),
+      window_(NULL), created_(false) {
+}
+
+NPError CreateInstanceInPaintTest::SetWindow(NPWindow* pNPWindow) {
+  if (test_id() == "1") {
+    if (!window_) {
+      static ATOM window_class = 0;
+      if (!window_class) {
+        WNDCLASSEX wcex;
+        wcex.cbSize         = sizeof(WNDCLASSEX);
+        wcex.style          = CS_DBLCLKS;
+        wcex.lpfnWndProc    =
+            &NPAPIClient::CreateInstanceInPaintTest::WindowProc;
+        wcex.cbClsExtra     = 0;
+        wcex.cbWndExtra     = 0;
+        wcex.hInstance      = GetModuleHandle(NULL);
+        wcex.hIcon          = 0;
+        wcex.hCursor        = 0;
+        wcex.hbrBackground  = reinterpret_cast<HBRUSH>(COLOR_WINDOW+1);
+        wcex.lpszMenuName   = 0;
+        wcex.lpszClassName  = L"CreateInstanceInPaintTestWindowClass";
+        wcex.hIconSm        = 0;
+        window_class = RegisterClassEx(&wcex);
+      }
+
+      HWND parent = reinterpret_cast<HWND>(pNPWindow->window);
+      window_ = CreateWindowEx(
+          WS_EX_LEFT | WS_EX_LTRREADING | WS_EX_RIGHTSCROLLBAR,
+          MAKEINTATOM(window_class), 0,
+          WS_CHILD | WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_VISIBLE ,
+          0, 0, 100, 100, parent, 0, GetModuleHandle(NULL), 0);
+      DCHECK(window_);
+      ::SetProp(window_, L"Plugin_Instance", this);
+    }
+  } else if (test_id() == "2") {
+    SignalTestCompleted();
+  } else {
+    NOTREACHED();
+  }
+  return NPERR_NO_ERROR;
+}
+
+LRESULT CALLBACK CreateInstanceInPaintTest::WindowProc(
+    HWND window, UINT message, WPARAM wparam, LPARAM lparam) {
+  if (message == WM_PAINT) {
+    CreateInstanceInPaintTest* this_instance =
+        reinterpret_cast<CreateInstanceInPaintTest*>
+            (::GetProp(window, L"Plugin_Instance"));
+    if (this_instance->test_id() == "1" && !this_instance->created_) {
+      this_instance->created_ = true;
+      this_instance->HostFunctions()->geturlnotify(
+          this_instance->id(), "javascript:CreateNewInstance()", NULL,
+          reinterpret_cast<void*>(1));
+    }
+  }
+
+  return DefWindowProc(window, message, wparam, lparam);
+}
+
+} // namespace NPAPIClient
diff --git a/webkit/glue/plugins/test/plugin_create_instance_in_paint.h b/webkit/glue/plugins/test/plugin_create_instance_in_paint.h
new file mode 100644
index 0000000..84d7a94
--- /dev/null
+++ b/webkit/glue/plugins/test/plugin_create_instance_in_paint.h
@@ -0,0 +1,33 @@
+// 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_CREATE_INSTANCE_IN_PAINT_H
+#define WEBKIT_GLUE_PLUGINS_TEST_PLUGIN_CREATE_INSTANCE_IN_PAINT_H
+
+#include "webkit/glue/plugins/test/plugin_test.h"
+
+namespace NPAPIClient {
+
+// This class tests that creating a new plugin via script while handling a
+// Windows message doesn't cause a deadlock.
+class CreateInstanceInPaintTest : public PluginTest {
+ public:
+  // Constructor.
+  CreateInstanceInPaintTest(NPP id, NPNetscapeFuncs *host_functions);
+  //
+  // NPAPI functions
+  //
+  virtual NPError SetWindow(NPWindow* pNPWindow);
+
+ private:
+  static LRESULT CALLBACK WindowProc(
+      HWND window, UINT message, WPARAM wparam, LPARAM lparam);
+
+  HWND window_;
+  bool created_;
+};
+
+} // namespace NPAPIClient
+
+#endif  // WEBKIT_GLUE_PLUGINS_TEST_PLUGIN_CREATE_INSTANCE_IN_PAINT_H
diff --git a/webkit/glue/plugins/test/plugin_test.h b/webkit/glue/plugins/test/plugin_test.h
index 77b280a..f763acc 100644
--- a/webkit/glue/plugins/test/plugin_test.h
+++ b/webkit/glue/plugins/test/plugin_test.h
@@ -110,6 +110,7 @@ class PluginTest {
 
   // The NPP Identifier for this plugin instance.
   NPP id() { return id_; }
+  std::string test_id() { return test_id_; }
 
  private:
   NPP                       id_;
diff --git a/webkit/glue/plugins/webplugin_delegate_impl.cc b/webkit/glue/plugins/webplugin_delegate_impl.cc
index cd11d07..7a48c48 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.cc
+++ b/webkit/glue/plugins/webplugin_delegate_impl.cc
@@ -145,7 +145,6 @@ WebPluginDelegateImpl::WebPluginDelegateImpl(
       windowless_(false),
       windowed_handle_(NULL),
       windowed_did_set_window_(false),
-      windowed_manage_position_(false),
       windowless_needs_set_window_(true),
       plugin_wnd_proc_(NULL),
       last_message_(0),
@@ -262,7 +261,7 @@ bool WebPluginDelegateImpl::Initialize(const GURL& url,
       return false;
   }
 
-  windowed_manage_position_ = plugin->SetWindow(windowed_handle_);
+  plugin->SetWindow(windowed_handle_);
 #if defined(OS_WIN)
   if (windowless_) {
     // For windowless plugins we should set the containing window handle
@@ -520,6 +519,8 @@ void WebPluginDelegateImpl::WindowedDestroyWindow() {
                        reinterpret_cast<LONG>(plugin_wnd_proc_));
     }
 
+    plugin_->WillDestroyWindow(windowed_handle_);
+
     DestroyWindow(windowed_handle_);
     TRACK_HWND_DESTRUCTION(windowed_handle_);
     windowed_handle_ = 0;
@@ -704,17 +705,17 @@ bool WebPluginDelegateImpl::WindowedReposition(
   if (window_rect_ == window_rect && clip_rect_ == clip_rect)
     return false;
 
-  // If windowed_manage_position_ is false, then the plugin will be moved
-  // elsewhere.  This allows the window moves/scrolling/clipping to be
-  // synchronized with the page and other windows.
+  // We only set the plugin's size here.  Its position is moved elsewhere, which
+  // allows the window moves/scrolling/clipping to be synchronized with the page
+  // and other windows.
   if (window_rect.size() != window_rect_.size()) {
     ::SetWindowPos(windowed_handle_,
                    NULL,
-                   windowed_manage_position_ ? window_rect.x() : 0,
-                   windowed_manage_position_ ? window_rect.y() : 0,
+                   0,
+                   0,
                    window_rect.width(),
                    window_rect.height(),
-                   SWP_SHOWWINDOW);
+                   0);
   }
 
   window_rect_ = window_rect;
@@ -745,8 +746,8 @@ void WebPluginDelegateImpl::WindowedSetWindow() {
   window_.clipRect.right = clip_rect_.x() + clip_rect_.width();
   window_.height = window_rect_.height();
   window_.width = window_rect_.width();
-  window_.x = windowed_manage_position_ ? window_rect_.x() : 0;
-  window_.y = windowed_manage_position_ ? window_rect_.y() : 0;
+  window_.x = 0;
+  window_.y = 0;
 
   window_.window = windowed_handle_;
   window_.type = NPWindowTypeWindow;
diff --git a/webkit/glue/plugins/webplugin_delegate_impl.h b/webkit/glue/plugins/webplugin_delegate_impl.h
index 9110c47..80c478b 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.h
+++ b/webkit/glue/plugins/webplugin_delegate_impl.h
@@ -160,9 +160,6 @@ class WebPluginDelegateImpl : public WebPluginDelegate {
   bool windowed_did_set_window_;
 #if defined(OS_WIN)
   gfx::Rect windowed_last_pos_;
-  // True if we should manage the position of the HWND.  If false, it's managed
-  // elsewhere and we should keep it at the origin.
-  bool windowed_manage_position_;
 #endif
 
   // this is an optimization to avoid calling SetWindow to the plugin