Only pump messages during a cookie query if it results in a user prompt.

Note: I left LocalStorage intact.  I want to first confirm that this CL
resolves the Intl2 performance regression.  If so, then I'll follow-up
with a CL to add similar treatment for LocalStorage.

R=jam
BUG=36310
TEST=none

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

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

10 files changed, 109 insertions, 24 deletions

diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 2460db3..70ab6e9 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -629,8 +629,10 @@ void ResourceMessageFilter::OnGetCookies(const GURL& url,
   if (context->cookie_policy()) {
     policy = context->cookie_policy()->CanGetCookies(
         url, first_party_for_cookies, callback);
-    if (policy == net::ERR_IO_PENDING)
+    if (policy == net::ERR_IO_PENDING) {
+      Send(new ViewMsg_SignalCookiePromptEvent());
       return;
+    }
   }
   callback->Run(policy);
 }
@@ -660,8 +662,10 @@ void ResourceMessageFilter::OnGetRawCookies(
   if (context->cookie_policy()) {
     policy = context->cookie_policy()->CanGetCookies(
        url, first_party_for_cookies, callback);
-    if (policy == net::ERR_IO_PENDING)
+    if (policy == net::ERR_IO_PENDING) {
+      Send(new ViewMsg_SignalCookiePromptEvent());
       return;
+    }
   }
   callback->Run(policy);
 }
diff --git a/chrome/chrome_renderer.gypi b/chrome/chrome_renderer.gypi
index 125c0fa..484653a 100755
--- a/chrome/chrome_renderer.gypi
+++ b/chrome/chrome_renderer.gypi
@@ -68,6 +68,8 @@
         'renderer/about_handler.h',
         'renderer/audio_message_filter.cc',
         'renderer/audio_message_filter.h',
+        'renderer/cookie_message_filter.cc',
+        'renderer/cookie_message_filter.h',
         'renderer/devtools_agent.cc',
         'renderer/devtools_agent.h',
         'renderer/devtools_agent_filter.cc',
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index 8b4020b..d9cee06 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -299,6 +299,11 @@ IPC_BEGIN_MESSAGES(View)
                       URLRequestStatus /* status */,
                       std::string /* security info */)
 
+  // Sent when user prompting is required before a ViewHostMsg_GetCookies
+  // message can complete.  This message indicates that the renderer should
+  // pump messages while waiting for cookies.
+  IPC_MESSAGE_CONTROL0(ViewMsg_SignalCookiePromptEvent)
+
   // Request for the renderer to evaluate an xpath to a frame and execute a
   // javascript: url in that frame's context. The message is completely
   // asynchronous and no corresponding response message is sent back.
diff --git a/chrome/renderer/cookie_message_filter.cc b/chrome/renderer/cookie_message_filter.cc
new file mode 100644
index 0000000..6772f84
--- /dev/null
+++ b/chrome/renderer/cookie_message_filter.cc
@@ -0,0 +1,19 @@
+// 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/cookie_message_filter.h"
+
+#include "chrome/common/render_messages.h"
+
+CookieMessageFilter::CookieMessageFilter()
+    : event_(true, false) {
+}
+
+bool CookieMessageFilter::OnMessageReceived(const IPC::Message& message) {
+  if (message.type() == ViewMsg_SignalCookiePromptEvent::ID) {
+    event_.Signal();
+    return true;
+  }
+  return false;
+}
diff --git a/chrome/renderer/cookie_message_filter.h b/chrome/renderer/cookie_message_filter.h
new file mode 100644
index 0000000..92b48b7
--- /dev/null
+++ b/chrome/renderer/cookie_message_filter.h
@@ -0,0 +1,29 @@
+// 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.
+
+#ifndef CHROME_RENDERER_COOKIE_MESSAGE_FILTER_H_
+#define CHROME_RENDERER_COOKIE_MESSAGE_FILTER_H_
+
+#include "base/waitable_event.h"
+#include "ipc/ipc_channel_proxy.h"
+
+// This class maintains a WaitableEvent that is signaled when an IPC to query
+// cookies from the browser should pump events.  Pumping events may be
+// necessary to avoid deadlocks if the browser blocks the cookie query on a
+// user prompt.
+class CookieMessageFilter : public IPC::ChannelProxy::MessageFilter {
+ public:
+  CookieMessageFilter();
+
+  base::WaitableEvent* pump_messages_event() { return &event_; }
+  void ResetPumpMessagesEvent() { event_.Reset(); }
+
+ private:
+  // IPC::ChannelProxy::MessageFilter implementation:
+  virtual bool OnMessageReceived(const IPC::Message& message);
+
+  base::WaitableEvent event_;
+};
+
+#endif  // CHROME_RENDERER_COOKIE_MESSAGE_FILTER_H_
diff --git a/chrome/renderer/render_thread.cc b/chrome/renderer/render_thread.cc
index 577ba9b..cc622b1 100644
--- a/chrome/renderer/render_thread.cc
+++ b/chrome/renderer/render_thread.cc
@@ -41,6 +41,7 @@
 #include "base/scoped_handle.h"
 #include "chrome/plugin/plugin_channel_base.h"
 #endif
