Add workerReady timing for ServiceWorker controlled requests [2/3]

Add workerReady timing for ServiceWorker controlled requests [2/3]

To make ServiceWorker startup time measureable, in addition to
workerStart, we need another timing "workerReady" which is the point
ServiceWorker finished starting and is ready to handle events.

This timing is marked in DidPrepareFetchEvent, as well as send_start,
but unlike send_start, it's not cleared by request job restarts.

[1/3] https://codereview.chromium.org/1149273003/
[2/3] THIS
[3/3] https://codereview.chromium.org/1166953007/

BUG=465640
TEST=all existing tests should pass

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

[Kunihiko Sakamoto, 2015-06-05 07:58:25]

ksakamoto@chromium.org changed reviewers:
+ horo@chromium.org, jam@chromium.org, mmenke@chromium.org, tsepez@chromium.org

[Kunihiko Sakamoto, 2015-06-05 07:58:26]

content/browser/service_worker: horo@
content/browser/loader: mmenke@
content/{child, public/common}: jam@
content/common/resource_messages.h: tsepez@

PTAL, thanks!

[horo, 2015-06-05 08:10:57]

lgtm

[mmenke, 2015-06-05 15:24:02]

loader/ LGTM

https://codereview.chromium.org/1146253004/diff/1/content/browser/loader/reso...
File content/browser/loader/resource_loader.cc (right):

https://codereview.chromium.org/1146253004/diff/1/content/browser/loader/reso...
content/browser/loader/resource_loader.cc:72:
&response->head.service_worker_ready_time);
Hrm...Wonder if we should just pass in response, or &response->head.  Fine
as-is, regardless.

[Tom Sepez, 2015-06-05 15:33:07]

Messages LGTM.

https://codereview.chromium.org/1146253004/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_controllee_request_handler.cc
(right):

https://codereview.chromium.org/1146253004/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_controllee_request_handler.cc:68:
// Save worker timeings of the first job.
nit: sp. timings

[jam, 2015-06-05 15:39:23]

lgtm

[Kunihiko Sakamoto, 2015-06-08 01:51:44]

Horo-san, can you take another look at service_worker/?

https://codereview.chromium.org/1146253004/diff/1/content/browser/loader/reso...
File content/browser/loader/resource_loader.cc (right):

https://codereview.chromium.org/1146253004/diff/1/content/browser/loader/reso...
content/browser/loader/resource_loader.cc:72:
&response->head.service_worker_ready_time);
On 2015/06/05 15:24:02, mmenke wrote:
> Hrm...Wonder if we should just pass in response, or &response->head.  Fine
> as-is, regardless.

That makes sense, done.

https://codereview.chromium.org/1146253004/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_controllee_request_handler.cc
(right):

https://codereview.chromium.org/1146253004/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_controllee_request_handler.cc:68:
// Save worker timeings of the first job.
On 2015/06/05 15:33:06, Tom Sepez wrote:
> nit: sp. timings

Done.

[mmenke, 2015-06-08 02:08:42]

Loader still LGTM, just two nits.

https://codereview.chromium.org/1146253004/diff/20001/content/browser/loader/...
File content/browser/loader/resource_loader.cc (right):

https://codereview.chromium.org/1146253004/diff/20001/content/browser/loader/...
content/browser/loader/resource_loader.cc:66:
handler->GetExtraResponseInfo(&response->head);
nit:  Should still use braces, because the condition takes more than one line.

https://codereview.chromium.org/1146253004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_context_request_handler.h
(right):

https://codereview.chromium.org/1146253004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_context_request_handler.h:32: void
GetExtraResponseInfo(ResourceResponseInfo* response_info) const override;
Should forward declare ResourceResponseInfo.

[Kunihiko Sakamoto, 2015-06-08 02:25:57]

https://codereview.chromium.org/1146253004/diff/20001/content/browser/loader/...
File content/browser/loader/resource_loader.cc (right):

https://codereview.chromium.org/1146253004/diff/20001/content/browser/loader/...
content/browser/loader/resource_loader.cc:66:
handler->GetExtraResponseInfo(&response->head);
On 2015/06/08 02:08:42, mmenke wrote:
> nit:  Should still use braces, because the condition takes more than one line.

Done.

https://codereview.chromium.org/1146253004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_context_request_handler.h
(right):

https://codereview.chromium.org/1146253004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_context_request_handler.h:32: void
GetExtraResponseInfo(ResourceResponseInfo* response_info) const override;
On 2015/06/08 02:08:42, mmenke wrote:
> Should forward declare ResourceResponseInfo.

Since this is an overridden function, I think it's ok to put forward-declaration
only in the base class's header file.

[horo, 2015-06-08 03:25:50]

> Horo-san, can you take another look at service_worker/?

lgtm.

[Kunihiko Sakamoto, 2015-06-08 03:57:45]

The CQ bit was checked by ksakamoto@chromium.org

[Kunihiko Sakamoto, 2015-06-08 03:57:45]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jam@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/1146253004/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-06-08 03:57:51]

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

[commit-bot: I haz the power, 2015-06-08 04:33:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-08 04:34:23]

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