Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(5)

Issue 2045153003: Speculatively launch Service Workers on mouse/touch events. [4/5] (Closed)

Created:
2 years, 5 months ago by horo
Modified:
2 years, 3 months ago
Reviewers:
Mark P, rkaplow, nhiroki
CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, blink-reviews-html_chromium.org, tzik, serviceworker-reviews, jam, kinuko+serviceworker, nhiroki, dglazkov+blink, darin-cc_chromium.org, asvitkine+watch_chromium.org, blink-reviews, horo+watch_chromium.org, kinuko+watch, blink-reviews-api_chromium.org, blink-worker-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Speculatively launch Service Workers on mouse/touch events. [4/5] 1/5: Introduce NavigationHintSender. https://codereview.chromium.org/2043863003/ 2/5: Pipe NavigationHints from NavigationHintSender to ChromeRenderMessageFilter https://codereview.chromium.org/2043083002/ 3/5: Call StartServiceWorkerForNavigationHint() from ChromeRenderMessageFilter https://codereview.chromium.org/2052613003/ 4/5: Measure the precision of the speculative launch of Service Workers for NavigationHints This CL. 5/5: Add flags to enable SupeculativeLaunchServiceWorker https://codereview.chromium.org/2053573002/ This CL introduces "ServiceWorker.NavigationHintPrecision" UMA to measure the precision of the speculative launch of Service Workers for navigation hints. It is recorded when the speculatively launched worker is stopped. If there was no event fetched to the worker, this value is false. This means that the speculative launch was meaningless. BUG=616502 Committed: https://crrev.com/b9d6af8dfc9c1bcaabe9d67dd6c30c30b3873cbb Cr-Commit-Position: refs/heads/master@{#409436}

Patch Set 1 #

Patch Set 2 : add test #

Total comments: 12

Patch Set 3 : incorporated nhiroki's comment #

Patch Set 4 : fix windows compile error #

Total comments: 10

Patch Set 5 : incorporated nhiroki's comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+274 lines, -9 lines) Patch
M content/browser/service_worker/service_worker_metrics.h View 1 2 2 chunks +10 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_metrics.cc View 1 2 2 chunks +34 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_version.h View 1 2 3 chunks +9 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_version.cc View 1 2 3 4 10 chunks +27 lines, -9 lines 0 comments Download
M content/browser/service_worker/service_worker_version_unittest.cc View 1 2 3 4 3 chunks +177 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 2 chunks +17 lines, -0 lines 0 comments Download

Messages

Total messages: 62 (48 generated)
horo
nhiroki@ Could you please review this?
2 years, 3 months ago (2016-07-28 01:50:31 UTC) #29
horo
nhiroki@ Could you please review this?
2 years, 3 months ago (2016-07-28 01:50:32 UTC) #30
nhiroki
https://codereview.chromium.org/2045153003/diff/260001/content/browser/service_worker/service_worker_metrics.cc File content/browser/service_worker/service_worker_metrics.cc (right): https://codereview.chromium.org/2045153003/diff/260001/content/browser/service_worker/service_worker_metrics.cc#newcode420 content/browser/service_worker/service_worker_metrics.cc:420: bool event_fired) { DCHECK(IsNavigationHintEvent(start_worker_purpose)) https://codereview.chromium.org/2045153003/diff/260001/content/browser/service_worker/service_worker_metrics.cc#newcode437 content/browser/service_worker/service_worker_metrics.cc:437: // Do nothing. ...
2 years, 3 months ago (2016-07-28 05:38:37 UTC) #31
horo
https://codereview.chromium.org/2045153003/diff/260001/content/browser/service_worker/service_worker_metrics.cc File content/browser/service_worker/service_worker_metrics.cc (right): https://codereview.chromium.org/2045153003/diff/260001/content/browser/service_worker/service_worker_metrics.cc#newcode420 content/browser/service_worker/service_worker_metrics.cc:420: bool event_fired) { On 2016/07/28 05:38:36, nhiroki wrote: > ...
2 years, 3 months ago (2016-07-28 08:15:48 UTC) #36
nhiroki
lgtm with minor comments https://codereview.chromium.org/2045153003/diff/300001/content/browser/service_worker/service_worker_version.cc File content/browser/service_worker/service_worker_version.cc (right): https://codereview.chromium.org/2045153003/diff/300001/content/browser/service_worker/service_worker_version.cc#newcode1761 content/browser/service_worker/service_worker_version.cc:1761: base::Optional<ServiceWorkerMetrics::EventType>(); start_worker_first_purpose = base::nullopt_t; https://codereview.chromium.org/2045153003/diff/300001/content/browser/service_worker/service_worker_version_unittest.cc ...
2 years, 3 months ago (2016-07-29 06:08:03 UTC) #43
horo
Thank you. https://codereview.chromium.org/2045153003/diff/300001/content/browser/service_worker/service_worker_version.cc File content/browser/service_worker/service_worker_version.cc (right): https://codereview.chromium.org/2045153003/diff/300001/content/browser/service_worker/service_worker_version.cc#newcode1761 content/browser/service_worker/service_worker_version.cc:1761: base::Optional<ServiceWorkerMetrics::EventType>(); On 2016/07/29 06:08:03, nhiroki wrote: > ...
2 years, 3 months ago (2016-07-29 08:59:04 UTC) #48
horo
mpearson@ Could you please review tools/metrics/histograms/histograms.xml?
2 years, 3 months ago (2016-07-29 09:02:12 UTC) #50
horo
rkaplow@ Could you please review histograms.xml?
2 years, 3 months ago (2016-08-02 01:41:15 UTC) #52
rkaplow
lgtm
2 years, 3 months ago (2016-08-02 17:07:51 UTC) #53
Mark P
On Fri, Jul 29, 2016 at 2:02 AM, <horo@chromium.org> wrote: > mpearson@ > Could you ...
2 years, 3 months ago (2016-08-02 21:10:46 UTC) #54
Mark P
On Fri, Jul 29, 2016 at 2:02 AM, <horo@chromium.org> wrote: > mpearson@ > Could you ...
2 years, 3 months ago (2016-08-02 21:10:47 UTC) #55
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2045153003/320001
2 years, 3 months ago (2016-08-02 23:55:28 UTC) #58
commit-bot: I haz the power
Committed patchset #5 (id:320001)
2 years, 3 months ago (2016-08-03 02:22:55 UTC) #60
commit-bot: I haz the power
2 years, 3 months ago (2016-08-03 02:24:36 UTC) #62
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/b9d6af8dfc9c1bcaabe9d67dd6c30c30b3873cbb
Cr-Commit-Position: refs/heads/master@{#409436}

Powered by Google App Engine
This is Rietveld 408576698