+#include "chrome/renderer/cookie_message_filter.h"
 #include "chrome/renderer/devtools_agent_filter.h"
 #include "chrome/renderer/extension_groups.h"
 #include "chrome/renderer/extensions/event_bindings.h"
@@ -224,11 +225,16 @@ void RenderThread::Init() {
   dns_master_.reset(new RenderDnsMaster());
   histogram_snapshots_.reset(new RendererHistogramSnapshots());
   appcache_dispatcher_.reset(new AppCacheDispatcher(this));
+  spellchecker_.reset(new SpellCheck());
+
   devtools_agent_filter_ = new DevToolsAgentFilter();
   AddFilter(devtools_agent_filter_.get());
+
   db_message_filter_ = new DBMessageFilter();
   AddFilter(db_message_filter_.get());
-  spellchecker_.reset(new SpellCheck());
+
+  cookie_message_filter_ = new CookieMessageFilter();
+  AddFilter(cookie_message_filter_.get());
 
 #if defined(OS_POSIX)
   suicide_on_channel_error_filter_ = new SuicideOnChannelErrorFilter;
@@ -372,11 +378,6 @@ 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;
 }
diff --git a/chrome/renderer/render_thread.h b/chrome/renderer/render_thread.h
index 416b71b..fb99d74 100644
--- a/chrome/renderer/render_thread.h
+++ b/chrome/renderer/render_thread.h
@@ -24,6 +24,7 @@
 #include "ipc/ipc_platform_file.h"
 
 class AppCacheDispatcher;
+class CookieMessageFilter;
 class DBMessageFilter;
 class DevToolsAgentFilter;
 class FilePath;
@@ -105,14 +106,6 @@ class RenderThread : public RenderThreadBase,
   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
@@ -137,6 +130,10 @@ 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_; }
@@ -240,6 +237,7 @@ class RenderThread : public RenderThreadBase,
 
   // Used on the renderer and IPC threads.
   scoped_refptr<DBMessageFilter> db_message_filter_;
+  scoped_refptr<CookieMessageFilter> cookie_message_filter_;
 
 #if defined(OS_POSIX)
   scoped_refptr<IPC::ChannelProxy::MessageFilter>
diff --git a/chrome/renderer/renderer_webkitclient_impl.cc b/chrome/renderer/renderer_webkitclient_impl.cc
index 9b7327d..68bfe06 100644
--- a/chrome/renderer/renderer_webkitclient_impl.cc
+++ b/chrome/renderer/renderer_webkitclient_impl.cc
@@ -19,6 +19,7 @@
 #include "chrome/common/render_messages.h"
 #include "chrome/common/webmessageportchannel_impl.h"
 #include "chrome/plugin/npobject_util.h"
+#include "chrome/renderer/cookie_message_filter.h"
 #include "chrome/renderer/net/render_dns_master.h"
 #include "chrome/renderer/render_thread.h"
 #include "chrome/renderer/render_view.h"
