Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(74)

Issue 2218943002: Introduce ServiceWorker.EventDispatchingDelay UMA (Closed)

Created:
4 years, 4 months ago by horo
Modified:
4 years, 4 months ago
CC:
blink-reviews, blink-reviews-api_chromium.org, blink-worker-reviews_chromium.org, chromium-reviews, darin-cc_chromium.org, dglazkov+blink, falken, horo+watch_chromium.org, jam, jsbell+serviceworker_chromium.org, kinuko+watch, kinuko+serviceworker, michaeln, mlamouri+watch-content_chromium.org, nhiroki, serviceworker-reviews, tzik
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Introduce ServiceWorker.EventDispatchingDelay UMA This UMA records the time taken between sending an event IPC from the browser process to a Service Worker and executing the event handler in the Service Worker. This CL depends on https://codereview.chromium.org/2249063004/. BUG=561209 Committed: https://crrev.com/b713230833c44dab8148b531dfe64a3c919620d4 Cr-Commit-Position: refs/heads/master@{#414214}

Patch Set 1 : all #

Patch Set 2 : diff from https://codereview.chromium.org/2249063004/#ps40001 #

Total comments: 2

Patch Set 3 : EventDispatchingDelay #

Total comments: 3

Patch Set 4 : add comment in background_sync.mojom #

Total comments: 4

Patch Set 5 : incorporated mpearson@'s comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+431 lines, -217 lines) Patch
M content/browser/background_sync/background_sync_manager.cc View 1 2 2 chunks +5 lines, -2 lines 0 comments Download
M content/browser/service_worker/embedded_worker_test_helper.cc View 1 2 5 chunks +8 lines, -6 lines 0 comments Download
M content/browser/service_worker/service_worker_browsertest.cc View 1 2 3 1 chunk +4 lines, -2 lines 0 comments Download
M content/browser/service_worker/service_worker_context_unittest.cc View 1 2 3 chunks +5 lines, -5 lines 0 comments Download
M content/browser/service_worker/service_worker_dispatcher_host_unittest.cc View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M content/browser/service_worker/service_worker_fetch_dispatcher.cc View 1 2 2 chunks +4 lines, -2 lines 0 comments Download
M content/browser/service_worker/service_worker_job_unittest.cc View 1 2 2 chunks +5 lines, -4 lines 0 comments Download
M content/browser/service_worker/service_worker_metrics.h View 1 2 2 chunks +8 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_metrics.cc View 1 2 3 9 chunks +64 lines, -18 lines 0 comments Download
M content/browser/service_worker/service_worker_register_job.h View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M content/browser/service_worker/service_worker_register_job.cc View 1 2 2 chunks +5 lines, -2 lines 0 comments Download
M content/browser/service_worker/service_worker_registration_unittest.cc View 1 2 5 chunks +9 lines, -4 lines 0 comments Download
M content/browser/service_worker/service_worker_url_request_job_unittest.cc View 1 2 7 chunks +16 lines, -10 lines 0 comments Download
M content/browser/service_worker/service_worker_version.h View 1 2 5 chunks +10 lines, -4 lines 0 comments Download
M content/browser/service_worker/service_worker_version.cc View 1 2 4 chunks +19 lines, -8 lines 0 comments Download
M content/browser/service_worker/service_worker_version_unittest.cc View 1 2 19 chunks +40 lines, -22 lines 0 comments Download
M content/common/service_worker/service_worker_messages.h View 1 2 2 chunks +25 lines, -16 lines 0 comments Download
M content/renderer/background_sync/background_sync_client_impl.cc View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M content/renderer/service_worker/service_worker_context_client.h View 1 2 2 chunks +24 lines, -15 lines 0 comments Download
M content/renderer/service_worker/service_worker_context_client.cc View 1 2 4 chunks +45 lines, -23 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/RespondWithObserver.h View 2 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/RespondWithObserver.cpp View 4 chunks +8 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeClient.h View 1 2 1 chunk +10 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.h View 1 2 1 chunk +4 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.cpp View 1 2 3 chunks +9 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/web/ServiceWorkerGlobalScopeClientImpl.h View 1 2 1 chunk +10 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/web/ServiceWorkerGlobalScopeClientImpl.cpp View 1 2 1 chunk +20 lines, -20 lines 0 comments Download
M third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp View 1 2 4 chunks +5 lines, -3 lines 0 comments Download
M third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom View 1 2 3 1 chunk +3 lines, -1 line 0 comments Download
M third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextClient.h View 1 2 2 chunks +10 lines, -10 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 3 chunks +44 lines, -0 lines 0 comments Download

Messages

Total messages: 81 (58 generated)
horo
shimazu@ Could you please review this?
4 years, 4 months ago (2016-08-17 07:35:10 UTC) #36
shimazu
https://codereview.chromium.org/2218943002/diff/140001/content/browser/service_worker/service_worker_metrics.cc File content/browser/service_worker/service_worker_metrics.cc (right): https://codereview.chromium.org/2218943002/diff/140001/content/browser/service_worker/service_worker_metrics.cc#newcode458 content/browser/service_worker/service_worker_metrics.cc:458: void ServiceWorkerMetrics::RecordFetchEventDispatchingDelay( I think this could be open to ...
4 years, 4 months ago (2016-08-18 04:58:53 UTC) #37
horo
https://codereview.chromium.org/2218943002/diff/140001/content/browser/service_worker/service_worker_metrics.cc File content/browser/service_worker/service_worker_metrics.cc (right): https://codereview.chromium.org/2218943002/diff/140001/content/browser/service_worker/service_worker_metrics.cc#newcode458 content/browser/service_worker/service_worker_metrics.cc:458: void ServiceWorkerMetrics::RecordFetchEventDispatchingDelay( On 2016/08/18 04:58:53, shimazu wrote: > I ...
4 years, 4 months ago (2016-08-22 14:57:35 UTC) #48
shimazu
lgtm
4 years, 4 months ago (2016-08-23 02:09:18 UTC) #51
horo
iclelland@ Please review: content/browser/background_sync/background_sync_manager.cc content/renderer/background_sync/background_sync_client_impl.cc haraken@ Please review: third_party/WebKit/Source/web/ServiceWorkerGlobalScopeClientImpl.h third_party/WebKit/Source/web/ServiceWorkerGlobalScopeClientImpl.cpp third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp
4 years, 4 months ago (2016-08-23 02:50:53 UTC) #53
horo
dcheng@ Please review content/common/service_worker/service_worker_messages.h third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom
4 years, 4 months ago (2016-08-23 03:21:20 UTC) #55
horo
mpearson@ Could you please review histograms.xml?
4 years, 4 months ago (2016-08-23 03:22:08 UTC) #57
haraken
WebKit/Source/ LGTM
4 years, 4 months ago (2016-08-23 04:00:57 UTC) #58
dcheng
https://codereview.chromium.org/2218943002/diff/180001/third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom File third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom (right): https://codereview.chromium.org/2218943002/diff/180001/third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom#newcode52 third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom:52: => (ServiceWorkerEventStatus status, double dispatch_event_time); Is it possible to ...
4 years, 4 months ago (2016-08-23 05:37:21 UTC) #59
horo
https://codereview.chromium.org/2218943002/diff/180001/third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom File third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom (right): https://codereview.chromium.org/2218943002/diff/180001/third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom#newcode52 third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom:52: => (ServiceWorkerEventStatus status, double dispatch_event_time); On 2016/08/23 05:37:21, dcheng ...
4 years, 4 months ago (2016-08-23 07:01:59 UTC) #60
Marijn Kruisselbrink
https://codereview.chromium.org/2218943002/diff/180001/third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom File third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom (right): https://codereview.chromium.org/2218943002/diff/180001/third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom#newcode52 third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom:52: => (ServiceWorkerEventStatus status, double dispatch_event_time); On 2016/08/23 at 07:01:58, ...
4 years, 4 months ago (2016-08-23 19:04:30 UTC) #61
horo
On 2016/08/23 19:04:30, Marijn Kruisselbrink wrote: > https://codereview.chromium.org/2218943002/diff/180001/third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom > File > third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom > (right): > ...
4 years, 4 months ago (2016-08-23 23:39:00 UTC) #62
horo
On 2016/08/23 19:04:30, Marijn Kruisselbrink wrote: > https://codereview.chromium.org/2218943002/diff/180001/third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom > File > third_party/WebKit/public/platform/modules/background_sync/background_sync.mojom > (right): > ...
4 years, 4 months ago (2016-08-23 23:39:00 UTC) #63
dcheng
On 2016/08/23 23:39:00, horo wrote: > On 2016/08/23 19:04:30, Marijn Kruisselbrink wrote: > > > ...
4 years, 4 months ago (2016-08-24 00:17:58 UTC) #64
horo
On 2016/08/24 00:17:58, dcheng wrote: > On 2016/08/23 23:39:00, horo wrote: > > On 2016/08/23 ...
4 years, 4 months ago (2016-08-24 00:44:20 UTC) #66
horo
mpearson@ Could you please review histograms.xml?
4 years, 4 months ago (2016-08-24 01:57:22 UTC) #68
iclelland
Background Sync code LGTM, thanks!
4 years, 4 months ago (2016-08-24 03:54:23 UTC) #71
Mark P
By the way, you're (implicitly) adding 18 histograms x 5 suffixes that apply to them. ...
4 years, 4 months ago (2016-08-24 03:59:29 UTC) #72
horo
Thank you https://codereview.chromium.org/2218943002/diff/200001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/2218943002/diff/200001/tools/metrics/histograms/histograms.xml#oldcode103103 tools/metrics/histograms/histograms.xml:103103: <histogram_suffixes name="ServiceWorkerSpecialApps" separator="."> On 2016/08/24 03:59:29, Mark ...
4 years, 4 months ago (2016-08-24 05:14:01 UTC) #73
Mark P
histograms.xml lgtm
4 years, 4 months ago (2016-08-24 20:48:38 UTC) #74
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/2218943002/220001
4 years, 4 months ago (2016-08-24 22:19:31 UTC) #77
commit-bot: I haz the power
Committed patchset #5 (id:220001)
4 years, 4 months ago (2016-08-25 00:18:45 UTC) #79
commit-bot: I haz the power
4 years, 4 months ago (2016-08-25 00:20:23 UTC) #81
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/b713230833c44dab8148b531dfe64a3c919620d4
Cr-Commit-Position: refs/heads/master@{#414214}

Powered by Google App Engine
This is Rietveld 408576698