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

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

Created:
4 years, 6 months ago by horo
Modified:
4 years, 6 months ago
CC:
chromium-reviews, blink-reviews, dglazkov+blink, blink-reviews-html_chromium.org, kinuko+watch, blink-reviews-api_chromium.org, serviceworker-reviews
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. [1/5] 1/5: Introduce NavigationHintSender. This CL. 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 https://codereview.chromium.org/2045153003/ 5/5: Add flags to enable SupeculativeLaunchServiceWorker https://codereview.chromium.org/2053573002/ We will introduces a new feature named "SpeculativeLaunchServiceWorker". When the feature is enabled, HTMLAnchorElement will have NavigationHintSender. It is similar to PrefetchEventHandler which was removed by https://codereview.chromium.org/344133002. When HTMLAnchorElement receives mousedown events for the left button or gesturetapunconfirmed or gestureshowpress events, a NavigationHint IPC message will be sent to the browser process. IPC related code is in [2/5] https://codereview.chromium.org/2043083002/ BUG=616502 Committed: https://crrev.com/cca7ec0e84c6ea291d90a79bfdcbcd5f0a55d612 Cr-Commit-Position: refs/heads/master@{#401225}

Patch Set 1 : #

Total comments: 5

Patch Set 2 : Remove DECLARE_VIRTUAL_TRACE_WRAPPERS #

Total comments: 4

Patch Set 3 : remove TRACE_WRAPPERS #

Total comments: 7

Patch Set 4 : incorporated falken's comment #

Total comments: 8

Patch Set 5 : incorporated falken's comment #

Total comments: 2

Patch Set 6 : incorporated kinuko's comment #

Patch Set 7 : NavigationHint #

Patch Set 8 : remove RuntimeEnabledFeatures.h include #

Patch Set 9 : rebase #

Total comments: 4

Patch Set 10 : incorporated kinuko's comment #

Total comments: 2

Patch Set 11 : move WebNavigationHintType to in NavigationHintSender #

Total comments: 2

Patch Set 12 : use DECLARE_TRACE #

Unified diffs Side-by-side diffs Delta from patch set Stats (+100 lines, -0 lines) Patch
M third_party/WebKit/Source/core/html/HTMLAnchorElement.h View 1 2 3 4 6 3 chunks +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +94 lines, -0 lines 0 comments Download

Messages

Total messages: 47 (14 generated)
horo
kouhei@, falken@ Could you please review this CL?
4 years, 6 months ago (2016-06-07 16:47:35 UTC) #5
kouhei (in TOK)
Please split the CLs more: Introduce NavigationHintSender, Use NavigationHintSender. Should we add UMAs to track ...
4 years, 6 months ago (2016-06-09 02:31:13 UTC) #7
haraken
https://codereview.chromium.org/2043863003/diff/40001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/40001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode56 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:56: DECLARE_VIRTUAL_TRACE_WRAPPERS(); On 2016/06/09 02:31:13, kouhei wrote: > I'm not ...
4 years, 6 months ago (2016-06-09 04:07:29 UTC) #8
horo
> Please split the CLs more: Introduce NavigationHintSender, Use > NavigationHintSender. Done. > Should we ...
4 years, 6 months ago (2016-06-09 05:27:58 UTC) #12
kouhei (in TOK)
https://codereview.chromium.org/2043863003/diff/60001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/60001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode139 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:139: DEFINE_TRACE_WRAPPERS(HTMLAnchorElement) ditto https://codereview.chromium.org/2043863003/diff/60001/third_party/WebKit/Source/core/html/HTMLAnchorElement.h File third_party/WebKit/Source/core/html/HTMLAnchorElement.h (right): https://codereview.chromium.org/2043863003/diff/60001/third_party/WebKit/Source/core/html/HTMLAnchorElement.h#newcode91 third_party/WebKit/Source/core/html/HTMLAnchorElement.h:91: DECLARE_VIRTUAL_TRACE_WRAPPERS(); ...
4 years, 6 months ago (2016-06-09 08:01:20 UTC) #13
horo
https://codereview.chromium.org/2043863003/diff/60001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/60001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode139 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:139: DEFINE_TRACE_WRAPPERS(HTMLAnchorElement) On 2016/06/09 08:01:19, kouhei wrote: > ditto Done. ...
4 years, 6 months ago (2016-06-09 08:20:46 UTC) #14
kouhei (in TOK)
ps3 lgtm
4 years, 6 months ago (2016-06-09 08:24:22 UTC) #15
horo
kinuko@ Could you please review third_party/WebKit/public/platform/WebNavigationHintType.h?
4 years, 6 months ago (2016-06-09 09:09:27 UTC) #17
falken
https://codereview.chromium.org/2043863003/diff/80001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/80001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode88 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:88: if (!url.protocolIsInHTTPFamily()) Why is it restricted to HTTP? Is ...
4 years, 6 months ago (2016-06-09 13:24:16 UTC) #18
kinuko
(Looking reasonable to me in general mod the points Matt commented, I'm also concerned a ...
4 years, 6 months ago (2016-06-14 06:38:59 UTC) #19
horo
https://codereview.chromium.org/2043863003/diff/80001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/80001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode88 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:88: if (!url.protocolIsInHTTPFamily()) On 2016/06/09 13:24:16, falken ooo till Jun ...
4 years, 6 months ago (2016-06-14 11:03:14 UTC) #20
falken
I'm probably outside of my depth (not an OWNER of this code), but some more ...
4 years, 6 months ago (2016-06-15 13:48:19 UTC) #21
horo
https://codereview.chromium.org/2043863003/diff/100001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/100001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode95 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:95: return false; On 2016/06/15 13:48:19, falken ooo till Jun ...
4 years, 6 months ago (2016-06-16 06:02:44 UTC) #22
kinuko
https://codereview.chromium.org/2043863003/diff/120001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/120001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode48 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:48: class HTMLAnchorElement::NavigationHintSender : public GarbageCollected<HTMLAnchorElement::NavigationHintSender> { Now it looks ...
4 years, 6 months ago (2016-06-16 06:14:31 UTC) #23
horo
https://codereview.chromium.org/2043863003/diff/120001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/120001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode48 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:48: class HTMLAnchorElement::NavigationHintSender : public GarbageCollected<HTMLAnchorElement::NavigationHintSender> { On 2016/06/16 06:14:31, ...
4 years, 6 months ago (2016-06-16 07:02:14 UTC) #24
kinuko
I think my question was whether we want to introduce as a SW specific fixture ...
4 years, 6 months ago (2016-06-16 07:19:12 UTC) #25
horo
On 2016/06/16 07:19:12, kinuko wrote: > I think my question was whether we want to ...
4 years, 6 months ago (2016-06-16 07:40:33 UTC) #26
kinuko
On 2016/06/16 07:40:33, horo (slow till June 20) wrote: > On 2016/06/16 07:19:12, kinuko wrote: ...
4 years, 6 months ago (2016-06-16 09:11:39 UTC) #27
horo
On 2016/06/16 09:11:39, kinuko wrote: > On 2016/06/16 07:40:33, horo (slow till June 20) wrote: ...
4 years, 6 months ago (2016-06-16 09:21:56 UTC) #28
kinuko
On 2016/06/16 09:21:56, horo (slow till June 20) wrote: > On 2016/06/16 09:11:39, kinuko wrote: ...
4 years, 6 months ago (2016-06-17 02:13:02 UTC) #29
falken
On 2016/06/17 02:13:02, kinuko wrote: > On 2016/06/16 09:21:56, horo (slow till June 20) wrote: ...
4 years, 6 months ago (2016-06-17 08:37:22 UTC) #30
horo
On 2016/06/17 08:37:22, falken wrote: > On 2016/06/17 02:13:02, kinuko wrote: > > On 2016/06/16 ...
4 years, 6 months ago (2016-06-17 19:59:53 UTC) #32
kinuko
https://codereview.chromium.org/2043863003/diff/220001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/220001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode87 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:87: if (!url.protocolIsInHTTPFamily()) nit: please add a brief comment about ...
4 years, 6 months ago (2016-06-20 04:53:59 UTC) #33
horo
https://codereview.chromium.org/2043863003/diff/220001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/220001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode87 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:87: if (!url.protocolIsInHTTPFamily()) On 2016/06/20 04:53:59, kinuko wrote: > nit: ...
4 years, 6 months ago (2016-06-21 04:04:31 UTC) #34
kinuko
lgtm https://codereview.chromium.org/2043863003/diff/240001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/240001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode46 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:46: enum class WebNavigationHintType { nit: It's supposed to ...
4 years, 6 months ago (2016-06-21 04:51:25 UTC) #35
horo
https://codereview.chromium.org/2043863003/diff/240001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/240001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode46 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:46: enum class WebNavigationHintType { On 2016/06/21 04:51:25, kinuko wrote: ...
4 years, 6 months ago (2016-06-21 05:05:47 UTC) #36
falken
not sure if you were waiting for me too... non-owner lgtm
4 years, 6 months ago (2016-06-22 03:36:05 UTC) #37
kouhei (in TOK)
https://codereview.chromium.org/2043863003/diff/260001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/260001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode62 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:62: DECLARE_VIRTUAL_TRACE(); This should be DECLARE_TRACE, or you can DEFINE_INLINE_TRACE
4 years, 6 months ago (2016-06-22 03:38:23 UTC) #38
horo
https://codereview.chromium.org/2043863003/diff/260001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp File third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp (right): https://codereview.chromium.org/2043863003/diff/260001/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp#newcode62 third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp:62: DECLARE_VIRTUAL_TRACE(); On 2016/06/22 03:38:23, kouhei wrote: > This should ...
4 years, 6 months ago (2016-06-22 05:04:31 UTC) #39
kouhei (in TOK)
OWNER lgtm
4 years, 6 months ago (2016-06-22 06:23:38 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2043863003/280001
4 years, 6 months ago (2016-06-22 06:26:01 UTC) #43
commit-bot: I haz the power
Committed patchset #12 (id:280001)
4 years, 6 months ago (2016-06-22 07:40:36 UTC) #45
commit-bot: I haz the power
4 years, 6 months ago (2016-06-22 07:42:34 UTC) #47
Message was sent while issue was closed.
Patchset 12 (id:??) landed as
https://crrev.com/cca7ec0e84c6ea291d90a79bfdcbcd5f0a55d612
Cr-Commit-Position: refs/heads/master@{#401225}

Powered by Google App Engine
This is Rietveld 408576698