Support Service Worker NavigationPreload with PlzNavigate.

Support Service Worker NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
TEST=content_browsertests --enable-browser-side-navigation --gtest_filter=ServiceWorkerBrowserTest/ServiceWorkerNavigationPreloadTest.*
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

[horo, 2016-10-24 08:11:12]

Description was changed from

==========
[not for review]NavigationPreload support with PlzNavigate
==========

to

==========
[not for review]NavigationPreload support with PlzNavigate
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[horo, 2016-10-26 02:29:54]

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

[commit-bot: I haz the power, 2016-10-26 02:30:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 03:32:45]

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

[commit-bot: I haz the power, 2016-10-26 03:32:46]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[horo, 2016-10-26 06:17:19]

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

[commit-bot: I haz the power, 2016-10-26 06:17:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 07:42:02]

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

[commit-bot: I haz the power, 2016-10-26 07:42:02]

Dry run: This issue passed the CQ dry run.

[horo, 2016-10-26 09:28:45]

Description was changed from

==========
[not for review]NavigationPreload support with PlzNavigate
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
[not for review]NavigationPreload support with PlzNavigate

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But when PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter().
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.
==========

[horo, 2016-10-26 09:40:26]

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

[commit-bot: I haz the power, 2016-10-26 09:40:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 11:23:56]

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

[commit-bot: I haz the power, 2016-10-26 11:23:57]

Dry run: This issue passed the CQ dry run.

[horo, 2016-10-26 17:28:12]

Description was changed from

==========
[not for review]NavigationPreload support with PlzNavigate

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But when PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter().
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.
==========

to

==========
Support NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But when PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter().
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.
==========

[horo, 2016-10-26 17:28:40]

Description was changed from

==========
Support NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But when PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter().
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.
==========

to

==========
Support NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But when PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
==========

[horo, 2016-10-26 17:29:18]

Description was changed from

==========
Support NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But when PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
==========

to

==========
Support NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
==========

[horo, 2016-10-26 17:30:18]

Patchset #1 (id:1) has been deleted

[horo, 2016-10-26 17:31:26]

Patchset #4 (id:80001) has been deleted

[horo, 2016-10-26 17:31:31]

Patchset #3 (id:60001) has been deleted

[horo, 2016-10-26 17:31:36]

Patchset #2 (id:40001) has been deleted

[horo, 2016-10-26 17:31:40]

Patchset #1 (id:20001) has been deleted

[horo, 2016-10-26 17:32:29]

Description was changed from

==========
Support NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
==========

to

==========
Support Service Worker NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
==========

[horo, 2016-10-26 17:34:37]

horo@chromium.org changed reviewers:
+ falken@chromium.org

[horo, 2016-10-26 17:34:38]

falken@
Could you please review this?

[falken, 2016-10-27 02:07:49]

this lgtm

https://codereview.chromium.org/2446553002/diff/100001/content/public/browser...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/2446553002/diff/100001/content/public/browser...
content/public/browser/render_process_host.h:340: // used for fetching
navigation preload request.
I think we can remove the second sentence, since the URL loader factory is
potentially useful for purposes other than navigation preload (I think). If it
could only possible be used for navigation preload, the function name should be
something like CreateURLLoaderFactoryGetterForNavigationPreload.

Ideally we'd avoid a new function on RPH and just have the caller bind the
function itself using RenderProcessHostImpl::CreateURLLoaderFactory, but I see
there is no GetWeakPtr() function and adding it might be controversial.

Also, does CreateURLLoaderFactoryGetter() have to be on the public interface or
can it just be on RenderProcessHostInstance?

[horo, 2016-10-27 02:19:43]

Description was changed from

==========
Support Service Worker NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
==========

to

==========
Support Service Worker NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[horo, 2016-10-27 02:19:47]

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

