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

Issue 2466513002: Deprecate ServiceWorkerMessageEvent in favor of MessageEvent (Closed)

Created:
2 years, 7 months ago by jungkees
Modified:
2 years, 6 months ago
CC:
chromium-reviews, shimazu+worker_chromium.org, kinuko+worker_chromium.org, blink-reviews-bindings_chromium.org, blink-reviews, horo+watch_chromium.org, falken+watch_chromium.org, kinuko+watch, blink-worker-reviews_chromium.org, Rick Byers
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Deprecate ServiceWorkerMessageEvent in favor of MessageEvent This extends MessageEvent to allow ServiceWorker as a type of source attribute according to the changes in HTML and replaces the existing ServiceWorkerMessageEvent code usage with this extended MessageEvent. Related spec issue: https://github.com/w3c/ServiceWorker/issues/989 Related spec change: - HTML: https://github.com/whatwg/html/pull/1944 - SW: https://github.com/w3c/ServiceWorker/commit/6c68bd87bdfd153186a5b9904e31676c56730e2f Related WPT: https://github.com/w3c/web-platform-tests/pull/4186 Intent to Deprecate and Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Xp9hmKyuOrI BUG=659074 Committed: https://crrev.com/faa6203e513b171743c9d9fd7121501e681186af Cr-Commit-Position: refs/heads/master@{#438545}

Patch Set 1 #

Patch Set 2 : Use EventTarget for |source| #

Patch Set 3 : Remove ServiceWorkerMessageEvent codes; update layout tests #

Total comments: 6

Patch Set 4 : Merge layout tests; add arg name comments; cleanup global-interface-listing-expected files. #

Total comments: 2

Patch Set 5 : Remove a dupe assertion; remove ServiceWorkerMessageEvent interface listings for linux/win settings #

Patch Set 6 : Rebase #

Patch Set 7 : Update cross-origin-objects-exceptions-expected.txt #

Patch Set 8 : Rebase and update cross-origin-objects-exceptions-expected.txt #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -528 lines) Patch
M android_webview/tools/system_webview_shell/test/data/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 1 chunk +0 lines, -7 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/events/constructors/serviceworker-message-event-constructor.html View 1 2 3 1 chunk +0 lines, -136 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/events/constructors/serviceworker-message-event-constructor-expected.txt View 1 2 3 1 chunk +0 lines, -112 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html View 1 2 3 4 1 chunk +14 lines, -4 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/serviceworker/serviceworker-message-event.html View 1 2 1 chunk +0 lines, -43 lines 0 comments Download
M third_party/WebKit/LayoutTests/imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions-expected.txt View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 1 chunk +0 lines, -8 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 1 chunk +0 lines, -8 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 1 chunk +0 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8MessageEventCustom.cpp View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/modules/BUILD.gn View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
D third_party/WebKit/Source/bindings/modules/v8/custom/V8ServiceWorkerMessageEventCustom.cpp View 1 2 1 chunk +0 lines, -24 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/custom/custom.gni View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/generated.gni View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/events/EventTarget.h View 1 2 3 4 5 6 7 2 chunks +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/events/EventTarget.cpp View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/events/MessageEvent.h View 1 2 3 4 5 3 chunks +2 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/events/MessageEvent.cpp View 1 3 chunks +4 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/events/MessageEvent.idl View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/modules_idl_files.gni View 1 2 3 4 5 6 7 3 chunks +0 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/BUILD.gn View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorker.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainer.cpp View 1 2 3 4 5 6 7 3 chunks +4 lines, -4 lines 0 comments Download
D third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerMessageEvent.h View 1 2 1 chunk +0 lines, -72 lines 0 comments Download
D third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerMessageEvent.cpp View 1 2 1 chunk +0 lines, -83 lines 0 comments Download

Messages

