Fix for CookieManager init race condition

Fix for CookieManager init race condition

As the cookiemanager can be used at any time from any thread, we need to
ensure it is constructed early in startup, synchronously to the
AndroidBrowserProcess.init() call.
This rolls back a little of the change made in
http://src.chromium.org/viewvc/chrome/trunk/src/android_webview/browser/net/init_native_callback.h?view=diff&r1=174173&r2=174174

(intenral issue b/8249158)

BUG=


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=185462

[boliu, 2013-03-01 00:13:11]

lgtm

[boliu, 2013-03-01 00:19:06]

Actually I take it back a bit. This will make everything non-lazy, so there is
no point in trying to make it seem it is/could be lazy in some cases.

https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
android_webview/browser/net/aw_url_request_context_getter.cc:76:
DidCreateURLRequestContextOnIOThread(url_request_context_.get());
This is not true...the getter is initialized but the context has not been
created yet.

Can we make this non-laziness explicit? ie copy all of the initialization code
in GetURLRequestContext here, then add a comment saying why we can't be lazy.

[Kristian Monsen, 2013-03-01 00:19:08]

https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
android_webview/browser/net/aw_url_request_context_getter.cc:76:
DidCreateURLRequestContextOnIOThread(url_request_context_.get());
Can this safely be used before the job factory is created?

[joth, 2013-03-01 00:28:13]

https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
android_webview/browser/net/aw_url_request_context_getter.cc:76:
DidCreateURLRequestContextOnIOThread(url_request_context_.get());
On 2013/03/01 00:19:06, boliu wrote:
> This is not true...the getter is initialized but the context has not been
> created yet.
> 

 I think you mean the opposite? the request context is created (that happend up
on line 61) but not (fully) initialized yet: it does not have all the job
factories installed.

Note the method here is merely DidCreate.... not DidCreateAndInit
but that's very subtle :-)

Would it be clearer if I just passed the net::CookieMonster* here? That's all
that is needed, and we know that is fully created at this point


> Can we make this non-laziness explicit? ie copy all of the initialization code
> in GetURLRequestContext here, then add a comment saying why we can't be lazy.

As AwURLRequestContextGetter::Init() blocks the UI thread, it'd be nice to
minimize what we do in here if it is possible...

[Kristian Monsen, 2013-03-01 00:30:08]

On 2013/03/01 00:28:13, joth wrote:
>
https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
> File android_webview/browser/net/aw_url_request_context_getter.cc (right):
> 
>
https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
> android_webview/browser/net/aw_url_request_context_getter.cc:76:
> DidCreateURLRequestContextOnIOThread(url_request_context_.get());
> On 2013/03/01 00:19:06, boliu wrote:
> > This is not true...the getter is initialized but the context has not been
> > created yet.
> > 
> 
>  I think you mean the opposite? the request context is created (that happend
up
> on line 61) but not (fully) initialized yet: it does not have all the job
> factories installed.
> 
> Note the method here is merely DidCreate.... not DidCreateAndInit
> but that's very subtle :-)
> 
> Would it be clearer if I just passed the net::CookieMonster* here? That's all
> that is needed, and we know that is fully created at this point
> 
Yes, and maybe add a DCHECK and comment?

> 
> > Can we make this non-laziness explicit? ie copy all of the initialization
code
> > in GetURLRequestContext here, then add a comment saying why we can't be
lazy.
> 
> As AwURLRequestContextGetter::Init() blocks the UI thread, it'd be nice to
> minimize what we do in here if it is possible...

[joth, 2013-03-01 00:54:57]

On 2013/03/01 00:30:08, Kristian Monsen wrote:
> On 2013/03/01 00:28:13, joth wrote:
> >
>
https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
> > File android_webview/browser/net/aw_url_request_context_getter.cc (right):
> > 
> >
>
https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
> > android_webview/browser/net/aw_url_request_context_getter.cc:76:
> > DidCreateURLRequestContextOnIOThread(url_request_context_.get());
> > On 2013/03/01 00:19:06, boliu wrote:
> > > This is not true...the getter is initialized but the context has not been
> > > created yet.
> > > 
> > 
> >  I think you mean the opposite? the request context is created (that happend
> up
> > on line 61) but not (fully) initialized yet: it does not have all the job
> > factories installed.
> > 
> > Note the method here is merely DidCreate.... not DidCreateAndInit
> > but that's very subtle :-)
> > 
> > Would it be clearer if I just passed the net::CookieMonster* here? That's
all
> > that is needed, and we know that is fully created at this point
> > 
> Yes, and maybe add a DCHECK and comment?
> 
Done

> > 
> > > Can we make this non-laziness explicit? ie copy all of the initialization
> code
> > > in GetURLRequestContext here, then add a comment saying why we can't be
> lazy.
> > 
> > As AwURLRequestContextGetter::Init() blocks the UI thread, it'd be nice to
> > minimize what we do in here if it is possible...

[Kristian Monsen, 2013-03-01 01:00:51]

lgtm

[commit-bot: I haz the power, 2013-03-01 01:11:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/12387028/5

[boliu, 2013-03-01 01:32:15]

On 2013/03/01 00:28:13, joth wrote:
>
https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
> File android_webview/browser/net/aw_url_request_context_getter.cc (right):
> 
>
https://codereview.chromium.org/12387028/diff/1/android_webview/browser/net/a...
> android_webview/browser/net/aw_url_request_context_getter.cc:76:
> DidCreateURLRequestContextOnIOThread(url_request_context_.get());
> On 2013/03/01 00:19:06, boliu wrote:
> > This is not true...the getter is initialized but the context has not been
> > created yet.
> > 
> 
>  I think you mean the opposite? the request context is created (that happend
up
> on line 61) but not (fully) initialized yet: it does not have all the job
> factories installed.
> 
> Note the method here is merely DidCreate.... not DidCreateAndInit
> but that's very subtle :-)
> 
> Would it be clearer if I just passed the net::CookieMonster* here? That's all
> that is needed, and we know that is fully created at this point
> 
> 
> > Can we make this non-laziness explicit? ie copy all of the initialization
code
> > in GetURLRequestContext here, then add a comment saying why we can't be
lazy.
> 
> As AwURLRequestContextGetter::Init() blocks the UI thread, it'd be nice to
> minimize what we do in here if it is possible...

Yeah, my bad for not being able to read. Still lgtm

[commit-bot: I haz the power, 2013-03-01 04:26:16]

Change committed as 185462