[commit-bot: I haz the power, 2016-10-27 02:20:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[horo, 2016-10-27 02:20:27]

https://codereview.chromium.org/2446553002/diff/100001/content/public/browser...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/2446553002/diff/100001/content/public/browser...
content/public/browser/render_process_host.h:340: // used for fetching
navigation preload request.
On 2016/10/27 02:07:49, falken wrote:
> I think we can remove the second sentence, since the URL loader factory is
> potentially useful for purposes other than navigation preload (I think). If it
> could only possible be used for navigation preload, the function name should
be
> something like CreateURLLoaderFactoryGetterForNavigationPreload.

done.

> Ideally we'd avoid a new function on RPH and just have the caller bind the
> function itself using RenderProcessHostImpl::CreateURLLoaderFactory, but I see
> there is no GetWeakPtr() function and adding it might be controversial.
> 
> Also, does CreateURLLoaderFactoryGetter() have to be on the public interface
or
> can it just be on RenderProcessHostInstance?

Are you saying about RenderProcessHostImpl?
We have to have this method in RenderProcessHost, because some unittests
(NavigatorTestWithBrowserSideNavigation) are using MockRenderProcessHost , not
RenderProcessHostImpl.

[horo, 2016-10-27 02:24:23]

Description was changed from

==========
Support Service Worker NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Support Service Worker NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
TEST=content_browsertests --enable-browser-side-navigation
--gtest_filter=ServiceWorkerBrowserTest/ServiceWorkerNavigationPreloadTest.*
==========

[horo, 2016-10-27 02:24:44]

Description was changed from

==========
Support Service Worker NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
TEST=content_browsertests --enable-browser-side-navigation
--gtest_filter=ServiceWorkerBrowserTest/ServiceWorkerNavigationPreloadTest.*
==========

to

==========
Support Service Worker NavigationPreload with PlzNavigate.

When ServiceWorkerFetchDispatcher sends the navigation preload request, it uses
MojoURLLoaderFactoryGetter to get the mojom::URLLoaderFactory which is
associated
with the RenderProcessHost.

This MojoURLLoaderFactoryGetter is created in
RenderProcessHostImpl::CreateMessageFilters()
and passed to ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can
get it
from ServiceWorkerDispatcherHost via ServiceWorkerProviderHost and
ServiceWorkerControlleeRequestHandler and ServiceWorkerURLRequestJob.

But if PlzNavigate is enabled, when
ServiceWorkerProviderHost::PreCreateNavigationHost()
creates a ServiceWorkerProviderHost, the ServiceWorkerProviderHost is not
associated
with any ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher can't get
the
MojoURLLoaderFactoryGetter.

To solve this problem, this CL introduces
RenderProcessHost::CreateURLLoaderFactoryGetter() and:
- NavigationRequest::OnStartChecksComplete() creates a
MojoURLLoaderFactoryGetter from
  the navigating_frame_host's process and pass it to
ServiceWorkerNavigationHandleCore.
- ServiceWorkerRequestHandler::InitializeForNavigation() gets the
  MojoURLLoaderFactoryGetter from ServiceWorkerNavigationHandleCore and pass it
  to ServiceWorkerProviderHost.
- So when ServiceWorkerProviderHost::CreateRequestHandler() is called, 
  ServiceWorkerProviderHost must have a MojoURLLoaderFactoryGetter.

BUG=649558
TEST=content_browsertests --enable-browser-side-navigation
--gtest_filter=ServiceWorkerBrowserTest/ServiceWorkerNavigationPreloadTest.*
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[horo, 2016-10-27 02:25:55]

horo@chromium.org changed reviewers:
+ clamy@chromium.org

[horo, 2016-10-27 02:25:56]

clamy@
Could you please review this?

[falken, 2016-10-27 02:27:56]

https://codereview.chromium.org/2446553002/diff/100001/content/public/browser...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/2446553002/diff/100001/content/public/browser...
content/public/browser/render_process_host.h:340: // used for fetching
navigation preload request.
On 2016/10/27 02:20:27, horo wrote:
> On 2016/10/27 02:07:49, falken wrote:
> > I think we can remove the second sentence, since the URL loader factory is
> > potentially useful for purposes other than navigation preload (I think). If
it
> > could only possible be used for navigation preload, the function name should
> be
> > something like CreateURLLoaderFactoryGetterForNavigationPreload.
> 
> done.
> 
> > Ideally we'd avoid a new function on RPH and just have the caller bind the
> > function itself using RenderProcessHostImpl::CreateURLLoaderFactory, but I
see
> > there is no GetWeakPtr() function and adding it might be controversial.
> > 
> > Also, does CreateURLLoaderFactoryGetter() have to be on the public interface
> or
> > can it just be on RenderProcessHostInstance?
> 
> Are you saying about RenderProcessHostImpl?
> We have to have this method in RenderProcessHost, because some unittests
> (NavigatorTestWithBrowserSideNavigation) are using MockRenderProcessHost , not
> RenderProcessHostImpl.

Yes, Impl. Acknowledged.

[horo, 2016-10-27 02:30:19]

horo@chromium.org changed reviewers:
+ alexmos@chromium.org

[horo, 2016-10-27 02:30:20]

Ah, clamy@ is ooo until Nov 2.

alexmos@
Could you please review content/browser/frame_host/OWNERS?

[commit-bot: I haz the power, 2016-10-27 05:06:25]

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

[commit-bot: I haz the power, 2016-10-27 05:06:26]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[alexmos, 2016-10-28 01:13:09]

frame_host LGTM.  

I'm not as familiar with service worker internals, so this is mostly a RS based
on falken@'s review.

[horo, 2016-10-28 01:45:30]

horo@chromium.org changed reviewers:
- clamy@chromium.org

[horo, 2016-10-28 01:48:19]

horo@chromium.org changed reviewers:
+ clamy@chromium.org, nasko@chromium.org

[horo, 2016-10-28 01:48:20]

nasko@
Could you please review content/browser/renderer_host/render_process_host_impl.*
content/public/browser/render_process_host.h
content/public/test/mock_render_process_host.*?

[horo, 2016-10-31 08:17:53]

horo@chromium.org changed reviewers:
- clamy@chromium.org, nasko@chromium.org

[horo, 2016-10-31 08:19:39]

horo@chromium.org changed reviewers:
+ clamy@chromium.org, nasko@chromium.org

[horo, 2016-10-31 08:20:14]

horo@chromium.org changed reviewers:
+ piman@chromium.org

[horo, 2016-10-31 08:20:15]

piman@

Could you please review content/browser/renderer_host/render_process_host_impl.*
content/public/browser/render_process_host.h
content/public/test/mock_render_process_host.*?

[horo, 2016-10-31 14:50:32]

https://codereview.chromium.org/2446553002/diff/120001/content/browser/render...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/2446553002/diff/120001/content/browser/render...
content/browser/renderer_host/render_process_host_impl.cc:1414:
weak_factory_.GetWeakPtr());
WeakPtr is not thread safe.
So we should not pass the MojoURLLoaderFactoryGetter to the IO thread.
http://www.chromium.org/developers/design-documents/threading#base_WeakPtr_an...

