Introduce FetchEventPreloadHandle to pass the preload response to FetchEvent.

Introduce FetchEventPreloadHandle to pass the preload response to FetchEvent.

FetchEvent.navigationPreload will be implemented in the following steps.
[1/4] Introduce FetchEventPreloadHandle to pass the preload response to FetchEvent.
      https://codereview.chromium.org/2417793002/ This CL.
[2/4] Implement ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload().
      https://codereview.chromium.org/2410333006/
[3/4] Implement FetchEvent.navigationPreload.
      https://codereview.chromium.org/2416843002/
[4/4] Add browser tests for NavigationPreload.
      https://codereview.chromium.org/2413643005/

This CL introduces FetchEventPreloadHandle in fetch_event_dispatcher.mojom.
If the NavigationPreload feature is enabled:
 - ServiceWorkerURLRequestJob calls ServiceWorkerFetchDispatcher::
   MaybeStartNavigationPreload() even if the worker is not started yet.
 - ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload() will start the
   navigation preload request and create a FetchEventPreloadHandle.
   https://codereview.chromium.org/2410333006/ implements this method.
 - The FetchEventPreloadHandle will be passed to the service worker when
   DispatchFetchEvent() dispatches the FetchEvent.
 - ServiceWorkerContextClient::FetchEventDispatcherImpl receives the
   FetchEventPreloadHandle and will pass it to FetchEvent.
   https://codereview.chromium.org/2416843002/ implements this logic.

BUG=649558
Committed: https://crrev.com/fc9f9dc28b5291e0582dc1fcb2fb35aa0409f34f
Cr-Commit-Position: refs/heads/master@{#425927}

[horo, 2016-10-13 08:05:52]

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

[commit-bot: I haz the power, 2016-10-13 08:06:07]

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-13 08:53:03]

Description was changed from

==========
Introduce FetchEventPreloadHandle to pass the navigation preload to FetchEvent.

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

to

==========
Introduce FetchEventPreloadHandle to pass the preload response to FetchEvent.

FetchEvent.navigationPreload will be implemented in the following steps.
[1/4] Introduce FetchEventPreloadHandle to pass the preload response to
FetchEvent.
      https://codereview.chromium.org/2417793002/ This CL.
[2/4] Implement ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload().
      https://codereview.chromium.org/2410333006/
[3/4] Implement FetchEvent.navigationPreload.
      https://codereview.chromium.org/2416843002/
[4/4] Add browser tests for NavigationPreload.
      https://codereview.chromium.org/2413643005/

This CL introduces FetchEventPreloadHandle in fetch_event_dispatcher.mojom.
If the NavigationPreload feature is enabled:
 - ServiceWorkerURLRequestJob calls ServiceWorkerFetchDispatcher::
   MaybeStartNavigationPreload() even if the worker is not started yet.
 - ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload() will start the
   navigation preload request and create a FetchEventPreloadHandle.
   https://codereview.chromium.org/2410333006/ implements this method.
 - The FetchEventPreloadHandle will be passed to the service worker when
   DispatchFetchEvent() dispatches the FetchEvent.
 - ServiceWorkerContextClient::FetchEventDispatcherImpl receives the
   FetchEventPreloadHandle and will pass it to FetchEvent.
   https://codereview.chromium.org/2416843002/ implements this logic.

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

[horo, 2016-10-13 09:00:53]

Patchset #1 (id:1) has been deleted

[horo, 2016-10-13 09:01:01]

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

[commit-bot: I haz the power, 2016-10-13 09:01:30]

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-13 09:57:49]

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

[commit-bot: I haz the power, 2016-10-13 09:57:50]

Dry run: This issue passed the CQ dry run.

[horo, 2016-10-13 10:02:41]

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

[horo, 2016-10-13 10:02:41]

falken@
Could you please review this?

[horo, 2016-10-14 05:13:26]

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

[commit-bot: I haz the power, 2016-10-14 05:13:33]

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

[falken, 2016-10-14 07:53:42]

lgtm

[horo, 2016-10-14 08:06:53]

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

[horo, 2016-10-14 08:06:54]

dcheng@
Could you please review
content/common/service_worker/fetch_event_dispatcher.mojom?

[commit-bot: I haz the power, 2016-10-14 08:31:25]

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

[commit-bot: I haz the power, 2016-10-14 08:31:26]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-10-14 19:26:54]

This CL itself looks fine.

However, it's usually preferred to land IPC / mojo changes with the accompany
implementation changes. Would that be possible here?

https://codereview.chromium.org/2417793002/diff/40001/content/browser/service...
File content/browser/service_worker/embedded_worker_test_helper.cc (right):

https://codereview.chromium.org/2417793002/diff/40001/content/browser/service...
content/browser/service_worker/embedded_worker_test_helper.cc:510: request,
base::Passed(std::move(preload_handle)), callback));
Nit: base::Passed(&preload_handle)

[horo, 2016-10-14 22:54:34]

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

[commit-bot: I haz the power, 2016-10-14 22:54:53]

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-15 00:14:21]

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

