Use a MessageLoopProxy to forward PAC script errors to the network delegate

This fixes a race/crash when the worker threads of the multi threaded proxy resolver outlive the io thread

BUG=none
TEST=none


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

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

4 files changed, 31 insertions, 18 deletions

diff --git a/net/proxy/network_delegate_error_observer.cc b/net/proxy/network_delegate_error_observer.cc
index f0f9fdd..6ffbe84 100644
--- a/net/proxy/network_delegate_error_observer.cc
+++ b/net/proxy/network_delegate_error_observer.cc
@@ -2,10 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/message_loop.h"
+#include "net/proxy/network_delegate_error_observer.h"
+
+#include "base/message_loop_proxy.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_delegate.h"
-#include "net/proxy/network_delegate_error_observer.h"
 
 namespace net {
 
@@ -14,7 +15,7 @@ namespace net {
 class NetworkDelegateErrorObserver::Core
     : public base::RefCountedThreadSafe<NetworkDelegateErrorObserver::Core> {
  public:
-  Core(NetworkDelegate* network_delegate, MessageLoop* io_loop);
+  Core(NetworkDelegate* network_delegate, base::MessageLoopProxy* origin_loop);
 
   void NotifyPACScriptError(int line_number, const string16& error);
 
@@ -26,16 +27,16 @@ class NetworkDelegateErrorObserver::Core
   virtual ~Core();
 
   NetworkDelegate* network_delegate_;
-  MessageLoop* const io_loop_;
+  scoped_refptr<base::MessageLoopProxy> origin_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(Core);
 };
 
 NetworkDelegateErrorObserver::Core::Core(NetworkDelegate* network_delegate,
-                                         MessageLoop* io_loop)
+                                         base::MessageLoopProxy* origin_loop)
   : network_delegate_(network_delegate),
-    io_loop_(io_loop) {
-  DCHECK(io_loop_);
+    origin_loop_(origin_loop) {
+  DCHECK(origin_loop);
 }
 
 NetworkDelegateErrorObserver::Core::~Core() {}
@@ -44,8 +45,8 @@ NetworkDelegateErrorObserver::Core::~Core() {}
 void NetworkDelegateErrorObserver::Core::NotifyPACScriptError(
     int line_number,
     const string16& error) {
-  if (MessageLoop::current() != io_loop_) {
-    io_loop_->PostTask(
+  if (!origin_loop_->BelongsToCurrentThread()) {
+    origin_loop_->PostTask(
         FROM_HERE,
         NewRunnableMethod(this, &Core::NotifyPACScriptError,
                           line_number, error));
@@ -56,7 +57,7 @@ void NetworkDelegateErrorObserver::Core::NotifyPACScriptError(
 }
 
 void NetworkDelegateErrorObserver::Core::Shutdown() {
-  CHECK_EQ(MessageLoop::current(), io_loop_);
+  CHECK(origin_loop_->BelongsToCurrentThread());
   network_delegate_ = NULL;
 }
 
@@ -64,8 +65,8 @@ void NetworkDelegateErrorObserver::Core::Shutdown() {
 
 NetworkDelegateErrorObserver::NetworkDelegateErrorObserver(
     NetworkDelegate* network_delegate,
-    MessageLoop* io_loop)
-    : core_(new Core(network_delegate, io_loop)) {}
+    base::MessageLoopProxy* origin_loop)
+    : core_(new Core(network_delegate, origin_loop)) {}
 
 NetworkDelegateErrorObserver::~NetworkDelegateErrorObserver() {
   core_->Shutdown();
diff --git a/net/proxy/network_delegate_error_observer.h b/net/proxy/network_delegate_error_observer.h
index bca57be..8ece647 100644
--- a/net/proxy/network_delegate_error_observer.h
+++ b/net/proxy/network_delegate_error_observer.h
@@ -9,18 +9,20 @@
 #include "base/memory/ref_counted.h"
 #include "net/proxy/proxy_resolver_error_observer.h"
 
-class MessageLoop;
+namespace base {
+class MessageLoopProxy;
+}
 
 namespace net {
 
 class NetworkDelegate;
 
 // An implementation of ProxyResolverErrorObserver that forwards PAC script
-// errors to a NetworkDelegate object on the IO thread.
+// errors to a NetworkDelegate object on the thread it lives on.
 class NetworkDelegateErrorObserver : public ProxyResolverErrorObserver {
  public:
   NetworkDelegateErrorObserver(NetworkDelegate* network_delegate,
-                               MessageLoop* io_loop);
+                               base::MessageLoopProxy* origin_loop);
   virtual ~NetworkDelegateErrorObserver();
 
   // ProxyResolverErrorObserver implementation.
diff --git a/net/proxy/network_delegate_error_observer_unittest.cc b/net/proxy/network_delegate_error_observer_unittest.cc
index 736f717..b5f09c6 100644
--- a/net/proxy/network_delegate_error_observer_unittest.cc
+++ b/net/proxy/network_delegate_error_observer_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "net/proxy/network_delegate_error_observer.h"
 
+#include "base/message_loop_proxy.h"
 #include "base/threading/thread.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_delegate.h"
@@ -60,7 +61,8 @@ TEST(NetworkDelegateErrorObserverTest, CallOnThread) {
   thread.Start();
   TestNetworkDelegate network_delegate;
   NetworkDelegateErrorObserver
-      observer(&network_delegate, MessageLoop::current());
+      observer(&network_delegate,
+               base::MessageLoopProxy::CreateForCurrentThread());
   thread.message_loop()->PostTask(
       FROM_HERE,
       NewRunnableMethod(&observer,
@@ -75,7 +77,8 @@ TEST(NetworkDelegateErrorObserverTest, CallOnThread) {
 TEST(NetworkDelegateErrorObserverTest, NoDelegate) {
   base::Thread thread("test_thread");
   thread.Start();
-  NetworkDelegateErrorObserver observer(NULL, MessageLoop::current());
+  NetworkDelegateErrorObserver
+      observer(NULL, base::MessageLoopProxy::CreateForCurrentThread());
   thread.message_loop()->PostTask(
       FROM_HERE,
       NewRunnableMethod(&observer,
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index f5b1417..5391c5d 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -9,6 +9,7 @@
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
 #include "base/string_util.h"
 #include "base/values.h"
 #include "googleurl/src/gurl.h"
@@ -167,13 +168,16 @@ class ProxyResolverFactoryForV8 : public ProxyResolverFactory {
   // |async_host_resolver|, |io_loop| and |net_log| must remain
   // valid for the duration of our lifetime.
   // |async_host_resolver| will only be operated on |io_loop|.
+  // TODO(willchan): remove io_loop and replace it with origin_loop.
   ProxyResolverFactoryForV8(HostResolver* async_host_resolver,
                             MessageLoop* io_loop,
+                            base::MessageLoopProxy* origin_loop,
                             NetLog* net_log,
                             NetworkDelegate* network_delegate)
       : ProxyResolverFactory(true /*expects_pac_bytes*/),
         async_host_resolver_(async_host_resolver),
         io_loop_(io_loop),
+        origin_loop_(origin_loop),
         net_log_(net_log),
         network_delegate_(network_delegate) {
   }
@@ -185,7 +189,8 @@ class ProxyResolverFactoryForV8 : public ProxyResolverFactory {
         new SyncHostResolverBridge(async_host_resolver_, io_loop_);
 
     NetworkDelegateErrorObserver* error_observer =
-        new NetworkDelegateErrorObserver(network_delegate_, io_loop_);
+        new NetworkDelegateErrorObserver(
+            network_delegate_, origin_loop_.get());
 
     // ProxyResolverJSBindings takes ownership of |error_observer|.
     ProxyResolverJSBindings* js_bindings =
@@ -199,6 +204,7 @@ class ProxyResolverFactoryForV8 : public ProxyResolverFactory {
  private:
   HostResolver* const async_host_resolver_;
   MessageLoop* io_loop_;
+  scoped_refptr<base::MessageLoopProxy> origin_loop_;
   NetLog* net_log_;
   NetworkDelegate* network_delegate_;
 };
@@ -417,6 +423,7 @@ ProxyService* ProxyService::CreateUsingV8ProxyResolver(
       new ProxyResolverFactoryForV8(
           host_resolver,
           MessageLoop::current(),
+          base::MessageLoopProxy::CreateForCurrentThread(),
           net_log,
           network_delegate);