Total messages: 101 (54 generated)
jungkees
While having tried this with idl-generated WindowOrMessagePortOrServiceWorker class, I encountered some irrelevant errors as follows: ...
2 years, 7 months ago (2016-10-31 12:12:57 UTC) #3
nhiroki
+haraken@ On 2016/10/31 12:12:57, jungkees wrote: > Also, even if we make this work, ServiceWorker ...
2 years, 7 months ago (2016-10-31 23:31:36 UTC) #6
bashi
On 2016/10/31 12:12:57, jungkees wrote: > While having tried this with idl-generated WindowOrMessagePortOrServiceWorker > class, ...
2 years, 7 months ago (2016-11-01 00:06:10 UTC) #7
haraken
On 2016/11/01 00:06:10, bashi1 (slow) wrote: > On 2016/10/31 12:12:57, jungkees wrote: > > While ...
2 years, 7 months ago (2016-11-01 02:42:44 UTC) #8
nhiroki
On 2016/11/01 02:42:44, haraken wrote: > On 2016/11/01 00:06:10, bashi1 (slow) wrote: > > On ...
2 years, 7 months ago (2016-11-01 04:12:18 UTC) #9
jungkees
On 2016/11/01 04:12:18, nhiroki wrote: > On 2016/11/01 02:42:44, haraken wrote: > > On 2016/11/01 ...
2 years, 7 months ago (2016-11-01 05:44:56 UTC) #10
haraken
On 2016/11/01 05:44:56, jungkees wrote: > On 2016/11/01 04:12:18, nhiroki wrote: > > On 2016/11/01 ...
2 years, 7 months ago (2016-11-01 06:25:15 UTC) #11
jungkees
On 2016/11/01 06:25:15, haraken wrote: > On 2016/11/01 05:44:56, jungkees wrote: > > On 2016/11/01 ...
2 years, 7 months ago (2016-11-01 07:04:19 UTC) #12
foolip
For the srcObject case, the fix was to move just that attribute to its own ...
2 years, 7 months ago (2016-11-09 22:09:12 UTC) #14
jungkees
On 2016/11/09 22:09:12, foolip wrote: > For the srcObject case, the fix was to move ...
2 years, 7 months ago (2016-11-10 01:09:49 UTC) #15
jungkees
nhiroki@, zino@, foolip@, I uploaded a snapshot for review. PTAL. (I encountered a 403 error ...
2 years, 7 months ago (2016-11-10 06:27:35 UTC) #19
foolip
On 2016/11/10 01:09:49, jungkees wrote: > On 2016/11/09 22:09:12, foolip wrote: > > For the ...
2 years, 7 months ago (2016-11-10 09:33:12 UTC) #20
jungkees
On 2016/11/10 09:33:12, foolip wrote: > On 2016/11/10 01:09:49, jungkees wrote: > > On 2016/11/09 ...
2 years, 7 months ago (2016-11-10 11:17:59 UTC) #23
zino
On 2016/11/10 11:17:59, jungkees wrote: > On 2016/11/10 09:33:12, foolip wrote: > > On 2016/11/10 ...
2 years, 7 months ago (2016-11-10 14:31:13 UTC) #26
zino
Hmm, I'm not expert in this part but it looks very good idea. Other reviewers, ...
2 years, 7 months ago (2016-11-10 14:35:38 UTC) #27
nhiroki
Sorry, I couldn't have time to review this CL today. I'll take a look early ...
2 years, 7 months ago (2016-11-11 11:34:36 UTC) #30
nhiroki
On 2016/11/10 06:27:35, jungkees wrote: > nhiroki@, zino@, foolip@, I uploaded a snapshot for review. ...
2 years, 7 months ago (2016-11-11 11:40:31 UTC) #31
jungkees
On 2016/11/11 11:34:36, nhiroki wrote: > Sorry, I couldn't have time to review this CL ...
2 years, 7 months ago (2016-11-11 14:37:21 UTC) #32
jungkees
On 2016/11/11 11:40:31, nhiroki wrote: > On 2016/11/10 06:27:35, jungkees wrote: > > nhiroki@, zino@, ...
2 years, 7 months ago (2016-11-11 14:43:52 UTC) #33
nhiroki
Looks good overall. https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html File third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html (right): https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html#newcode36 third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html:36: 'source should be ServiceWorker.'); Can you ...
2 years, 7 months ago (2016-11-14 05:48:55 UTC) #37
jungkees
I uploaded a new snapshot having addressed the comments. PTAL. https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html File third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html (right): https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html#newcode36 ...
2 years, 7 months ago (2016-11-14 14:17:11 UTC) #38
nhiroki
lgtm https://codereview.chromium.org/2466513002/diff/60001/third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html File third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html (right): https://codereview.chromium.org/2466513002/diff/60001/third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html#newcode29 third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html:29: 'ServiceWorkerMessageEvent should not be defined.'); This assertion wouldn't ...
2 years, 7 months ago (2016-11-15 03:42:15 UTC) #43
haraken
Implementation-wise LGTM. You'll need to get an approval of an API owner.
2 years, 7 months ago (2016-11-15 05:00:42 UTC) #44
jungkees
On 2016/11/15 05:00:42, haraken wrote: > Implementation-wise LGTM. > > You'll need to get an ...
2 years, 7 months ago (2016-11-15 05:08:00 UTC) #45
jungkees
I uploaded a new snapshot having addressed nhiroki@'s comment and removed ServiceWorkerMessageEvent interfaces from global-interface-listing-expected ...
2 years, 7 months ago (2016-11-15 16:06:49 UTC) #46
foolip
On 2016/11/15 05:08:00, jungkees wrote: > On 2016/11/15 05:00:42, haraken wrote: > > Implementation-wise LGTM. ...
2 years, 7 months ago (2016-11-15 16:16:43 UTC) #49
jungkees
On 2016/11/15 16:16:43, foolip wrote: > On 2016/11/15 05:08:00, jungkees wrote: > > On 2016/11/15 ...
2 years, 7 months ago (2016-11-16 05:10:58 UTC) #52
jungkees
On 2016/11/16 05:10:58, jungkees wrote: > On 2016/11/15 16:16:43, foolip wrote: > > On 2016/11/15 ...
2 years, 6 months ago (2016-12-08 06:06:11 UTC) #54
foolip
On 2016/12/08 06:06:11, jungkees wrote: > On 2016/11/16 05:10:58, jungkees wrote: > > On 2016/11/15 ...
2 years, 6 months ago (2016-12-08 11:48:49 UTC) #55
jungkees
I uploaded a new snapshot having rebased it. PTAL.
2 years, 6 months ago (2016-12-09 19:36:04 UTC) #64
zino
On 2016/12/09 19:36:04, jungkees wrote: > I uploaded a new snapshot having rebased it. PTAL. ...
2 years, 6 months ago (2016-12-09 19:37:18 UTC) #67
jungkees
> Oh, you looks hard worker. ;) zino@, you always stay up until that hour? ...
2 years, 6 months ago (2016-12-12 02:24:45 UTC) #70
jungkees
On 2016/12/12 02:24:45, jungkees wrote: > > Oh, you looks hard worker. ;) > > ...
2 years, 6 months ago (2016-12-13 01:14:47 UTC) #75
nhiroki
On 2016/12/13 01:14:47, jungkees wrote: > On 2016/12/12 02:24:45, jungkees wrote: > > > Oh, ...
2 years, 6 months ago (2016-12-13 02:22:22 UTC) #76
nhiroki
On 2016/12/13 02:22:22, nhiroki wrote: > On 2016/12/13 01:14:47, jungkees wrote: > > On 2016/12/12 ...
2 years, 6 months ago (2016-12-13 02:26:15 UTC) #77
jungkees
On 2016/12/13 02:26:15, nhiroki wrote: > On 2016/12/13 02:22:22, nhiroki wrote: > > On 2016/12/13 ...
2 years, 6 months ago (2016-12-13 02:35:54 UTC) #79
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/2466513002/120001
2 years, 6 months ago (2016-12-13 02:37:11 UTC) #82
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/324385)
2 years, 6 months ago (2016-12-13 02:47:49 UTC) #84
jungkees
On 2016/12/13 02:47:49, commit-bot: I haz the power wrote: > Try jobs failed on following ...
2 years, 6 months ago (2016-12-13 05:49:15 UTC) #86
boliu
On 2016/12/13 05:49:15, jungkees wrote: > On 2016/12/13 02:47:49, commit-bot: I haz the power wrote: ...
2 years, 6 months ago (2016-12-13 16:05:01 UTC) #87
jungkees
On 2016/12/13 16:05:01, boliu wrote: > On 2016/12/13 05:49:15, jungkees wrote: > > On 2016/12/13 ...
2 years, 6 months ago (2016-12-14 01:21:12 UTC) #88
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/2466513002/120001
2 years, 6 months ago (2016-12-14 01:22:10 UTC) #90
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/355854) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, ...
2 years, 6 months ago (2016-12-14 03:11:57 UTC) #92
jungkees
On 2016/12/14 03:11:57, commit-bot: I haz the power wrote: > Try jobs failed on following ...
2 years, 6 months ago (2016-12-14 16:09:28 UTC) #93
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/2466513002/140001
2 years, 6 months ago (2016-12-14 16:09:59 UTC) #96
commit-bot: I haz the power
Committed patchset #8 (id:140001)
2 years, 6 months ago (2016-12-14 18:39:05 UTC) #99
commit-bot: I haz the power
2 years, 6 months ago (2016-12-14 18:43:22 UTC) #101
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/faa6203e513b171743c9d9fd7121501e681186af
Cr-Commit-Position: refs/heads/master@{#438545}

Powered by Google App Engine
This is Rietveld 408576698