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

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

Created:
2 years, 3 months ago by horo
Modified:
2 years, 1 month 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, 1 month ago (2016-07-28 01:50:31 UTC) #29
horo
nhiroki@ Could you please review this?
2 years, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month ago (2016-07-29 08:59:04 UTC) #48
horo
mpearson@ Could you please review tools/metrics/histograms/histograms.xml?
2 years, 1 month ago (2016-07-29 09:02:12 UTC) #50
horo
rkaplow@ Could you please review histograms.xml?
2 years, 1 month ago (2016-08-02 01:41:15 UTC) #52
rkaplow
lgtm
2 years, 1 month 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, 1 month 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, 1 month 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, 1 month ago (2016-08-02 23:55:28 UTC) #58
commit-bot: I haz the power
Committed patchset #5 (id:320001)
2 years, 1 month ago (2016-08-03 02:22:55 UTC) #60
commit-bot: I haz the power
2 years, 1 month 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