Chromecast startup race: don't add filter before HostResolver exists.

Chromecast startup race: don't add filter before HostResolver exists.

HostResolver is on the I/O thread, which doesn't always have a chance
to run before filters are added. Get HostResolver from the I/O thread.

R=lcwu@chromium.org,byungchul@chromium.org
BUG=None, but the internal builders are failing

Committed: https://crrev.com/3e32f30e87ac59351c01af0ec7d1e972298d3b29
Cr-Commit-Position: refs/heads/master@{#326092}

[byungchul, 2015-04-17 00:22:20]

https://codereview.chromium.org/1081673004/diff/1/chromecast/browser/cast_con...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1081673004/diff/1/chromecast/browser/cast_con...
chromecast/browser/cast_content_browser_client.cc:89: base::Unretained(this),
host->GetID()));
It changes the order of message filter. Is it okay?

And, can it guarantee host_resolver is available if synchronized with browser IO
thread? It seems like it also depend on when we call url request context getter.

[gunsch, 2015-04-17 01:06:27]

https://codereview.chromium.org/1081673004/diff/1/chromecast/browser/cast_con...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1081673004/diff/1/chromecast/browser/cast_con...
chromecast/browser/cast_content_browser_client.cc:89: base::Unretained(this),
host->GetID()));
On 2015/04/17 00:22:20, byungchul wrote:
> It changes the order of message filter. Is it okay?
> 
> And, can it guarantee host_resolver is available if synchronized with browser
IO
> thread? It seems like it also depend on when we call url request context
getter.

Changing order is okay. Order was not so important, just based on when they were
added.

I took a closer look and you're correct that it depends, since initialization is
lazily done. I now force IO-thread initialization immediately in
CastBrowserMainParts so that this startup path is much more explicit.

[byungchul, 2015-04-17 17:56:17]

https://codereview.chromium.org/1081673004/diff/20001/chromecast/browser/cast...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/1081673004/diff/20001/chromecast/browser/cast...
chromecast/browser/cast_browser_main_parts.cc:284:
url_request_context_factory_->GetSystemGetter())));
I would call this in CastContentBrowserClient::RenderProcessWillLaunch(). You
can replace the first callback PostTaskAndReplyWithResult() with this callback.
It is better to put related logic in one place.

[gunsch, 2015-04-17 18:24:42]

https://codereview.chromium.org/1081673004/diff/20001/chromecast/browser/cast...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/1081673004/diff/20001/chromecast/browser/cast...
chromecast/browser/cast_browser_main_parts.cc:284:
url_request_context_factory_->GetSystemGetter())));
On 2015/04/17 17:56:17, byungchul wrote:
> I would call this in CastContentBrowserClient::RenderProcessWillLaunch(). You
> can replace the first callback PostTaskAndReplyWithResult() with this
callback.
> It is better to put related logic in one place.

SGTM. I think CastContentBrowserClient is the only place where we have this
thread-race dependency, so putting this there seems reasonable. If we had it in
more places I might prefer to move the logic here.

[gunsch, 2015-04-20 20:02:03]

ping? our internal tests are still failing because of this

[gunsch, 2015-04-20 20:02:09]

The CQ bit was checked by gunsch@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-20 20:05:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1081673004/40001

[commit-bot: I haz the power, 2015-04-20 21:22:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-20 21:22:01]

Dry run: This issue passed the CQ dry run.

[byungchul, 2015-04-21 00:24:30]

lgtm

https://codereview.chromium.org/1081673004/diff/40001/chromecast/browser/cast...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1081673004/diff/40001/chromecast/browser/cast...
chromecast/browser/cast_content_browser_client.cc:98: base::Unretained(this),
host->GetID()));
I am fine with this, but you can merge 2 posttasks into 1, and call
url_request_context_factory_->host_resolver() in AddExtraFilter().

[gunsch, 2015-04-21 17:10:46]

https://codereview.chromium.org/1081673004/diff/40001/chromecast/browser/cast...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1081673004/diff/40001/chromecast/browser/cast...
chromecast/browser/cast_content_browser_client.cc:98: base::Unretained(this),
host->GetID()));
I like that idea, done.

[byungchul, 2015-04-21 17:49:25]

lgtm

[lcwu1, 2015-04-21 18:25:55]

lgtm

[gunsch, 2015-04-21 18:43:17]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2015-04-21 18:43:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1081673004/60001

[commit-bot: I haz the power, 2015-04-21 19:12:14]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-21 19:13:02]

Patchset 4 (id:??) landed as
https://crrev.com/3e32f30e87ac59351c01af0ec7d1e972298d3b29
Cr-Commit-Position: refs/heads/master@{#326092}