diff options
22 files changed, 343 insertions, 241 deletions
diff --git a/chrome/browser/plugin_process_host.cc b/chrome/browser/plugin_process_host.cc index 6cea0c7..9ed3dbf 100644 --- a/chrome/browser/plugin_process_host.cc +++ b/chrome/browser/plugin_process_host.cc @@ -254,92 +254,16 @@ void PluginDownloadUrlHelper::DownloadCompletedHelper(bool success) { delete this; } - -// Sends the reply to the create window message on the IO thread. -class SendReplyTask : public Task { - public: - SendReplyTask(FilePath plugin_path, HWND window, IPC::Message* reply_msg) - : plugin_path_(plugin_path), - reply_msg_(reply_msg), - window_(window){ } - - virtual void Run() { - PluginProcessHost* plugin = - PluginService::GetInstance()->FindPluginProcess(plugin_path_); - if (!plugin) - return; - - plugin->AddWindow(window_); - plugin->Send(reply_msg_); - } - - private: - FilePath plugin_path_; - IPC::Message* reply_msg_; - HWND window_; -}; - -// Creates a child window of the given HWND on the UI thread. -class CreateWindowTask : public Task { - public: - CreateWindowTask( - FilePath plugin_path, HWND parent, IPC::Message* reply_msg) - : plugin_path_(plugin_path), parent_(parent), reply_msg_(reply_msg) { } - - virtual void Run() { - static ATOM window_class = 0; - if (!window_class) { - WNDCLASSEX wcex; - wcex.cbSize = sizeof(WNDCLASSEX); - wcex.style = CS_DBLCLKS; - wcex.lpfnWndProc = DefWindowProc; - 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 = kWrapperNativeWindowClassName; - wcex.hIconSm = 0; - window_class = RegisterClassEx(&wcex); - } - - HWND window = CreateWindowEx( - WS_EX_LEFT | WS_EX_LTRREADING | WS_EX_RIGHTSCROLLBAR, - MAKEINTATOM(window_class), 0, - WS_CHILD | WS_CLIPCHILDREN | WS_CLIPSIBLINGS, - 0, 0, 0, 0, parent_, 0, GetModuleHandle(NULL), 0); - TRACK_HWND_CREATION(window); - - PluginProcessHostMsg_CreateWindow::WriteReplyParams( - reply_msg_, window); - - g_browser_process->io_thread()->message_loop()->PostTask( - FROM_HERE, new SendReplyTask(plugin_path_, window, reply_msg_)); - } - - private: - FilePath plugin_path_; - HWND parent_; - IPC::Message* reply_msg_; -}; - -void PluginProcessHost::OnCreateWindow(HWND parent, - IPC::Message* reply_msg) { - // Need to create this window on the UI thread. - PluginService::GetInstance()->main_message_loop()->PostTask( - FROM_HERE, new CreateWindowTask(info_.path, parent, reply_msg)); -} - -void PluginProcessHost::OnDestroyWindow(HWND window) { +void PluginProcessHost::OnPluginWindowDestroyed(HWND window, HWND parent) { + // The window is destroyed at this point, we just care about its parent, which + // is the intermediate window we created. std::set<HWND>::iterator window_index = - plugin_parent_windows_set_.find(window); - if (window_index != plugin_parent_windows_set_.end()) { - plugin_parent_windows_set_.erase(window_index); - } + plugin_parent_windows_set_.find(parent); + if (window_index == plugin_parent_windows_set_.end()) + return; - PostMessage(window, WM_CLOSE, 0, 0); + plugin_parent_windows_set_.erase(window_index); + PostMessage(parent, WM_CLOSE, 0, 0); } void PluginProcessHost::OnDownloadUrl(const std::string& url, @@ -499,9 +423,8 @@ void PluginProcessHost::OnMessageReceived(const IPC::Message& msg) { IPC_MESSAGE_HANDLER_DELAY_REPLY(PluginProcessHostMsg_ResolveProxy, OnResolveProxy) #if defined(OS_WIN) - IPC_MESSAGE_HANDLER_DELAY_REPLY(PluginProcessHostMsg_CreateWindow, - OnCreateWindow) - IPC_MESSAGE_HANDLER(PluginProcessHostMsg_DestroyWindow, OnDestroyWindow) + IPC_MESSAGE_HANDLER(PluginProcessHostMsg_PluginWindowDestroyed, + OnPluginWindowDestroyed) IPC_MESSAGE_HANDLER(PluginProcessHostMsg_DownloadUrl, OnDownloadUrl) #endif IPC_MESSAGE_UNHANDLED_ERROR() diff --git a/chrome/browser/plugin_process_host.h b/chrome/browser/plugin_process_host.h index c43fef2..a326bee 100644 --- a/chrome/browser/plugin_process_host.h +++ b/chrome/browser/plugin_process_host.h @@ -104,8 +104,7 @@ class PluginProcessHost : public ChildProcessHost, void OnPluginMessage(const std::vector<uint8>& data); #if defined(OS_WIN) - void OnCreateWindow(HWND parent, IPC::Message* reply_msg); - void OnDestroyWindow(HWND window); + void OnPluginWindowDestroyed(HWND window, HWND parent); void OnDownloadUrl(const std::string& url, int source_pid, gfx::NativeWindow caller_window); #endif diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.cc b/chrome/browser/renderer_host/render_widget_host_view_win.cc index fb243d3..1a29a99 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_win.cc +++ b/chrome/browser/renderer_host/render_widget_host_view_win.cc @@ -11,7 +11,9 @@ #include "base/win_util.h" #include "chrome/browser/browser_accessibility.h" #include "chrome/browser/browser_accessibility_manager.h" +#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_trial.h" +#include "chrome/browser/plugin_process_host.h" #include "chrome/browser/renderer_host/backing_store.h" #include "chrome/browser/renderer_host/render_process_host.h" #include "chrome/browser/renderer_host/render_widget_host.h" @@ -116,6 +118,33 @@ static bool GetNewTextDirection(WebTextDirection* direction) { return true; } +class NotifyPluginProcessHostTask : public Task { + public: + NotifyPluginProcessHostTask(HWND window, HWND parent) + : window_(window), parent_(parent) { } + + private: + void Run() { + DWORD plugin_process_id; + GetWindowThreadProcessId(window_, &plugin_process_id); + for (ChildProcessHost::Iterator iter(ChildProcessInfo::PLUGIN_PROCESS); + !iter.Done(); ++iter) { + PluginProcessHost* plugin = static_cast<PluginProcessHost*>(*iter); + if (plugin->GetProcessId() == plugin_process_id) { + plugin->AddWindow(parent_); + return; + } + } + + // The plugin process might have died in the time to execute the task, don't + // leak the HWND. + PostMessage(parent_, WM_CLOSE, 0, 0); + } + + HWND window_; // Plugin HWND, created and destroyed in the plugin process. + HWND parent_; // Parent HWND, created and destroyed on the browser UI thread. +}; + } // namespace // RenderWidgetHostView -------------------------------------------------------- @@ -219,6 +248,10 @@ void RenderWidgetHostViewWin::MovePluginWindows( if (plugin_window_moves.empty()) return; + bool oop_plugins = + !CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess) && + !CommandLine::ForCurrentProcess()->HasSwitch(switches::kInProcessPlugins); + HDWP defer_window_pos_info = ::BeginDeferWindowPos(static_cast<int>(plugin_window_moves.size())); @@ -230,18 +263,36 @@ void RenderWidgetHostViewWin::MovePluginWindows( for (size_t i = 0; i < plugin_window_moves.size(); ++i) { unsigned long flags = 0; const WebPluginGeometry& move = plugin_window_moves[i]; + HWND window = move.window; // As the plugin parent window which lives on the browser UI thread is // destroyed asynchronously, it is possible that we have a stale window // sent in by the renderer for moving around. - if (!::IsWindow(move.window)) + // Note: get the parent before checking if the window is valid, to avoid a + // race condition where the window is destroyed after the check but before + // the GetParent call. + HWND parent = ::GetParent(window); + if (!::IsWindow(window)) continue; - // The renderer should only be trying to move windows that are children - // of its render widget window. - if (::IsChild(m_hWnd, move.window) == 0) { - NOTREACHED(); - continue; + if (oop_plugins) { + if (parent == m_hWnd) { + // The plugin window is a direct child of this window, add an + // intermediate window that lives on this thread to speed up scrolling. + // Note this only works with out of process plugins since we depend on + // PluginProcessHost to destroy the intermediate HWNDs. + parent = ReparentWindow(window); + ::ShowWindow(window, SW_SHOW); // Window was created hidden. + } else if (::GetParent(parent) != m_hWnd) { + // The renderer should only be trying to move windows that are children + // of its render widget window. + NOTREACHED(); + continue; + } + + // We move the intermediate parent window which doesn't result in cross- + // process synchronous Windows messages. + window = parent; } if (move.visible) @@ -257,10 +308,10 @@ void RenderWidgetHostViewWin::MovePluginWindows( // Note: System will own the hrgn after we call SetWindowRgn, // so we don't need to call DeleteObject(hrgn) - ::SetWindowRgn(move.window, hrgn, !move.clip_rect.IsEmpty()); + ::SetWindowRgn(window, hrgn, !move.clip_rect.IsEmpty()); defer_window_pos_info = ::DeferWindowPos(defer_window_pos_info, - move.window, NULL, + window, NULL, move.window_rect.x(), move.window_rect.y(), move.window_rect.width(), @@ -274,6 +325,37 @@ void RenderWidgetHostViewWin::MovePluginWindows( ::EndDeferWindowPos(defer_window_pos_info); } +HWND RenderWidgetHostViewWin::ReparentWindow(HWND window) { + static ATOM window_class = 0; + if (!window_class) { + WNDCLASSEX wcex; + wcex.cbSize = sizeof(WNDCLASSEX); + wcex.style = CS_DBLCLKS; + wcex.lpfnWndProc = ::DefWindowProc; + 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 = kWrapperNativeWindowClassName; + wcex.hIconSm = 0; + window_class = RegisterClassEx(&wcex); + } + + HWND parent = CreateWindowEx( + WS_EX_LEFT | WS_EX_LTRREADING | WS_EX_RIGHTSCROLLBAR, + MAKEINTATOM(window_class), 0, + WS_CHILD | WS_CLIPCHILDREN | WS_CLIPSIBLINGS, + 0, 0, 0, 0, ::GetParent(window), 0, GetModuleHandle(NULL), 0); + DCHECK(parent); + ::SetParent(window, parent); + g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE, + new NotifyPluginProcessHostTask(window, parent)); + return parent; +} + void RenderWidgetHostViewWin::Focus() { if (IsWindow()) SetFocus(); diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.h b/chrome/browser/renderer_host/render_widget_host_view_win.h index 893e129..3501ca1 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_win.h +++ b/chrome/browser/renderer_host/render_widget_host_view_win.h @@ -210,6 +210,9 @@ class RenderWidgetHostViewWin : // given paint rect. void DrawResizeCorner(const gfx::Rect& paint_rect, HDC dc); + // Create an intermediate window between the given HWND and its parent. + HWND ReparentWindow(HWND window); + // The associated Model. RenderWidgetHost* render_widget_host_; diff --git a/chrome/common/plugin_messages_internal.h b/chrome/common/plugin_messages_internal.h index 11a0365..54839fb 100644 --- a/chrome/common/plugin_messages_internal.h +++ b/chrome/common/plugin_messages_internal.h @@ -78,9 +78,10 @@ IPC_BEGIN_MESSAGES(PluginProcessHost) HWND /* parent */, HWND /* child */) - // Destroys the given window on the UI thread. - IPC_MESSAGE_CONTROL1(PluginProcessHostMsg_DestroyWindow, - HWND /* window */) + // Destroys the given window's parent on the UI thread. + IPC_MESSAGE_CONTROL2(PluginProcessHostMsg_PluginWindowDestroyed, + HWND /* window */, + HWND /* parent */) IPC_MESSAGE_ROUTED3(PluginProcessHostMsg_DownloadUrl, std::string /* URL */, diff --git a/chrome/plugin/webplugin_proxy.cc b/chrome/plugin/webplugin_proxy.cc index 7933082..6a827ce 100644 --- a/chrome/plugin/webplugin_proxy.cc +++ b/chrome/plugin/webplugin_proxy.cc @@ -38,49 +38,28 @@ WebPluginProxy::WebPluginProxy( delegate_(delegate), waiting_for_paint_(false), #pragma warning(suppress: 4355) // can use this - runnable_method_factory_(this), - parent_window_(NULL) { + runnable_method_factory_(this) { } WebPluginProxy::~WebPluginProxy() { if (cp_browsing_context_) GetContextMap().erase(cp_browsing_context_); - - if (parent_window_) { - PluginThread::current()->Send( - new PluginProcessHostMsg_DestroyWindow(parent_window_)); - } } bool WebPluginProxy::Send(IPC::Message* msg) { return channel_->Send(msg); } -bool WebPluginProxy::SetWindow(gfx::NativeView window) { - if (window) { - // To make scrolling windowed plugins fast, we create the page's direct - // child windows in the browser process. This way no cross process messages - // are sent. - HWND old_parent = GetParent(window); - IPC::SyncMessage* msg = new PluginProcessHostMsg_CreateWindow( - old_parent, &parent_window_); - - // Need to process window messages in the meantime to avoid a deadlock if - // the browser paints or sends any other (synchronous) WM_ message to the - // plugin window. - msg->EnableMessagePumping(); - PluginThread::current()->Send(msg); - - SetParent(window, parent_window_); - - // We want the browser process to move this window which has a message loop - // in its process. - window = parent_window_; - } - +void WebPluginProxy::SetWindow(gfx::NativeView window) { Send(new PluginHostMsg_SetWindow(route_id_, gfx::IdFromNativeView(window))); +} - return false; +void WebPluginProxy::WillDestroyWindow(gfx::NativeView window) { +#if defined(OS_WIN) + PluginThread::current()->Send( + new PluginProcessHostMsg_PluginWindowDestroyed( + window, ::GetParent(window))); +#endif } #if defined(OS_WIN) diff --git a/chrome/plugin/webplugin_proxy.h b/chrome/plugin/webplugin_proxy.h index 8c4629d..ea2c124 100644 --- a/chrome/plugin/webplugin_proxy.h +++ b/chrome/plugin/webplugin_proxy.h @@ -34,7 +34,8 @@ class WebPluginProxy : public WebPlugin { ~WebPluginProxy(); // WebPlugin overrides - bool SetWindow(gfx::NativeView window); + void SetWindow(gfx::NativeView window); + void WillDestroyWindow(gfx::NativeView window); #if defined(OS_WIN) void SetWindowlessPumpEvent(HANDLE pump_messages_event); void SetModalDialogEvent(HANDLE modal_dialog_event); @@ -136,7 +137,6 @@ class WebPluginProxy : public WebPlugin { bool waiting_for_paint_; uint32 cp_browsing_context_; scoped_ptr<base::WaitableEvent> modal_dialog_event_; - HWND parent_window_; // Variables used for desynchronized windowless plugin painting. See note in // webplugin_delegate_proxy.h for how this works. diff --git a/chrome/test/data/npapi/create_instance_in_paint.html b/chrome/test/data/npapi/create_instance_in_paint.html new file mode 100644 index 0000000..80e8935 --- /dev/null +++ b/chrome/test/data/npapi/create_instance_in_paint.html @@ -0,0 +1,44 @@ +<html> + +<head> +<script src="npapi.js"></script> +</head> + +<body> +<div id="statusPanel" style="border: 1px solid red; width: 100%"> +Test running.... +</div> + + +Plugin Instance In Paint Test<p> + +Tests that there's no deadlock when a plugin instance is created while handling a paint message. + +<DIV ID="PluginDiv"> +<embed type="application/vnd.npapi-test" + src="foo" + name="create_instance_in_paint" + id="1" + mode="np_embed" +> +</DIV> + +<DIV id="PluginDiv2"></DIV> + +<script> +var height = document.body.offsetHeight; + + +function CreateNewInstance() { + var obj = document.createElement('embed');
+ obj.setAttribute('type', 'application/vnd.npapi-test');
+ obj.setAttribute('src', 'bar');
+ obj.setAttribute('name', 'create_instance_in_paint');
+ obj.setAttribute('id', '2'); + obj.setAttribute('mode', 'np_embed'); + document.getElementById("PluginDiv2").appendChild(obj); +} +</script> + +</body> +</html> diff --git a/chrome/test/ui/npapi_test_helper.cc b/chrome/test/ui/npapi_test_helper.cc index 859d3df..ab891f6 100644 --- a/chrome/test/ui/npapi_test_helper.cc +++ b/chrome/test/ui/npapi_test_helper.cc @@ -4,30 +4,9 @@ // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are // met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following disclaimer -// in the documentation and/or other materials provided with the -// distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -// NPAPI test helper classes. +// 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 "chrome/test/ui/npapi_test_helper.h" diff --git a/chrome/test/ui/npapi_test_helper.h b/chrome/test/ui/npapi_test_helper.h index 7b79e0e..6af2d73 100644 --- a/chrome/test/ui/npapi_test_helper.h +++ b/chrome/test/ui/npapi_test_helper.h @@ -1,31 +1,9 @@ -// Copyright 2008, Google Inc. -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following disclaimer -// in the documentation and/or other materials provided with the -// distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// 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 CHROME_TEST_HELPER_H_ +#define CHROME_TEST_HELPER_H_ #include "chrome/test/ui/ui_test.h" @@ -52,3 +30,5 @@ class NPAPIIncognitoTester : public NPAPITester { protected: virtual void SetUp(); }; + +#endif // CHROME_TEST_HELPER_H_ diff --git a/chrome/test/ui/npapi_uitest.cc b/chrome/test/ui/npapi_uitest.cc index 7826878..3a8905a 100644 --- a/chrome/test/ui/npapi_uitest.cc +++ b/chrome/test/ui/npapi_uitest.cc @@ -1,35 +1,6 @@ -// Copyright 2008, Google Inc. -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following disclaimer -// in the documentation and/or other materials provided with the -// distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -// -// NPAPI UnitTests. -// +// 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. // windows headers #include <windows.h> @@ -220,6 +191,17 @@ TEST_F(NPAPIVisiblePluginTester, VerifyPluginWindowRect) { kTestCompleteSuccess, kShortWaitTimeout); } +// Tests that creating a new instance of a plugin while another one is handling +// a paint message doesn't cause deadlock. +TEST_F(NPAPIVisiblePluginTester, CreateInstanceInPaint) { + show_window_ = true; + std::wstring test_case = L"create_instance_in_paint.html"; + GURL url = GetTestUrl(L"npapi", test_case); + NavigateToURL(url); + WaitForFinish("create_instance_in_paint", "2", url, kTestCompleteCookie, + kTestCompleteSuccess, kShortWaitTimeout); +} + TEST_F(NPAPIVisiblePluginTester, VerifyNPObjectLifetimeTest) { if (!UITest::in_process_renderer()) { show_window_ = true; 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 diff --git a/webkit/glue/webplugin.h b/webkit/glue/webplugin.h index 8c56100..64791ea 100644 --- a/webkit/glue/webplugin.h +++ b/webkit/glue/webplugin.h @@ -86,10 +86,11 @@ class WebPlugin { // windowed (i.e. handle is not NULL) or windowless (handle is NULL). This // tells the WebPlugin to send mouse/keyboard events to the plugin delegate, // as well as the information about the HDC for paint operations. - // The return value is only valid for windowed plugins. If true, it means - // that the the HWND position should be managed by the delegate. If false, - // the plugin should be kept at the origin and it'll get moved elsewhere. - virtual bool SetWindow(gfx::NativeView window) = 0; + virtual void SetWindow(gfx::NativeView window) = 0; + + // Called by the plugin delegate to let it know that the window is being + // destroyed. + virtual void WillDestroyWindow(gfx::NativeView window) = 0; #if defined(OS_WIN) // The pump_messages_event is a event handle which is valid only for // windowless plugins and is used in NPP_HandleEvent calls to pump messages diff --git a/webkit/glue/webplugin_impl.cc b/webkit/glue/webplugin_impl.cc index 1407b4d..9d5b2c5 100644 --- a/webkit/glue/webplugin_impl.cc +++ b/webkit/glue/webplugin_impl.cc @@ -334,7 +334,7 @@ WebPluginImpl::WebPluginImpl(WebCore::HTMLPlugInElement* element, WebPluginImpl::~WebPluginImpl() { } -bool WebPluginImpl::SetWindow(gfx::NativeView window) { +void WebPluginImpl::SetWindow(gfx::NativeView window) { if (window) { DCHECK(!windowless_); // Make sure not called twice. window_ = window; @@ -342,8 +342,6 @@ bool WebPluginImpl::SetWindow(gfx::NativeView window) { DCHECK(!window_); // Make sure not called twice. windowless_ = true; } - - return true; } bool WebPluginImpl::CompleteURL(const std::string& url_in, diff --git a/webkit/glue/webplugin_impl.h b/webkit/glue/webplugin_impl.h index 86ece4b..64f964e 100644 --- a/webkit/glue/webplugin_impl.h +++ b/webkit/glue/webplugin_impl.h @@ -147,7 +147,8 @@ class WebPluginImpl : public WebPlugin, int arg_count, char** arg_names, char** arg_values); // WebPlugin implementation: - bool SetWindow(gfx::NativeView window); + void SetWindow(gfx::NativeView window); + void WillDestroyWindow(gfx::NativeView window) { } #if defined(OS_WIN) void SetWindowlessPumpEvent(HANDLE pump_messages_event) { } #endif diff --git a/webkit/tools/test_shell/test_webview_delegate_win.cc b/webkit/tools/test_shell/test_webview_delegate_win.cc index 16d84e9..b06b1ec 100755 --- a/webkit/tools/test_shell/test_webview_delegate_win.cc +++ b/webkit/tools/test_shell/test_webview_delegate_win.cc @@ -153,7 +153,19 @@ void TestWebViewDelegate::DidMove(WebWidget* webwidget, // Note: System will own the hrgn after we call SetWindowRgn, // so we don't need to call DeleteObject(hrgn) ::SetWindowRgn(move.window, hrgn, FALSE); - ::ShowWindow(move.window, move.visible ? SW_SHOW : SW_HIDE); + unsigned long flags = 0; + if (move.visible) + flags |= SWP_SHOWWINDOW; + else + flags |= SWP_HIDEWINDOW; + + ::SetWindowPos(move.window, + NULL, + move.window_rect.x(), + move.window_rect.y(), + move.window_rect.width(), + move.window_rect.height(), + flags); } void TestWebViewDelegate::RunModal(WebWidget* webwidget) { |