Change BrowserThreadDelegate to run Init() async.

android_webview/browser/aw_browser_context.cc

 // Copyright (c) 2012 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 "android_webview/browser/aw_browser_context.h"
 
 #include "android_webview/browser/aw_form_database_service.h"
 #include "android_webview/browser/aw_pref_store.h"
 #include "android_webview/browser/aw_quota_manager_bridge.h"
 #include "android_webview/browser/jni_dependency_factory.h"
@@ skipping 69 matching lines @@
 // static
 AwBrowserContext* AwBrowserContext::FromWebContents(
     content::WebContents* web_contents) {
   // This is safe; this is the only implementation of the browser context.
   return static_cast<AwBrowserContext*>(web_contents->GetBrowserContext());
 }
 
 void AwBrowserContext::InitializeBeforeThreadCreation() {
   DCHECK(!url_request_context_getter_.get());
   url_request_context_getter_ = new AwURLRequestContextGetter(this);
+  url_request_context_getter_->InitializeOnUiThread();

[awong, 2013/07/11 20:43:48]

I think we have to move this into PreMainMessageLoopRun for it to work. 
InitializeOnUiThread() calls CreateCookieStore() which calls
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO), and
BrowserThread::GetBlockingPool()->GetSequencedTaskRunner().

I don't think these exist until after thread creation.

AFAICT there are no other delegates in AwBrowserContext which I think means that
all the message loops must be empty in PreMainMessageLoopRun.

If that logic sounds right, then I think it'd be cleaner to 
  (a) Make AwURLRequestContextGetter NOT a BrowserThreadDelegate
  (b) Move your AwURLRequestContextGetter::InitializeOnIiThread() to 
      CookieStore* AwBrowserContext::CreateCookieStore()
  (c) First thing in PreMainMesasgeLoopRun, do
      scoped_refptr<CookieStore> cookie_store = CreateCookieStore();
      url_request_context_getter_ = new AwURLRequestContextGetter(this);
      BrowserThread::PostTask(IO, FROM_HERE,
          Bind(&AwURLRequestContextGetter::InitOnIOThread, 
               url_request_context_getter_,
               cookie_store));

The upside of not being a BrowserThreadDelegate is that the threading
constraints are better understood.

Please push back on me if you think I'm missing something. This code is tricky
enough that I'm only 80% confident in my analysis.

[Maria, 2013/07/11 21:11:37]

I think you are correct that I cannot be creating the cookie store until we have
a message loop for IO thread. However, I don't like the suggestion fix below
because it relies on the current knowledge of there not being a
BrowserThreadDelegate -- if someone was to add it in the future, they would
break our assumptions of an empty message loop.

Instead, what do you think about changing BrowserThreadDelegate interface to
have three methods:

Init()
InitAsync()
CleanUp

Then we will have a delegate->Init() call followed by delegate->InitAsync() call
scheduled as the first call on the loop.

So we would rename IOThread::Init into InitAsync and InitializeOnUiThread would
become Init() method.


On 2013/07/11 20:43:48, awong wrote:

 }
 
 void AwBrowserContext::PreMainMessageLoopRun() {
   visitedlink_master_.reset(
       new visitedlink::VisitedLinkMaster(this, this, false));
   visitedlink_master_->Init();
 }
 
 void AwBrowserContext::AddVisitedURLs(const std::vector<GURL>& urls) {
   DCHECK(visitedlink_master_);
@@ skipping 130 matching lines @@
 
 void AwBrowserContext::RebuildTable(
     const scoped_refptr<URLEnumerator>& enumerator) {
   // Android WebView rebuilds from WebChromeClient.getVisitedHistory. The client
   // can change in the lifetime of this WebView and may not yet be set here.
   // Therefore this initialization path is not used.
   enumerator->OnComplete(true);
 }
 
 }  // namespace android_webview