Commit 65e2f0f3 authored by Kenichi Ishibashi's avatar Kenichi Ishibashi Committed by Commit Bot

S13nServiceWorker: Fix `requestStart` performance timing for navigation

ServiceWorkerNavigationLoader is responsible for setting up LoadTimingInfo
which are used to create web-exposed PerformanceResourceTiming. Specifically,
the field |send_start| in LoadTimingInfo is populated as `requestStart` in
PerformanceResourceTiming.

The resource timing spec[1] doesn't state explicitly but according to a
spec discussion[2], following ordering would be reasonable:

 workerStart <= fetchStart <= requestStart

Before this CL, ServiceWorkerNavigationLoader recorded `requestStart` before
starting a service worker, at the same time `workerStart` is recorded. This
doesn't seem a good timing because `requestStart` would be less than
`fetchStart`, which is recorded after the service worker is started.
This CL moves the timing to record |send_start| (and corresponding |send_end|)
to the time the service worker is started.

Note that web-exposed `fetchStart` still doesn't follow the above ordering
even after this CL, as we have a special case code for `fetchStart` in navigation.
A follow-up CL will address `fetchStart` case.

The wpt service-workers/service-worker/navigation-timing.https.html should
cover this change.

[1] https://w3c.github.io/resource-timing/#sec-performanceresourcetiming
[2] https://github.com/w3c/resource-timing/issues/119

Bug: 782958
Change-Id: I339d41d8c93a2cbcd053d4348d8e0f51b1134925
Reviewed-on: https://chromium-review.googlesource.com/1212423Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589458}
parent e3235dc9
......@@ -234,9 +234,10 @@ void ServiceWorkerNavigationLoader::StartRequest(
fetch_dispatcher_->MaybeStartNavigationPreloadWithURLLoader(
resource_request_, url_loader_factory_getter_.get(),
base::DoNothing(/* TODO(crbug/762357): metrics? */));
// Record worker start time here as |fetch_dispatcher_| will start a service
// worker if there is no running service worker.
response_head_.service_worker_start_time = base::TimeTicks::Now();
response_head_.load_timing.send_start = base::TimeTicks::Now();
response_head_.load_timing.send_end = base::TimeTicks::Now();
fetch_dispatcher_->Run();
}
......@@ -276,7 +277,12 @@ void ServiceWorkerNavigationLoader::DidPrepareFetchEvent(
"initial_worker_status",
EmbeddedWorkerInstance::StatusToString(initial_worker_status));
response_head_.service_worker_ready_time = base::TimeTicks::Now();
// At this point a service worker is running and the fetch event is about
// to dispatch. Record some load timings.
base::TimeTicks now = base::TimeTicks::Now();
response_head_.service_worker_ready_time = now;
response_head_.load_timing.send_start = now;
response_head_.load_timing.send_end = now;
// Note that we don't record worker preparation time in S13nServiceWorker
// path for now. If we want to measure worker preparation time we can
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment