Report the plugin position to windowed plugins as (0,0)

.  the NPAPI documentation says it should be reported relative to the parent HWND, which is the plugin's coordinates on the page.  This assumption breaks in Chrome because we have an intermediate HWND.  I've tested on a bunch of pages to see if this change causes regressions, if we find any in the future we can reconsider this.

BUG=6742

Review URL: http://codereview.chromium.org/42626

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

9 files changed, 44 insertions, 41 deletions

diff --git a/chrome/plugin/webplugin_proxy.cc b/chrome/plugin/webplugin_proxy.cc
index 352e287..dbe9b5a 100644
--- a/chrome/plugin/webplugin_proxy.cc
+++ b/chrome/plugin/webplugin_proxy.cc
@@ -70,7 +70,7 @@ bool WebPluginProxy::Send(IPC::Message* msg) {
   return channel_->Send(msg);
 }
 
-void WebPluginProxy::SetWindow(HWND window, HANDLE pump_messages_event) {
+bool WebPluginProxy::SetWindow(HWND window, HANDLE pump_messages_event) {
   HANDLE pump_messages_event_for_renderer = NULL;
 
   if (pump_messages_event) {
@@ -104,6 +104,8 @@ void WebPluginProxy::SetWindow(HWND window, HANDLE pump_messages_event) {
 
   Send(new PluginHostMsg_SetWindow(route_id_, window,
                                    pump_messages_event_for_renderer));
+
+  return false;
 }
 
 void WebPluginProxy::CancelResource(int id) {
diff --git a/chrome/plugin/webplugin_proxy.h b/chrome/plugin/webplugin_proxy.h
index c51769d..78ca43c 100644
--- a/chrome/plugin/webplugin_proxy.h
+++ b/chrome/plugin/webplugin_proxy.h
@@ -35,7 +35,7 @@ class WebPluginProxy : public WebPlugin {
   ~WebPluginProxy();
 
   // WebPlugin overrides
-  void SetWindow(HWND window, HANDLE pump_messages_event);
+  bool SetWindow(HWND window, HANDLE pump_messages_event);
   void CancelResource(int id);
   void Invalidate();
   void InvalidateRect(const gfx::Rect& rect);
diff --git a/webkit/glue/plugins/test/plugin_window_size_test.cc b/webkit/glue/plugins/test/plugin_window_size_test.cc
index 0321658..a0cb687 100644
--- a/webkit/glue/plugins/test/plugin_window_size_test.cc
+++ b/webkit/glue/plugins/test/plugin_window_size_test.cc
@@ -13,8 +13,8 @@ PluginWindowSizeTest::PluginWindowSizeTest(NPP id,
 }
 
 NPError PluginWindowSizeTest::SetWindow(NPWindow* pNPWindow) {
-  if (!pNPWindow ||
-      !::IsWindow(reinterpret_cast<HWND>(pNPWindow->window))) {
+  HWND window = reinterpret_cast<HWND>(pNPWindow->window);
+  if (!pNPWindow || !::IsWindow(window)) {
     SetError("Invalid arguments passed in");
     return NPERR_INVALID_PARAM;
   }
@@ -27,12 +27,22 @@ NPError PluginWindowSizeTest::SetWindow(NPWindow* pNPWindow) {
 
   if (!::IsRectEmpty(&window_rect)) {
     RECT client_rect = {0};
-    ::GetClientRect(reinterpret_cast<HWND>(pNPWindow->window),
-                    &client_rect);
+    ::GetClientRect(window, &client_rect);
     if (::IsRectEmpty(&client_rect)) {
       SetError("The client rect of the plugin window is empty. Test failed");
     }
 
+    // Bug 6742: ensure that the coordinates passed in are relative to the
+    // parent HWND.
+    POINT origin_from_os;
+    RECT window_rect_from_os;
+    ::GetWindowRect(window, &window_rect_from_os);
+    origin_from_os.x = window_rect_from_os.left;
+    origin_from_os.y = window_rect_from_os.top;
+    ::ScreenToClient(GetParent(window), &origin_from_os);
+    if (origin_from_os.x != pNPWindow->x || origin_from_os.y != pNPWindow->y)
+      SetError("Wrong position passed in to SetWindow!  Test failed");
+
     SignalTestCompleted();
   }
 
diff --git a/webkit/glue/plugins/webplugin_delegate_impl.cc b/webkit/glue/plugins/webplugin_delegate_impl.cc
index 564659d..3d18525 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.cc
+++ b/webkit/glue/plugins/webplugin_delegate_impl.cc
@@ -145,6 +145,7 @@ 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),
@@ -266,7 +267,8 @@ bool WebPluginDelegateImpl::Initialize(const GURL& url,
       return false;
   }
 
-  plugin->SetWindow(windowed_handle_, handle_event_pump_messages_event_);
+  windowed_manage_position_ =
+      plugin->SetWindow(windowed_handle_, handle_event_pump_messages_event_);
   plugin_url_ = url.spec();
 
   // The windowless version of the Silverlight plugin calls the
@@ -550,10 +552,10 @@ void WebPluginDelegateImpl::OnThrottleMessage() {
     const MSG& msg = *it;
     if (processed.find(msg.hwnd) == processed.end()) {
       WNDPROC proc = reinterpret_cast<WNDPROC>(msg.time);
-	  // It is possible that the window was closed after we queued
-	  // this message.  This is a rare event; just verify the window
-	  // is alive.  (see also bug 1259488)
-	  if (IsWindow(msg.hwnd))
+      // It is possible that the window was closed after we queued
+      // this message.  This is a rare event; just verify the window
+      // is alive.  (see also bug 1259488)
+      if (IsWindow(msg.hwnd))
           CallWindowProc(proc, msg.hwnd, msg.message, msg.wParam, msg.lParam);
       processed[msg.hwnd] = 1;
       it = throttle_queue->erase(it);
@@ -695,20 +697,14 @@ bool WebPluginDelegateImpl::WindowedReposition(
   if (window_rect_ == window_rect && clip_rect_ == clip_rect)
     return false;
 
-  // There are a few parts to managing the plugin windows:
-  //   - Initial geometry, show / resize / position the window.
-  //   - Geometry updates, resize the window.
-  //   - Geometry updates, move the window or update the clipping region.
-  // This code should handle the first two, positioning and sizing the window
-  // initially, and resizing it when the size changes.  Clipping and moving are
-  // handled separately by WebPlugin, after it has called this code.  This
-  // allows window moves, like scrolling, to be synchronized with painting.
-  // See WebPluginImpl::setFrameRect().
+  // 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.
   if (window_rect.size() != window_rect_.size()) {
     ::SetWindowPos(windowed_handle_,
                    NULL,
-                   0,
-                   0,
+                   windowed_manage_position_ ? window_rect.x() : 0,
+                   windowed_manage_position_ ? window_rect.y() : 0,
                    window_rect.width(),
                    window_rect.height(),
                    SWP_SHOWWINDOW);
@@ -742,8 +738,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 = window_rect_.x();
-  window_.y = window_rect_.y();
+  window_.x = windowed_manage_position_ ? window_rect_.x() : 0;
+  window_.y = windowed_manage_position_ ? window_rect_.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 c45b8f3..74b0e13 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.h
+++ b/webkit/glue/plugins/webplugin_delegate_impl.h
@@ -158,6 +158,9 @@ 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 c9b9251..814839a 100644
--- a/webkit/glue/webplugin.h
+++ b/webkit/glue/webplugin.h
@@ -89,7 +89,10 @@ class WebPlugin {
   // 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
   // if the plugin enters a modal loop.
-  virtual void SetWindow(gfx::NativeView window,
+  // 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,
                          HANDLE pump_messages_event) = 0;
   // Cancels a pending request.
   virtual void CancelResource(int id) = 0;
diff --git a/webkit/glue/webplugin_impl.cc b/webkit/glue/webplugin_impl.cc
index 0a56271..e822cfc 100644
--- a/webkit/glue/webplugin_impl.cc
+++ b/webkit/glue/webplugin_impl.cc
@@ -336,7 +336,7 @@ WebPluginImpl::WebPluginImpl(WebCore::HTMLPlugInElement* element,
 WebPluginImpl::~WebPluginImpl() {
 }
 
-void WebPluginImpl::SetWindow(gfx::NativeView window,
+bool WebPluginImpl::SetWindow(gfx::NativeView window,
                               HANDLE pump_messages_event) {
   if (window) {
     DCHECK(!windowless_);  // Make sure not called twice.
@@ -345,6 +345,8 @@ void 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 e5a526d..02d9357 100644
--- a/webkit/glue/webplugin_impl.h
+++ b/webkit/glue/webplugin_impl.h
@@ -147,7 +147,7 @@ class WebPluginImpl : public WebPlugin,
                 int arg_count, char** arg_names, char** arg_values);
 
   // WebPlugin implementation:
-  void SetWindow(gfx::NativeView window, HANDLE pump_messages_event);
+  bool SetWindow(gfx::NativeView window, HANDLE pump_messages_event);
 
   // Given a (maybe partial) url, completes using the base url.
   bool CompleteURL(const std::string& url_in, std::string* url_out);
diff --git a/webkit/tools/test_shell/test_webview_delegate_win.cc b/webkit/tools/test_shell/test_webview_delegate_win.cc
index ff4a7e2..28c5034 100755
--- a/webkit/tools/test_shell/test_webview_delegate_win.cc
+++ b/webkit/tools/test_shell/test_webview_delegate_win.cc
@@ -149,20 +149,7 @@ 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);
-
-  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);
+  ::ShowWindow(move.window, move.visible ? SW_SHOW : SW_HIDE);
 }
 
 void TestWebViewDelegate::RunModal(WebWidget* webwidget) {