[cros] Make g_browser_process's call on the UI thread.

chrome/browser/extensions/extension_web_socket_proxy_private_api.cc

Index: chrome/browser/extensions/extension_web_socket_proxy_private_api.cc
diff --git a/chrome/browser/extensions/extension_web_socket_proxy_private_api.cc b/chrome/browser/extensions/extension_web_socket_proxy_private_api.cc
index b2cb933310eb8958e65432b5548307e193f94e87..ddfe14caba80905d303569edab238801c7713165 100644
--- a/chrome/browser/extensions/extension_web_socket_proxy_private_api.cc
+++ b/chrome/browser/extensions/extension_web_socket_proxy_private_api.cc
@@ -63,28 +63,29 @@ void WebSocketProxyPrivate::Observe(
 
 void WebSocketProxyPrivate::ResolveHost() {
 #if defined(OS_CHROMEOS)
-  if (!content::BrowserThread::CurrentlyOn(content::BrowserThread::IO)) {
-    content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,
-        base::Bind(&WebSocketProxyPrivate::ResolveHost, this));
-    return;
-  }
+  IOThread* io_thread = g_browser_process->io_thread();

[stevenjb, 2011/11/28 18:45:39]

One reason that g_browser_process->io_thread() is disallowed from other threads
is that access to the IOThread members are not inherently thread safe. This code
as written could cause a subtle shutdown crash when accessing io_thread from
ResolveHostIOPart().

Furthermore, resolver_ could get destroyed on shutdown after it is reset and
before it is accessed in ResolveHostIOPart() during shutdown. 

resolver_->Resolve() itself does not appear to be blocking, so I'm not sure why
it needs to be called from the IO thread? If it does, resolver_ should get set
on the UI thread, and some mechanism added to prevent resolver_ from being
deleted before it is accessed from the IO thread.

[altimofeev, 2011/11/29 12:30:20]

Please find my answers inline.

On 2011/11/28 18:45:39, Steven Bennetts wrote:
Few points because of which I think the code is correct:
1. There is 'DCHECK(Currently on IO thread)' in IOThread::globals()  getter. So
it is designed to be called on IOThread.
2. In case IOThread was deleted, PostMessage prevents us from running task on IO
thread, thus avoiding usage of the invalid pointer.
3. There are places in the code, where we are passing io_thread pointer (e.g.
profiles code)

Anyway, I'll be glad to hear the right way of getting host_resolver.
I think it cannot. The reason is that WebSocketProxyPrivate inherits from
RefCountedThreadSafe. And there is AddRef() call in RunImpl() method, which is
compensated only in Finalize() method.
 
I haven't found any DCHECKs, but since host_resolver belongs to the IO thread,
it is logically correct to use it on the IO thread too.

+  content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&WebSocketProxyPrivate::ResolveHostIOPart, this, io_thread));
+#endif
+}
+
+void WebSocketProxyPrivate::ResolveHostIOPart(IOThread* io_thread) {
+#if defined(OS_CHROMEOS)
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
   DCHECK(resolver_ == NULL);
-  if (g_browser_process) {
-    IOThread* io_thread = g_browser_process->io_thread();
-    if (io_thread && io_thread->globals()) {
-      net::HostResolver* host_resolver =
-          io_thread->globals()->host_resolver.get();
-      if (host_resolver) {
-        resolver_.reset(new net::SingleRequestHostResolver(host_resolver));
-        net::HostResolver::RequestInfo info(net::HostPortPair(
-            hostname_, port_));
-        int result = resolver_->Resolve(info, &addr_,
-            base::Bind(&WebSocketProxyPrivate::OnHostResolution, this),
-            net::BoundNetLog());
-        if (result != net::ERR_IO_PENDING)
-          OnHostResolution(result);
-        return;
-      }
+  if (io_thread && io_thread->globals()) {
+    net::HostResolver* host_resolver =
+        io_thread->globals()->host_resolver.get();
+    if (host_resolver) {
+      resolver_.reset(new net::SingleRequestHostResolver(host_resolver));
+      net::HostResolver::RequestInfo info(net::HostPortPair(
+          hostname_, port_));
+      int result = resolver_->Resolve(info, &addr_,
+          base::Bind(&WebSocketProxyPrivate::OnHostResolution, this),
+          net::BoundNetLog());
+      if (result != net::ERR_IO_PENDING)
+        OnHostResolution(result);
+      return;
     }
   }
   NOTREACHED();