[commit-bot: I haz the power, 2016-10-15 00:14:21]

Dry run: This issue passed the CQ dry run.

[horo, 2016-10-15 00:42:24]

> This CL itself looks fine.
>
> However, it's usually preferred to land IPC / mojo changes with the accompany
> implementation changes. Would that be possible here?

https://codereview.chromium.org/2410333006/ implements the browser side, and
https://codereview.chromium.org/2416843002/ implements the rendere side.
The both CLs are big, so I think that merging three CLs makes code review hard.

https://codereview.chromium.org/2417793002/diff/40001/content/browser/service...
File content/browser/service_worker/embedded_worker_test_helper.cc (right):

https://codereview.chromium.org/2417793002/diff/40001/content/browser/service...
content/browser/service_worker/embedded_worker_test_helper.cc:510: request,
base::Passed(std::move(preload_handle)), callback));
On 2016/10/14 19:26:54, dcheng wrote:
> Nit: base::Passed(&preload_handle)

Done.

[dcheng, 2016-10-18 01:55:51]

On 2016/10/15 00:42:24, horo wrote:
> > This CL itself looks fine.
> >
> > However, it's usually preferred to land IPC / mojo changes with the
accompany
> > implementation changes. Would that be possible here?
> 
> https://codereview.chromium.org/2410333006/ implements the browser side, and
> https://codereview.chromium.org/2416843002/ implements the rendere side.
> The both CLs are big, so I think that merging three CLs makes code review
hard.
> 
>
https://codereview.chromium.org/2417793002/diff/40001/content/browser/service...
> File content/browser/service_worker/embedded_worker_test_helper.cc (right):
> 
>
https://codereview.chromium.org/2417793002/diff/40001/content/browser/service...
> content/browser/service_worker/embedded_worker_test_helper.cc:510: request,
> base::Passed(std::move(preload_handle)), callback));
> On 2016/10/14 19:26:54, dcheng wrote:
> > Nit: base::Passed(&preload_handle)
> 
> Done.

Since it looks like CL 2 is already being independently reviewed, let's make
things easy and keep it like this. Please include a security reviewer on the
followup CLs though.

(In the future, I think it is not so bad to combine these; it is larger, but
it's not that much larger, and it gives a lot more helpful context for the
security review.)

mojo lgtm

[horo, 2016-10-18 07:50:46]

On 2016/10/18 01:55:51, dcheng wrote:
> On 2016/10/15 00:42:24, horo wrote:
> > > This CL itself looks fine.
> > >
> > > However, it's usually preferred to land IPC / mojo changes with the
> accompany
> > > implementation changes. Would that be possible here?
> > 
> > https://codereview.chromium.org/2410333006/ implements the browser side, and
> > https://codereview.chromium.org/2416843002/ implements the rendere side.
> > The both CLs are big, so I think that merging three CLs makes code review
> hard.
> > 
> >
>
https://codereview.chromium.org/2417793002/diff/40001/content/browser/service...
> > File content/browser/service_worker/embedded_worker_test_helper.cc (right):
> > 
> >
>
https://codereview.chromium.org/2417793002/diff/40001/content/browser/service...
> > content/browser/service_worker/embedded_worker_test_helper.cc:510: request,
> > base::Passed(std::move(preload_handle)), callback));
> > On 2016/10/14 19:26:54, dcheng wrote:
> > > Nit: base::Passed(&preload_handle)
> > 
> > Done.
> 
> Since it looks like CL 2 is already being independently reviewed, let's make
> things easy and keep it like this. Please include a security reviewer on the
> followup CLs though.
> 
> (In the future, I think it is not so bad to combine these; it is larger, but
> it's not that much larger, and it gives a lot more helpful context for the
> security review.)
> 
> mojo lgtm

OK. I got it.
Thank you.

[horo, 2016-10-18 07:51:04]

The CQ bit was checked by horo@chromium.org

[horo, 2016-10-18 07:51:04]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org
Link to the patchset: https://codereview.chromium.org/2417793002/#ps60001
(title: "incorporated dchecng's comment")

[commit-bot: I haz the power, 2016-10-18 07:51:25]

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-18 09:08:43]

Description was changed from

==========
Introduce FetchEventPreloadHandle to pass the preload response to FetchEvent.

FetchEvent.navigationPreload will be implemented in the following steps.
[1/4] Introduce FetchEventPreloadHandle to pass the preload response to
FetchEvent.
      https://codereview.chromium.org/2417793002/ This CL.
[2/4] Implement ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload().
      https://codereview.chromium.org/2410333006/
[3/4] Implement FetchEvent.navigationPreload.
      https://codereview.chromium.org/2416843002/
[4/4] Add browser tests for NavigationPreload.
      https://codereview.chromium.org/2413643005/