@@ -136,7 +137,7 @@ WebString RendererWebKitClientImpl::cookies(
   // back to Send to verify.  See http://crbug.com/36310.
 
   std::string value_utf8;
-  RenderThread::current()->Send(
+  SendCookieMessage(
       new ViewHostMsg_GetCookies(routing_id, url, first_party_for_cookies,
                                  &value_utf8));
 
@@ -152,11 +153,10 @@ bool RendererWebKitClientImpl::rawCookies(
   int32 routing_id = RenderThread::RoutingIDForCurrentContext();
 
   std::vector<webkit_glue::WebCookie> cookies;
-  RenderThread::current()->SendAndRunNestedMessageLoop(
+  SendCookieMessage(
       new ViewHostMsg_GetRawCookies(routing_id, url, first_party_for_cookies,
                                     &cookies));
 
-
   WebVector<WebKit::WebCookie> result(cookies.size());
   int i = 0;
   for (std::vector<webkit_glue::WebCookie>::iterator it = cookies.begin();
@@ -414,3 +414,21 @@ WebKit::WebString RendererWebKitClientImpl::signedPublicKeyAndChallengeString(
       &signed_public_key));
   return WebString::fromUTF8(signed_public_key);
 }
+
+//------------------------------------------------------------------------------
+
+void RendererWebKitClientImpl::SendCookieMessage(IPC::SyncMessage* message) {
+  RenderThread* render_thread = RenderThread::current();
+
+  CookieMessageFilter* filter = render_thread->cookie_message_filter();
+  message->set_pump_messages_event(filter->pump_messages_event());
+  render_thread->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));
+  }
+}
diff --git a/chrome/renderer/renderer_webkitclient_impl.h b/chrome/renderer/renderer_webkitclient_impl.h
index cbbeb11..b3a446c 100644
--- a/chrome/renderer/renderer_webkitclient_impl.h
+++ b/chrome/renderer/renderer_webkitclient_impl.h
@@ -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.
 
 #ifndef CHROME_RENDERER_RENDERER_WEBKITCLIENT_IMPL_H_
 #define CHROME_RENDERER_RENDERER_WEBKITCLIENT_IMPL_H_
@@ -20,6 +20,10 @@
 #include "third_party/WebKit/WebKit/chromium/public/linux/WebSandboxSupport.h"
 #endif
 
+namespace IPC {
+class SyncMessage;
+}
+
 class RendererWebKitClientImpl : public webkit_glue::WebKitClientImpl {
  public:
   RendererWebKitClientImpl() : sudden_termination_disables_(0) {}
@@ -106,6 +110,8 @@ class RendererWebKitClientImpl : public webkit_glue::WebKitClientImpl {
   };
 #endif
 
+  void SendCookieMessage(IPC::SyncMessage* message);
+
   webkit_glue::WebClipboardImpl clipboard_;
 
   MimeRegistry mime_registry_;
diff --git a/chrome/renderer/renderer_webstoragearea_impl.cc b/chrome/renderer/renderer_webstoragearea_impl.cc
index e1a9fb9..e4e87fa 100644
--- a/chrome/renderer/renderer_webstoragearea_impl.cc
+++ b/chrome/renderer/renderer_webstoragearea_impl.cc
@@ -54,9 +54,12 @@ void RendererWebStorageAreaImpl::setItem(
   CHECK(routing_id != MSG_ROUTING_CONTROL);
 
   NullableString16 old_value;
-  RenderThread::current()->SendAndRunNestedMessageLoop(
+  IPC::SyncMessage* message =
       new ViewHostMsg_DOMStorageSetItem(routing_id, storage_area_id_, key,
-                                        value, url, &result, &old_value));
+                                        value, url, &result, &old_value);
+  // TODO(darin): Response to ViewMsg_SignalCookiePromptEvent instead.
+  message->EnableMessagePumping();
+  RenderThread::current()->Send(message);
   old_value_webkit = old_value;
 
   if (result == WebStorageArea::ResultBlockedByPolicy) {