ServiceWorker: Add prepare_callback in DispatchFetchEvent to notify the end of SW preparation.

ServiceWorker: Add prepare_callback in DispatchFetchEvent to notify the end of SW preparation.

This callback is needed to measure the performance of booting ServiceWorker.

BUG=401389
TEST=N/A

Committed: https://crrev.com/a27920ff436639a033cd36945542e89c3b913133
Cr-Commit-Position: refs/heads/master@{#292129}

[nhiroki, 2014-08-22 04:50:58]

drive-by-comment:

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_url_request_job.cc (right):

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_url_request_job.cc:327: void
ServiceWorkerURLRequestJob::DidPrepareFetchEvent() {
Can you add a comment here?

For example, if you will implement this function in a subsequent CL, you can
add...

  // TODO(shimazu): Implement this (http://crbug.com/XXXXXX).

Or, if you want to do nothing here, you can add...

  // Nothing to do.

In general, an empty body would say nothing about an implementer's intention and
a sourcecode reader cannot figure out whether the implementer just forgot to
make it or not.

[horo, 2014-08-22 05:11:51]

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_url_request_job.cc (right):

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_url_request_job.cc:327: void
ServiceWorkerURLRequestJob::DidPrepareFetchEvent() {
Please write TODO comments.

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_version.cc (right):

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_version.cc:301: request,
prepare_callback,
new line

[shimazu, 2014-08-25 02:17:40]

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_url_request_job.cc (right):

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_url_request_job.cc:327: void
ServiceWorkerURLRequestJob::DidPrepareFetchEvent() {
On 2014/08/22 04:50:58, nhiroki wrote:
> Can you add a comment here?
> 
> For example, if you will implement this function in a subsequent CL, you can
> add...
> 
>   // TODO(shimazu): Implement this (http://crbug.com/XXXXXX).
> 
> Or, if you want to do nothing here, you can add...
> 
>   // Nothing to do.
> 
> In general, an empty body would say nothing about an implementer's intention
and
> a sourcecode reader cannot figure out whether the implementer just forgot to
> make it or not.

Done.

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_url_request_job.cc:327: void
ServiceWorkerURLRequestJob::DidPrepareFetchEvent() {
On 2014/08/22 05:11:51, horo wrote:
> Please write TODO comments.

Done.

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_version.cc (right):

https://codereview.chromium.org/493373002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_version.cc:301: request,
prepare_callback,
On 2014/08/22 05:11:51, horo wrote:
> new line

Done.

[horo, 2014-08-25 02:23:16]

lgtm

[shimazu, 2014-08-27 01:42:46]

The CQ bit was checked by shimazu@chromium.org

[commit-bot: I haz the power, 2014-08-27 01:43:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/shimazu@chromium.org/493373002/20001

[nhiroki, 2014-08-27 01:52:09]

LGTM (sorry for my delayed response)

https://codereview.chromium.org/493373002/diff/20001/content/browser/service_...
File content/browser/service_worker/service_worker_fetch_dispatcher.cc (right):

https://codereview.chromium.org/493373002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_fetch_dispatcher.cc:71:
prepare_callback.Run();
nit: you can just call prepare_callback_.Run().

https://codereview.chromium.org/493373002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_fetch_dispatcher.cc:80:
fetch_callback.Run(status, fetch_result, response);
ditto.

[commit-bot: I haz the power, 2014-08-27 02:30:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/46731)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/52097)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-27 02:35:35]

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

[commit-bot: I haz the power, 2014-08-27 02:35:37]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[shimazu, 2014-08-27 04:47:55]

Test code is added, so please review again:)

[nhiroki, 2014-08-27 05:05:39]

lgtm2 with a nit

https://codereview.chromium.org/493373002/diff/40001/content/browser/service_...
File content/browser/service_worker/service_worker_browsertest.cc (right):

https://codereview.chromium.org/493373002/diff/40001/content/browser/service_...
content/browser/service_worker/service_worker_browsertest.cc:553:
CreatePrepareReceiver(prepare_result),
nit: I think we don't have to use this indirect call. How about calling
"base::Bind(&ReceivePrepareResult, is_prepared)" directly?

[horo, 2014-08-27 08:49:20]

lgtm 2

[shimazu, 2014-08-27 08:51:25]

The CQ bit was checked by shimazu@chromium.org

[commit-bot: I haz the power, 2014-08-27 08:51:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/shimazu@chromium.org/493373002/40001

[commit-bot: I haz the power, 2014-08-27 09:47:34]

Committed patchset #3 (40001) as f8c3c9462b1066c9a764f65d7f646e46aaf20a2e

[commit-bot: I haz the power, 2014-09-10 02:50:50]

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