Modal loops of joy.

We need to pump messages in the renderer and in any related plugin processes
when sending sync IPCs to get cookies and set localstorage items because these
IPCs can block on a user prompt.

This code moves complexity into RenderThread::Send().  That function is now
responsible for calling WebView::{willEnter,didExit}ModalLoop() and for
broadcasting the messages to signal/reset modal dialog events in the related
plugin processes.

This change also adds code to optionally suspend the shared webkit timer.  This
was needed to fix bug 36063.  It also helps make spinning a nested loop to wait
for document.cookie results a bit less risky since we'll be re-entering WebKit
in fewer cases.

R=jam
BUG=34917,36063
TEST=none

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

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

9 files changed, 195 insertions, 60 deletions

diff --git a/chrome/renderer/print_web_view_helper_mac.mm b/chrome/renderer/print_web_view_helper_mac.mm
index 6e2cc2d..96113fb 100644
--- a/chrome/renderer/print_web_view_helper_mac.mm
+++ b/chrome/renderer/print_web_view_helper_mac.mm
@@ -102,8 +102,10 @@ void PrintWebViewHelper::Print(WebFrame* frame, bool script_initiated) {
       params.has_selection = frame->hasSelection();
       params.expected_pages_count = expected_pages_count;
 
-      msg = new ViewHostMsg_ScriptedPrint(params, &print_settings);
-      if (render_view_->SendAndRunNestedMessageLoop(msg)) {
+      msg = new ViewHostMsg_ScriptedPrint(routing_id(), params,
+                                          &print_settings);
+      msg->EnableMessagePumping();
+      if (Send(msg)) {
         msg = NULL;
 
         // If the settings are invalid, early quit.
diff --git a/chrome/renderer/print_web_view_helper_win.cc b/chrome/renderer/print_web_view_helper_win.cc
index 6ba70a3..b75aa10 100644
--- a/chrome/renderer/print_web_view_helper_win.cc
+++ b/chrome/renderer/print_web_view_helper_win.cc
@@ -102,8 +102,10 @@ void PrintWebViewHelper::Print(WebFrame* frame, bool script_initiated) {
       params.has_selection = false;
       params.expected_pages_count = expected_pages_count;
 
-      msg = new ViewHostMsg_ScriptedPrint(params, &print_settings);
-      if (render_view_->SendAndRunNestedMessageLoop(msg)) {
+      msg = new ViewHostMsg_ScriptedPrint(routing_id(), params,
+                                          &print_settings);
+      msg->EnableMessagePumping();
+      if (Send(msg)) {
         msg = NULL;
 
         // If the settings are invalid, early quit.
diff --git a/chrome/renderer/render_thread.cc b/chrome/renderer/render_thread.cc
index 7914507..26f4508 100644
--- a/chrome/renderer/render_thread.cc
+++ b/chrome/renderer/render_thread.cc
@@ -29,6 +29,7 @@
 #include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/db_message_filter.h"
+#include "chrome/common/plugin_messages.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/renderer_preferences.h"
 #include "chrome/common/url_constants.h"
@@ -49,6 +50,7 @@
 #include "chrome/renderer/external_extension.h"
 #include "chrome/renderer/loadtimes_extension_bindings.h"
 #include "chrome/renderer/net/render_dns_master.h"
+#include "chrome/renderer/plugin_channel_host.h"
 #include "chrome/renderer/render_process.h"
 #include "chrome/renderer/render_view.h"
 #include "chrome/renderer/render_view_visitor.h"
@@ -206,6 +208,9 @@ void RenderThread::Init() {
   std::string type_str = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
       switches::kProcessType);
   is_extension_process_ = type_str == switches::kExtensionProcess;
+  do_not_suspend_webkit_shared_timer_ = false;
+  do_not_notify_webkit_of_modal_loop_ = false;
+  did_notify_webkit_of_modal_loop_ = false;
   plugin_refresh_allowed_ = true;
   cache_stats_task_pending_ = false;
   widget_count_ = 0;
@@ -260,6 +265,90 @@ RenderThread* RenderThread::current() {
   return lazy_tls.Pointer()->Get();
 }
 
+int32 RenderThread::RoutingIDForCurrentContext() {
+  int32 routing_id = MSG_ROUTING_CONTROL;
+  if (v8::Context::InContext()) {
+    RenderView* view =
+        RenderView::FromWebView(WebFrame::frameForCurrentContext()->view());
+    DCHECK(view);
+    routing_id = view->routing_id();
+  } else {
+    DLOG(WARNING) << "Not called within a script context!";
+  }
+  return routing_id;
+}
+
+bool RenderThread::Send(IPC::Message* msg) {
+  gfx::NativeViewId host_window = 0;
+  bool pumping_events = false;
+
+  // Inform related plugins when they also need to pump events.
+  if (msg->is_sync() && msg->is_caller_pumping_messages()) {
+    pumping_events = true;
+    RenderWidget* widget =
+        static_cast<RenderWidget*>(ResolveRoute(msg->routing_id()));
+    if (widget)
+      host_window = widget->host_window();
+  }
+
+  bool do_not_suspend_webkit_shared_timer = false;
+  std::swap(do_not_suspend_webkit_shared_timer,
+            do_not_suspend_webkit_shared_timer_);
+
+  bool do_not_notify_webkit_of_modal_loop = false;
+  std::swap(do_not_notify_webkit_of_modal_loop,
+            do_not_notify_webkit_of_modal_loop_);
+
+  if (pumping_events) {
+    if (!do_not_suspend_webkit_shared_timer)
+      webkit_client_->SuspendSharedTimer();
+
+    // WebKit does not like nested calls to willEnterModalLoop.
+    // TODO(darin): Fix WebKit to allow nesting.
+    if (!do_not_notify_webkit_of_modal_loop &&
+        !did_notify_webkit_of_modal_loop_) {
+      WebView::willEnterModalLoop();
+      did_notify_webkit_of_modal_loop_ = true;
+    }
+
+    if (host_window) {
+      PluginChannelHost::Broadcast(
+          new PluginMsg_SignalModalDialogEvent(host_window));
+    }
+  }
+
+  bool rv = ChildThread::Send(msg);
+
+  if (pumping_events) {
+    if (host_window) {
+      PluginChannelHost::Broadcast(
+          new PluginMsg_ResetModalDialogEvent(host_window));
+    }
+
+    if (!do_not_notify_webkit_of_modal_loop &&
+        did_notify_webkit_of_modal_loop_) {
+      WebView::didExitModalLoop();
+      did_notify_webkit_of_modal_loop_ = false;
+    }
+
+    if (!do_not_suspend_webkit_shared_timer)
+      webkit_client_->ResumeSharedTimer();
+  }
+
+  return rv;
+}
+
+void RenderThread::AddRoute(int32 routing_id,
+                            IPC::Channel::Listener* listener) {
+  widget_count_++;
+  return ChildThread::AddRoute(routing_id, listener);
+}
+
+void RenderThread::RemoveRoute(int32 routing_id) {
+  widget_count_--;
+  return ChildThread::RemoveRoute(routing_id);
+}
+
 void RenderThread::AddFilter(IPC::ChannelProxy::MessageFilter* filter) {
   channel()->AddFilter(filter);
 }
@@ -283,6 +372,19 @@ void RenderThread::WidgetRestored() {
     idle_timer_.Stop();
 }
 
+bool RenderThread::SendAndRunNestedMessageLoop(IPC::SyncMessage* message) {
+  message->EnableMessagePumping();
+  return Send(message);
+}
+
+void RenderThread::DoNotSuspendWebKitSharedTimer() {
+  do_not_suspend_webkit_shared_timer_ = true;
+}
+
+void RenderThread::DoNotNotifyWebKitOfModalLoop() {
+  do_not_notify_webkit_of_modal_loop_ = true;
+}
+
 void RenderThread::Resolve(const char* name, size_t length) {
   return dns_master_->Resolve(name, length);
 }
diff --git a/chrome/renderer/render_thread.h b/chrome/renderer/render_thread.h
index 0336ba8..416b71b 100644
--- a/chrome/renderer/render_thread.h
+++ b/chrome/renderer/render_thread.h
@@ -92,26 +92,35 @@ class RenderThread : public RenderThreadBase,
   // be accessed when running on the render thread itself
   static RenderThread* current();
 
-  // Overridden from RenderThreadBase.
-  virtual bool Send(IPC::Message* msg) {
-    return ChildThread::Send(msg);
-  }
-
-  virtual void AddRoute(int32 routing_id, IPC::Channel::Listener* listener) {
-    widget_count_++;
-    return ChildThread::AddRoute(routing_id, listener);
-  }
-  virtual void RemoveRoute(int32 routing_id) {
-    widget_count_--;
-    return ChildThread::RemoveRoute(routing_id);
-  }
+  // Returns the routing ID of the RenderWidget containing the current script
+  // execution context (corresponding to WebFrame::frameForCurrentContext).
+  static int32 RoutingIDForCurrentContext();
 
+  // Overridden from RenderThreadBase.
+  virtual bool Send(IPC::Message* msg);
+  virtual void AddRoute(int32 routing_id, IPC::Channel::Listener* listener);
+  virtual void RemoveRoute(int32 routing_id);
   virtual void AddFilter(IPC::ChannelProxy::MessageFilter* filter);
   virtual void RemoveFilter(IPC::ChannelProxy::MessageFilter* filter);
-
   virtual void WidgetHidden();
   virtual void WidgetRestored();
 
+  // Send a synchronous message and run a nested message loop, while waiting
+  // for a reply.
+  //
+  // NOTE: Only use this method if the handler for the message may need to show
+  // UI before replying.
+  //
+  bool SendAndRunNestedMessageLoop(IPC::SyncMessage* message);
+
+  // These methods modify how the next message is sent.  Normally, when sending
+  // a synchronous message that runs a nested message loop, we need to suspend
+  // callbacks into WebKit.  This involves disabling timers and deferring
+  // resource loads.  However, there are exceptions when we need to customize
+  // the behavior.
+  void DoNotSuspendWebKitSharedTimer();
+  void DoNotNotifyWebKitOfModalLoop();
+
   VisitedLinkSlave* visited_link_slave() const {
     return visited_link_slave_.get();
   }
@@ -255,6 +264,10 @@ class RenderThread : public RenderThreadBase,
   // True if this renderer is running extensions.
   bool is_extension_process_;
 
+  bool do_not_suspend_webkit_shared_timer_;
+  bool do_not_notify_webkit_of_modal_loop_;
+  bool did_notify_webkit_of_modal_loop_;
+
   // Timer that periodically calls IdleHandler.
   base::RepeatingTimer<RenderThread> idle_timer_;
 
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 7a5fad2..aab3445 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -32,7 +32,6 @@
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/jstemplate_builder.h"
 #include "chrome/common/page_zoom.h"
-#include "chrome/common/plugin_messages.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/renderer_preferences.h"
 #include "chrome/common/thumbnail_score.h"
@@ -1407,6 +1406,18 @@ bool RenderView::RunJavaScriptMessage(int type,
   return success;
 }
 
+bool RenderView::SendAndRunNestedMessageLoop(IPC::SyncMessage* message) {
+  // Before WebKit asks us to show an alert (etc.), it takes care of doing the
+  // equivalent of WebView::willEnterModalLoop.  In the case of showModalDialog
+  // it is particularly important that we do not call willEnterModalLoop as
+  // that would defer resource loads for the dialog itself.
+  if (RenderThread::current())  // Will be NULL during unit tests.
+    RenderThread::current()->DoNotNotifyWebKitOfModalLoop();
+
+  message->EnableMessagePumping();  // Runs a nested message loop.
+  return Send(message);
+}
+
 void RenderView::AddGURLSearchProvider(const GURL& osd_url, bool autodetected) {
   if (!osd_url.is_empty())
     Send(new ViewHostMsg_PageHasOSDD(routing_id_, page_id_, osd_url,
@@ -1989,6 +2000,16 @@ void RenderView::closeWidgetSoon() {
 void RenderView::runModal() {
   DCHECK(did_show_) << "should already have shown the view";
 
+  // We must keep WebKit's shared timer running in this case in order to allow
+  // showModalDialog to function properly.
+  //
+  // TODO(darin): WebKit should really be smarter about suppressing events and
+  // timers so that we do not need to manage the shared timer in such a heavy
+  // handed manner.
+  //
+  if (RenderThread::current())  // Will be NULL during unit tests.
+    RenderThread::current()->DoNotSuspendWebKitSharedTimer();
+
   SendAndRunNestedMessageLoop(new ViewHostMsg_RunModal(routing_id_));
 }
 
@@ -4451,19 +4472,6 @@ void RenderView::EnsureDocumentTag() {
 #endif
 }
 
-bool RenderView::SendAndRunNestedMessageLoop(IPC::SyncMessage* message) {
-  PluginChannelHost::Broadcast(
-      new PluginMsg_SignalModalDialogEvent(host_window_));
-
-  message->EnableMessagePumping();  // Runs a nested message loop.
-  bool rv = Send(message);
-
-  PluginChannelHost::Broadcast(
-      new PluginMsg_ResetModalDialogEvent(host_window_));
-
-  return rv;
-}
-
 #if defined(OS_MACOSX)
 gfx::PluginWindowHandle RenderView::AllocateFakePluginWindowHandle() {
   gfx::PluginWindowHandle window = NULL;
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index f9ab206..60deea6 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -157,10 +157,6 @@ class RenderView : public RenderWidget,
     return static_cast<WebKit::WebView*>(webwidget());
   }
 
-  gfx::NativeViewId host_window() const {
-    return host_window_;
-  }
-
   int browser_window_id() const {
     return browser_window_id_;
   }
@@ -450,9 +446,6 @@ class RenderView : public RenderWidget,
     return webkit_preferences_;
   }
 
-  // Sends a message and runs a nested message loop.
-  bool SendAndRunNestedMessageLoop(IPC::SyncMessage* message);
-
   // Called when the "idle" user script state has been reached. See
   // UserScript::DOCUMENT_IDLE.
   void OnUserScriptIdleTriggered(WebKit::WebFrame* frame);
@@ -573,6 +566,9 @@ class RenderView : public RenderWidget,
                             const GURL& frame_url,
                             std::wstring* result);
 
+  // Sends a message and runs a nested message loop.
+  bool SendAndRunNestedMessageLoop(IPC::SyncMessage* message);
+
   // Adds search provider from the given OpenSearch description URL as a
   // keyword search.
   void AddGURLSearchProvider(const GURL& osd_url, bool autodetected);
diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h
index 27086a4..d8aca5b 100644
--- a/chrome/renderer/render_widget.h
+++ b/chrome/renderer/render_widget.h
@@ -66,6 +66,10 @@ class RenderWidget : public IPC::Channel::Listener,
     return webwidget_;
   }
 
+  gfx::NativeViewId host_window() const {
+    return host_window_;
+  }
+
   // IPC::Channel::Listener
   virtual void OnMessageReceived(const IPC::Message& msg);
 
diff --git a/chrome/renderer/renderer_webkitclient_impl.cc b/chrome/renderer/renderer_webkitclient_impl.cc
index 504d8f3..a475616 100644
--- a/chrome/renderer/renderer_webkitclient_impl.cc
+++ b/chrome/renderer/renderer_webkitclient_impl.cc
@@ -116,13 +116,7 @@ void RendererWebKitClientImpl::setCookies(const WebURL& url,
                                           const WebString& value) {
   // TODO(darin): Modify WebKit to pass the WebFrame.  This code may be reached
   // when there is no active script context.
-  int routing_id = MSG_ROUTING_CONTROL;
-  if (v8::Context::InContext()) {
-    RenderView* view =
-        RenderView::FromWebView(WebFrame::frameForCurrentContext()->view());
-    DCHECK(view);
-    routing_id = view->routing_id();
-  }
+  int32 routing_id = RenderThread::RoutingIDForCurrentContext();
 
   std::string value_utf8;
   UTF16ToUTF8(value.data(), value.length(), &value_utf8);
@@ -133,9 +127,15 @@ void RendererWebKitClientImpl::setCookies(const WebURL& url,
 
 WebString RendererWebKitClientImpl::cookies(
     const WebURL& url, const WebURL& first_party_for_cookies) {
+  // TODO(darin): Modify WebKit to pass the WebFrame.  This code may be reached
+  // when there is no active script context.
+  int32 routing_id = RenderThread::RoutingIDForCurrentContext();
+
   std::string value_utf8;
-  RenderThread::current()->Send(
-      new ViewHostMsg_GetCookies(url, first_party_for_cookies, &value_utf8));
+  RenderThread::current()->SendAndRunNestedMessageLoop(
+      new ViewHostMsg_GetCookies(routing_id, url, first_party_for_cookies,
+                                 &value_utf8));
+
   return WebString::fromUTF8(value_utf8);
 }
 
@@ -143,9 +143,15 @@ bool RendererWebKitClientImpl::rawCookies(
     const WebURL& url,
     const WebURL& first_party_for_cookies,
     WebVector<WebKit::WebCookie>* raw_cookies) {
+  // TODO(darin): Modify WebKit to pass the WebFrame.  This code may be reached
+  // when there is no active script context.
+  int32 routing_id = RenderThread::RoutingIDForCurrentContext();
+
   std::vector<webkit_glue::WebCookie> cookies;
-  RenderThread::current()->Send(
-      new ViewHostMsg_GetRawCookies(url, first_party_for_cookies, &cookies));
+  RenderThread::current()->SendAndRunNestedMessageLoop(
+      new ViewHostMsg_GetRawCookies(routing_id, url, first_party_for_cookies,
+                                    &cookies));
+
 
   WebVector<WebKit::WebCookie> result(cookies.size());
   int i = 0;
diff --git a/chrome/renderer/renderer_webstoragearea_impl.cc b/chrome/renderer/renderer_webstoragearea_impl.cc
index 20ce740..e1a9fb9 100644
--- a/chrome/renderer/renderer_webstoragearea_impl.cc
+++ b/chrome/renderer/renderer_webstoragearea_impl.cc
@@ -1,6 +1,6 @@
-// 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.
+// Copyright (c) 2010 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/renderer/renderer_webstoragearea_impl.h"
 
@@ -9,10 +9,12 @@
 #include "chrome/renderer/render_view.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebFrame.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebURL.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebView.h"
 
 using WebKit::WebFrame;
 using WebKit::WebString;
 using WebKit::WebURL;
+using WebKit::WebView;
 
 RendererWebStorageAreaImpl::RendererWebStorageAreaImpl(
     int64 namespace_id, const WebString& origin) {
@@ -48,18 +50,18 @@ WebString RendererWebStorageAreaImpl::getItem(const WebString& key) {
 void RendererWebStorageAreaImpl::setItem(
     const WebString& key, const WebString& value, const WebURL& url,
     WebStorageArea::Result& result, WebString& old_value_webkit) {
+  int32 routing_id = RenderThread::RoutingIDForCurrentContext();
+  CHECK(routing_id != MSG_ROUTING_CONTROL);
+
   NullableString16 old_value;
-  RenderThread::current()->Send(
-      new ViewHostMsg_DOMStorageSetItem(storage_area_id_, key, value, url,
-                                        &result, &old_value));
+  RenderThread::current()->SendAndRunNestedMessageLoop(
+      new ViewHostMsg_DOMStorageSetItem(routing_id, storage_area_id_, key,
+                                        value, url, &result, &old_value));
   old_value_webkit = old_value;
 
   if (result == WebStorageArea::ResultBlockedByPolicy) {
-    RenderView* view =
-        RenderView::FromWebView(WebFrame::frameForCurrentContext()->view());
-    DCHECK(view);
     RenderThread::current()->Send(new ViewHostMsg_ContentBlocked(
-        view->routing_id(), CONTENT_SETTINGS_TYPE_COOKIES));
+        routing_id, CONTENT_SETTINGS_TYPE_COOKIES));
   }
 }