I will create another CL to avoid this problem.

Sorry for confusion.

[piman, 2016-10-31 20:43:25]

https://codereview.chromium.org/2446553002/diff/120001/content/browser/render...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/2446553002/diff/120001/content/browser/render...
content/browser/renderer_host/render_process_host_impl.cc:1414:
weak_factory_.GetWeakPtr());
On 2016/10/31 14:50:32, horo wrote:
> WeakPtr is not thread safe.
> So we should not pass the MojoURLLoaderFactoryGetter to the IO thread.
>
http://www.chromium.org/developers/design-documents/threading#base_WeakPtr_an...
> 
> I will create another CL to avoid this problem.
> 
> Sorry for confusion.

I'll wait for the update before a full review. Just one thing: if there are
specific expectations about the value returned from CreateURLLoaderFactoryGetter
(i.e. used on a different thread than where it's queried?), can you document
that in the API?

FYI, WeakPtr is not generally thread-safe, however it can be copied to other
threads - I don't know if that matters to you. The restriction is that it can
only be dereferenced  on the same thread as it gets cancelled (i.e. the thread
the WeakPtrFactory gets destroyed).

[horo, 2016-10-31 22:09:42]

On 2016/10/31 20:43:25, piman wrote:
>
https://codereview.chromium.org/2446553002/diff/120001/content/browser/render...
> File content/browser/renderer_host/render_process_host_impl.cc (right):
> 
>
https://codereview.chromium.org/2446553002/diff/120001/content/browser/render...
> content/browser/renderer_host/render_process_host_impl.cc:1414:
> weak_factory_.GetWeakPtr());
> On 2016/10/31 14:50:32, horo wrote:
> > WeakPtr is not thread safe.
> > So we should not pass the MojoURLLoaderFactoryGetter to the IO thread.
> >
>
http://www.chromium.org/developers/design-documents/threading#base_WeakPtr_an...
> > 
> > I will create another CL to avoid this problem.
> > 
> > Sorry for confusion.
> 
> I'll wait for the update before a full review. Just one thing: if there are
> specific expectations about the value returned from
CreateURLLoaderFactoryGetter
> (i.e. used on a different thread than where it's queried?), can you document
> that in the API?
> 
> FYI, WeakPtr is not generally thread-safe, however it can be copied to other
> threads - I don't know if that matters to you. The restriction is that it can
> only be dereferenced  on the same thread as it gets cancelled (i.e. the thread
> the WeakPtrFactory gets destroyed).

ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload() executes the
MojoURLLoaderFactoryGetter on the IO thread.
But the WeakPtr is dereferenced on the UI thread in the destructor of
RenderProcessHostImpl.
So it is the problem.

[horo, 2016-11-01 10:15:40]

On 2016/10/31 22:09:42, horo wrote:
> On 2016/10/31 20:43:25, piman wrote:
> >
>
https://codereview.chromium.org/2446553002/diff/120001/content/browser/render...
> > File content/browser/renderer_host/render_process_host_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/2446553002/diff/120001/content/browser/render...
> > content/browser/renderer_host/render_process_host_impl.cc:1414:
> > weak_factory_.GetWeakPtr());
> > On 2016/10/31 14:50:32, horo wrote:
> > > WeakPtr is not thread safe.
> > > So we should not pass the MojoURLLoaderFactoryGetter to the IO thread.
> > >
> >
>
http://www.chromium.org/developers/design-documents/threading#base_WeakPtr_an...
> > > 
> > > I will create another CL to avoid this problem.
> > > 
> > > Sorry for confusion.
> > 
> > I'll wait for the update before a full review. Just one thing: if there are
> > specific expectations about the value returned from
> CreateURLLoaderFactoryGetter
> > (i.e. used on a different thread than where it's queried?), can you document
> > that in the API?
> > 
> > FYI, WeakPtr is not generally thread-safe, however it can be copied to other
> > threads - I don't know if that matters to you. The restriction is that it
can
> > only be dereferenced  on the same thread as it gets cancelled (i.e. the
thread
> > the WeakPtrFactory gets destroyed).
> 
> ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload() executes the
> MojoURLLoaderFactoryGetter on the IO thread.
> But the WeakPtr is dereferenced on the UI thread in the destructor of
> RenderProcessHostImpl.
> So it is the problem.

I created two CLs.
- Stop using MojoURLLoaderFactoryGetter
  https://codereview.chromium.org/2465813003/
- Support Service Worker NavigationPreload with PlzNavigate (without
MojoURLLoaderFactoryGetter)
  https://codereview.chromium.org/2460223003/