Synchronous XHR should only pump events if we are going to show a cookie

prompt.

Also, LocalStorage.setItem needs to reset the CookieMessageFilter's
pump_messages_event after it is done with sending its IPC.

To support both of these fixes, the contents of RendererWebCookieJarImpl's
SendSynchronousMessage is merged into RenderThread::Send.

R=jam
BUG=37571
TEST=none

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

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

6 files changed, 65 insertions, 59 deletions

diff --git a/chrome/common/resource_dispatcher.cc b/chrome/common/resource_dispatcher.cc
index de1c239..d5fcd89 100644
--- a/chrome/common/resource_dispatcher.cc
+++ b/chrome/common/resource_dispatcher.cc
@@ -225,7 +225,7 @@ void IPCResourceLoaderBridge::SyncLoad(SyncLoadResponse* response) {
   SyncLoadResult result;
   IPC::SyncMessage* msg = new ViewHostMsg_SyncLoad(routing_id_, request_id_,
                                                    request_, &result);
-  msg->EnableMessagePumping();
+  // NOTE: This may pump events (see RenderThread::Send).
   if (!dispatcher_->message_sender()->Send(msg)) {
     response->status.set_status(URLRequestStatus::FAILED);
     return;
diff --git a/chrome/renderer/render_thread.cc b/chrome/renderer/render_thread.cc
index 05f4c7b..7ad062e 100644
--- a/chrome/renderer/render_thread.cc
+++ b/chrome/renderer/render_thread.cc
@@ -210,8 +210,8 @@ 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;
+  suspend_webkit_shared_timer_ = true;
+  notify_webkit_of_modal_loop_ = true;
   did_notify_webkit_of_modal_loop_ = false;
   plugin_refresh_allowed_ = true;
   cache_stats_task_pending_ = false;
@@ -288,39 +288,61 @@ int32 RenderThread::RoutingIDForCurrentContext() {
 }
 
 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();
+  // Certain synchronous messages can result in an app-modal cookie prompt.
+  // This could cause a complete hang of Chrome if a windowed plug-in is trying
+  // to communicate with the renderer thread since the browser's UI thread
+  // could be stuck (within a Windows API call) trying to synchronously
+  // communicate with the plug-in.  The remedy is to pump messages on this
+  // thread while the cookie prompt is showing.  This creates an opportunity
+  // for re-entrancy into WebKit, so we need to take care to disable callbacks,
+  // timers, and pending network loads that could trigger such callbacks.
+
+  bool pumping_events = false, may_show_cookie_prompt = false;
+  if (msg->is_sync()) {
+    if (msg->is_caller_pumping_messages()) {
+      pumping_events = true;
+    } else {
+      switch (msg->type()) {
+        case ViewHostMsg_GetCookies::ID:
+        case ViewHostMsg_GetRawCookies::ID:
+        case ViewHostMsg_DOMStorageSetItem::ID:
+        case ViewHostMsg_SyncLoad::ID:
+          may_show_cookie_prompt = true;
+          pumping_events = true;
+          break;
+      }
+    }
   }
 
-  bool do_not_suspend_webkit_shared_timer = false;
-  std::swap(do_not_suspend_webkit_shared_timer,
-            do_not_suspend_webkit_shared_timer_);
+  bool suspend_webkit_shared_timer = true;  // default value
+  std::swap(suspend_webkit_shared_timer, 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_);
+  bool notify_webkit_of_modal_loop = true;  // default value
+  std::swap(notify_webkit_of_modal_loop, notify_webkit_of_modal_loop_);
+
+  gfx::NativeViewId host_window = 0;
 
   if (pumping_events) {
-    if (!do_not_suspend_webkit_shared_timer)
+    // See ViewMsg_SignalCookiePromptEvent.
+    if (may_show_cookie_prompt) {
+      static_cast<IPC::SyncMessage*>(msg)->set_pump_messages_event(
+          cookie_message_filter_->pump_messages_event());
+    }
+
+    if (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_) {
+    if (notify_webkit_of_modal_loop && !did_notify_webkit_of_modal_loop_) {
       WebView::willEnterModalLoop();
       did_notify_webkit_of_modal_loop_ = true;
     }
 
-    if (host_window) {
+    RenderWidget* widget =
+        static_cast<RenderWidget*>(ResolveRoute(msg->routing_id()));
+    if (widget) {
+      host_window = widget->host_window();
       PluginChannelHost::Broadcast(
           new PluginMsg_SignalModalDialogEvent(host_window));
     }
@@ -334,14 +356,22 @@ bool RenderThread::Send(IPC::Message* msg) {
           new PluginMsg_ResetModalDialogEvent(host_window));
     }
 
-    if (!do_not_notify_webkit_of_modal_loop &&
-        did_notify_webkit_of_modal_loop_) {
+    if (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)
+    if (suspend_webkit_shared_timer)
       webkit_client_->ResumeSharedTimer();
+
+    // We may end up nesting calls to Send, so we defer the reset until we
+    // return to the top-most message loop.
+    if (may_show_cookie_prompt &&
+        cookie_message_filter_->pump_messages_event()->IsSignaled()) {
+      MessageLoop::current()->PostNonNestableTask(FROM_HERE,
+          NewRunnableMethod(cookie_message_filter_.get(),
+                            &CookieMessageFilter::ResetPumpMessagesEvent));
+    }
   }
 
   return rv;
@@ -382,11 +412,11 @@ void RenderThread::WidgetRestored() {
 }
 
 void RenderThread::DoNotSuspendWebKitSharedTimer() {
-  do_not_suspend_webkit_shared_timer_ = true;
+  suspend_webkit_shared_timer_ = false;
 }
 
 void RenderThread::DoNotNotifyWebKitOfModalLoop() {
-  do_not_notify_webkit_of_modal_loop_ = true;
+  notify_webkit_of_modal_loop_ = false;
 }
 
 void RenderThread::Resolve(const char* name, size_t length) {
diff --git a/chrome/renderer/render_thread.h b/chrome/renderer/render_thread.h
index 2aaf768..51d742e 100644
--- a/chrome/renderer/render_thread.h
+++ b/chrome/renderer/render_thread.h
@@ -140,10 +140,6 @@ class RenderThread : public RenderThreadBase,
     return spellchecker_.get();
   }
 
-  CookieMessageFilter* cookie_message_filter() const {
-    return cookie_message_filter_.get();
-  }
-
   bool plugin_refresh_allowed() const { return plugin_refresh_allowed_; }
 
   bool is_extension_process() const { return is_extension_process_; }
@@ -275,8 +271,8 @@ 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 suspend_webkit_shared_timer_;
+  bool notify_webkit_of_modal_loop_;
   bool did_notify_webkit_of_modal_loop_;
 
   // Timer that periodically calls IdleHandler.
diff --git a/chrome/renderer/renderer_webcookiejar_impl.cc b/chrome/renderer/renderer_webcookiejar_impl.cc
index 9e9263a..53a7794 100644
--- a/chrome/renderer/renderer_webcookiejar_impl.cc
+++ b/chrome/renderer/renderer_webcookiejar_impl.cc
@@ -5,7 +5,6 @@
 #include "chrome/renderer/renderer_webcookiejar_impl.h"
 
 #include "chrome/common/render_messages.h"
-#include "chrome/renderer/cookie_message_filter.h"
 #include "chrome/renderer/render_thread.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebCookie.h"
 
@@ -14,23 +13,6 @@ using WebKit::WebString;
 using WebKit::WebURL;
 using WebKit::WebVector;
 
-void RendererWebCookieJarImpl::SendSynchronousMessage(
-    IPC::SyncMessage* message) {
-  CookieMessageFilter* filter =
-      RenderThread::current()->cookie_message_filter();
-
-  message->set_pump_messages_event(filter->pump_messages_event());
-  sender_->Send(message);
-
-  // We may end up nesting calls to SendCookieMessage, so we defer the reset
-  // until we return to the top-most message loop.
-  if (filter->pump_messages_event()->IsSignaled()) {
-    MessageLoop::current()->PostNonNestableTask(FROM_HERE,
-        NewRunnableMethod(filter,
-                          &CookieMessageFilter::ResetPumpMessagesEvent));
-  }
-}
-
 void RendererWebCookieJarImpl::setCookie(
     const WebURL& url, const WebURL& first_party_for_cookies,
     const WebString& value) {
@@ -43,7 +25,8 @@ void RendererWebCookieJarImpl::setCookie(
 WebString RendererWebCookieJarImpl::cookies(
     const WebURL& url, const WebURL& first_party_for_cookies) {
   std::string value_utf8;
-  SendSynchronousMessage(new ViewHostMsg_GetCookies(
+  // NOTE: This may pump events (see RenderThread::Send).
+  sender_->Send(new ViewHostMsg_GetCookies(
       MSG_ROUTING_NONE, url, first_party_for_cookies, &value_utf8));
   return WebString::fromUTF8(value_utf8);
 }
@@ -57,7 +40,8 @@ void RendererWebCookieJarImpl::rawCookies(
     const WebURL& url, const WebURL& first_party_for_cookies,
     WebVector<WebCookie>& raw_cookies) {
   std::vector<webkit_glue::WebCookie> cookies;
-  SendSynchronousMessage(new ViewHostMsg_GetRawCookies(
+  // NOTE: This may pump events (see RenderThread::Send).
+  sender_->Send(new ViewHostMsg_GetRawCookies(
       MSG_ROUTING_NONE, url, first_party_for_cookies, &cookies));
 
   WebVector<WebCookie> result(cookies.size());
diff --git a/chrome/renderer/renderer_webcookiejar_impl.h b/chrome/renderer/renderer_webcookiejar_impl.h
index e4f113a..0b1ed355 100644
--- a/chrome/renderer/renderer_webcookiejar_impl.h
+++ b/chrome/renderer/renderer_webcookiejar_impl.h
@@ -21,8 +21,6 @@ class RendererWebCookieJarImpl : public WebKit::WebCookieJar {
   }
 
  private:
-  void SendSynchronousMessage(IPC::SyncMessage* message);
-
   // WebKit::WebCookieJar methods:
   virtual void setCookie(
       const WebKit::WebURL& url, const WebKit::WebURL& first_party_for_cookies,
diff --git a/chrome/renderer/renderer_webstoragearea_impl.cc b/chrome/renderer/renderer_webstoragearea_impl.cc
index 61c3448..d2d13dc 100644
--- a/chrome/renderer/renderer_webstoragearea_impl.cc
+++ b/chrome/renderer/renderer_webstoragearea_impl.cc
@@ -5,7 +5,6 @@
 #include "chrome/renderer/renderer_webstoragearea_impl.h"
 
 #include "chrome/common/render_messages.h"
-#include "chrome/renderer/cookie_message_filter.h"
 #include "chrome/renderer/render_thread.h"
 #include "chrome/renderer/render_view.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebFrame.h"
@@ -58,8 +57,7 @@ void RendererWebStorageAreaImpl::setItem(
   IPC::SyncMessage* message =
       new ViewHostMsg_DOMStorageSetItem(routing_id, storage_area_id_, key,
                                         value, url, &result, &old_value);
-  message->set_pump_messages_event(
-      RenderThread::current()->cookie_message_filter()->pump_messages_event());
+  // NOTE: This may pump events (see RenderThread::Send).
   RenderThread::current()->Send(message);
   old_value_webkit = old_value;