This CL introduces FetchEventPreloadHandle in fetch_event_dispatcher.mojom.
If the NavigationPreload feature is enabled:
 - ServiceWorkerURLRequestJob calls ServiceWorkerFetchDispatcher::
   MaybeStartNavigationPreload() even if the worker is not started yet.
 - ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload() will start the
   navigation preload request and create a FetchEventPreloadHandle.
   https://codereview.chromium.org/2410333006/ implements this method.
 - The FetchEventPreloadHandle will be passed to the service worker when
   DispatchFetchEvent() dispatches the FetchEvent.
 - ServiceWorkerContextClient::FetchEventDispatcherImpl receives the
   FetchEventPreloadHandle and will pass it to FetchEvent.
   https://codereview.chromium.org/2416843002/ implements this logic.

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

to

==========
Introduce FetchEventPreloadHandle to pass the preload response to FetchEvent.

FetchEvent.navigationPreload will be implemented in the following steps.
[1/4] Introduce FetchEventPreloadHandle to pass the preload response to
FetchEvent.
      https://codereview.chromium.org/2417793002/ This CL.
[2/4] Implement ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload().
      https://codereview.chromium.org/2410333006/
[3/4] Implement FetchEvent.navigationPreload.
      https://codereview.chromium.org/2416843002/
[4/4] Add browser tests for NavigationPreload.
      https://codereview.chromium.org/2413643005/

This CL introduces FetchEventPreloadHandle in fetch_event_dispatcher.mojom.
If the NavigationPreload feature is enabled:
 - ServiceWorkerURLRequestJob calls ServiceWorkerFetchDispatcher::
   MaybeStartNavigationPreload() even if the worker is not started yet.
 - ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload() will start the
   navigation preload request and create a FetchEventPreloadHandle.
   https://codereview.chromium.org/2410333006/ implements this method.
 - The FetchEventPreloadHandle will be passed to the service worker when
   DispatchFetchEvent() dispatches the FetchEvent.
 - ServiceWorkerContextClient::FetchEventDispatcherImpl receives the
   FetchEventPreloadHandle and will pass it to FetchEvent.
   https://codereview.chromium.org/2416843002/ implements this logic.

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

[commit-bot: I haz the power, 2016-10-18 09:08:45]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-10-18 09:12:01]

Description was changed from

==========
Introduce FetchEventPreloadHandle to pass the preload response to FetchEvent.

FetchEvent.navigationPreload will be implemented in the following steps.
[1/4] Introduce FetchEventPreloadHandle to pass the preload response to
FetchEvent.
      https://codereview.chromium.org/2417793002/ This CL.
[2/4] Implement ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload().
      https://codereview.chromium.org/2410333006/
[3/4] Implement FetchEvent.navigationPreload.
      https://codereview.chromium.org/2416843002/
[4/4] Add browser tests for NavigationPreload.
      https://codereview.chromium.org/2413643005/

This CL introduces FetchEventPreloadHandle in fetch_event_dispatcher.mojom.
If the NavigationPreload feature is enabled:
 - ServiceWorkerURLRequestJob calls ServiceWorkerFetchDispatcher::
   MaybeStartNavigationPreload() even if the worker is not started yet.
 - ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload() will start the
   navigation preload request and create a FetchEventPreloadHandle.
   https://codereview.chromium.org/2410333006/ implements this method.
 - The FetchEventPreloadHandle will be passed to the service worker when
   DispatchFetchEvent() dispatches the FetchEvent.
 - ServiceWorkerContextClient::FetchEventDispatcherImpl receives the
   FetchEventPreloadHandle and will pass it to FetchEvent.
   https://codereview.chromium.org/2416843002/ implements this logic.

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

to

==========
Introduce FetchEventPreloadHandle to pass the preload response to FetchEvent.

FetchEvent.navigationPreload will be implemented in the following steps.
[1/4] Introduce FetchEventPreloadHandle to pass the preload response to
FetchEvent.
      https://codereview.chromium.org/2417793002/ This CL.
[2/4] Implement ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload().
      https://codereview.chromium.org/2410333006/
[3/4] Implement FetchEvent.navigationPreload.
      https://codereview.chromium.org/2416843002/
[4/4] Add browser tests for NavigationPreload.
      https://codereview.chromium.org/2413643005/

This CL introduces FetchEventPreloadHandle in fetch_event_dispatcher.mojom.
If the NavigationPreload feature is enabled:
 - ServiceWorkerURLRequestJob calls ServiceWorkerFetchDispatcher::
   MaybeStartNavigationPreload() even if the worker is not started yet.
 - ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload() will start the
   navigation preload request and create a FetchEventPreloadHandle.
   https://codereview.chromium.org/2410333006/ implements this method.
 - The FetchEventPreloadHandle will be passed to the service worker when
   DispatchFetchEvent() dispatches the FetchEvent.
 - ServiceWorkerContextClient::FetchEventDispatcherImpl receives the
   FetchEventPreloadHandle and will pass it to FetchEvent.
   https://codereview.chromium.org/2416843002/ implements this logic.

BUG=649558

Committed: https://crrev.com/fc9f9dc28b5291e0582dc1fcb2fb35aa0409f34f
Cr-Commit-Position: refs/heads/master@{#425927}
==========

[commit-bot: I haz the power, 2016-10-18 09:12:02]

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