Defer window.close(), resizeTo() and moveTo() actions

by posting a task back to the message loop before notifying
the RenderWidgetHost to perform these operations.
Otherwise the JS code races with the browser to use the
modified window.

BUG=http://crbug.com/6377
BUG=http://crbug.com/6192

Cache a pending_window_rect on the render_view (moved from
prior CL where I had it on the chrome_client_impl).  This 
is a short lived cache, and not a complete solution.  It
fixes this case, where a JS script makes multiple operations
and expects the GetWindowSize() to be correct immedately
after having called SetWindowSize().  

BUG=http://crbug.com/835



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

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

3 files changed, 59 insertions, 3 deletions

diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 2b3c250..d0f0c74 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -1948,6 +1948,7 @@ void RenderView::Show(WebWidget* webwidget, WindowOpenDisposition disposition) {
   // browser process will impose a default position otherwise.
   Send(new ViewHostMsg_ShowView(opener_id_, routing_id_, disposition,
       initial_pos_, opened_by_user_gesture_, creator_url_));
+  SetPendingWindowRect(initial_pos_);
 }
 
 void RenderView::CloseWidgetSoon(WebWidget* webwidget) {
diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc
index 5bed201..7387665 100644
--- a/chrome/renderer/render_widget.cc
+++ b/chrome/renderer/render_widget.cc
@@ -54,7 +54,8 @@ RenderWidget::RenderWidget(RenderThreadBase* render_thread, bool activatable)
       ime_control_new_state_(false),
       ime_control_updated_(false),
       ime_control_busy_(false),
-      activatable_(activatable) {
+      activatable_(activatable),
+      pending_window_rect_count_(0) {
   RenderProcess::current()->AddRefProcess();
   DCHECK(render_thread_);
 }
@@ -129,6 +130,7 @@ IPC_DEFINE_MESSAGE_MAP(RenderWidget)
   IPC_MESSAGE_HANDLER(ViewMsg_ImeSetComposition, OnImeSetComposition)
   IPC_MESSAGE_HANDLER(ViewMsg_Repaint, OnMsgRepaint)
   IPC_MESSAGE_HANDLER(ViewMsg_SetTextDirection, OnSetTextDirection)
+  IPC_MESSAGE_HANDLER(ViewMsg_Move_ACK, OnRequestMoveAck)
   IPC_MESSAGE_UNHANDLED_ERROR()
 IPC_END_MESSAGE_MAP()
 
@@ -249,6 +251,11 @@ void RenderWidget::OnPaintRectAck() {
   DoDeferredPaint();
 }
 
+void RenderWidget::OnRequestMoveAck() {
+  DCHECK(pending_window_rect_count_);
+  pending_window_rect_count_--;
+}
+
 void RenderWidget::OnScrollRectAck() {
   DCHECK(scroll_reply_pending());
 
@@ -574,6 +581,7 @@ void RenderWidget::Show(WebWidget* webwidget,
     // process will impose a default position otherwise.
     render_thread_->Send(new ViewHostMsg_ShowWidget(
         opener_id_, routing_id_, initial_pos_));
+    SetPendingWindowRect(initial_pos_);
   }
 }
 
@@ -605,12 +613,21 @@ void RenderWidget::Blur(WebWidget* webwidget) {
   Send(new ViewHostMsg_Blur(routing_id_));
 }
 
+void RenderWidget::DoDeferredClose() {
+  Send(new ViewHostMsg_Close(routing_id_));
+}
+
 void RenderWidget::CloseWidgetSoon(WebWidget* webwidget) {
   // If a page calls window.close() twice, we'll end up here twice, but that's
   // OK.  It is safe to send multiple Close messages.
 
-  // Ask the RenderWidgetHost to initiate close.
-  Send(new ViewHostMsg_Close(routing_id_));
+  // Ask the RenderWidgetHost to initiate close.  We could be called from deep
+  // in Javascript.  If we ask the RendwerWidgetHost to close now, the window
+  // could be closed before the JS finishes executing.  So instead, post a
+  // message back to the message loop, which won't run until the JS is
+  // complete, and then the Close message can be sent.
+  MessageLoop::current()->PostNonNestableTask(FROM_HERE, NewRunnableMethod(
+      this, &RenderWidget::DoDeferredClose));
 }
 
 void RenderWidget::GenerateFullRepaint() {
@@ -625,6 +642,11 @@ void RenderWidget::Close() {
 }
 
 void RenderWidget::GetWindowRect(WebWidget* webwidget, WebRect* result) {
+  if (pending_window_rect_count_) {
+    *result = pending_window_rect_;
+    return;
+  }
+
   gfx::Rect rect;
   Send(new ViewHostMsg_GetWindowRect(routing_id_, host_window_, &rect));
   *result = rect;
@@ -633,12 +655,28 @@ void RenderWidget::GetWindowRect(WebWidget* webwidget, WebRect* result) {
 void RenderWidget::SetWindowRect(WebWidget* webwidget, const WebRect& pos) {
   if (did_show_) {
     Send(new ViewHostMsg_RequestMove(routing_id_, pos));
+    SetPendingWindowRect(pos);
   } else {
     initial_pos_ = pos;
   }
 }
 
+void RenderWidget::SetPendingWindowRect(const WebRect& rect) {
+  pending_window_rect_ = rect;
+  pending_window_rect_count_++;
+}
+
 void RenderWidget::GetRootWindowRect(WebWidget* webwidget, WebRect* result) {
+  if (pending_window_rect_count_) {
+    // NOTE(mbelshe): If there is a pending_window_rect_, then getting
+    // the RootWindowRect is probably going to return wrong results since the
+    // browser may not have processed the Move yet.  There isn't really anything
+    // good to do in this case, and it shouldn't happen - since this size is
+    // only really needed for windowToScreen, which is only used for Popups.
+    *result = pending_window_rect_;
+    return;
+  }
+
   gfx::Rect rect;
   Send(new ViewHostMsg_GetRootWindowRect(routing_id_, host_window_, &rect));
   *result = rect;
diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h
index 62777e8..660ccbb 100644
--- a/chrome/renderer/render_widget.h
+++ b/chrome/renderer/render_widget.h
@@ -18,6 +18,7 @@
 #include "skia/ext/platform_canvas.h"
 #include "skia/include/SkBitmap.h"
 
+#include "third_party/WebKit/WebKit/chromium/public/WebRect.h"
 #include "webkit/glue/webwidget_delegate.h"
 #include "webkit/glue/webcursor.h"
 
@@ -119,6 +120,8 @@ class RenderWidget : public IPC::Channel::Listener,
 
   void DoDeferredPaint();
   void DoDeferredScroll();
+  void DoDeferredClose();
+  void DoDeferredSetWindowRect(const WebKit::WebRect& pos);
 
   // This method is called immediately after PaintRect but before the
   // corresponding paint or scroll message is send to the widget host.
@@ -138,6 +141,7 @@ class RenderWidget : public IPC::Channel::Listener,
   void OnWasRestored(bool needs_repainting);
   void OnPaintRectAck();
   void OnScrollRectAck();
+  void OnRequestMoveAck();
   void OnHandleInputEvent(const IPC::Message& message);
   void OnMouseCaptureLost();
   void OnSetFocus(bool enable);
@@ -175,6 +179,14 @@ class RenderWidget : public IPC::Channel::Listener,
   // the focus on our own when the browser did not focus us.
   void ClearFocus();
 
+  // Set the pending window rect.
+  // Because the real render_widget is hosted in another process, there is
+  // a time period where we may have set a new window rect which has not yet
+  // been processed by the browser.  So we maintain a pending window rect
+  // size.  If JS code sets the WindowRect, and then immediately calls
+  // GetWindowRect() we'll use this pending window rect as the size.
+  void SetPendingWindowRect(const WebKit::WebRect& r);
+
   // Routing ID that allows us to communicate to the parent browser process
   // RenderWidgetHost. When MSG_ROUTING_NONE, no messages may be sent.
   int32 routing_id_;
@@ -280,6 +292,11 @@ class RenderWidget : public IPC::Channel::Listener,
   // A custom background for the widget.
   SkBitmap background_;
 
+  // While we are waiting for the browser to update window sizes,
+  // we track the pending size temporarily.
+  int pending_window_rect_count_;
+  WebKit::WebRect pending_window_rect_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderWidget);
 };