Deprecate ServiceWorkerMessageEvent in favor of MessageEvent

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}

[jungkees, 2016-10-31 12:10:46]

Description was changed from

==========
(WIP) Extend Message Event

HTML extends MessageEvent to allow ServiceWorker as a type of source attribute.
Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent. This patch incorporates these spec changes.

BUG=659074

R=nhiroki@chromium.org, jinho.bang@samsung.com
==========

to

==========
(WIP) Extend Message Event

HTML extends MessageEvent to allow ServiceWorker as a type of source attribute.
Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent. This patch incorporates these spec changes.

BUG=659074

R=nhiroki@chromium.org, jinho.bang@samsung.com
==========

[jungkees, 2016-10-31 12:10:47]

jungkee.song@samsung.com changed reviewers:
+ bashi@chromium.org, foolip@chromium.org, rbyers@chromium.org

[jungkees, 2016-10-31 12:12:57]

While having tried this with idl-generated WindowOrMessagePortOrServiceWorker
class, I encountered some irrelevant errors as follows:

In file included from
gen/blink/bindings/core/v8/WindowOrMessagePortOrServiceWorker.cpp:15:
../../third_party/WebKit/Source/core/css/cssom/WindowGetComputedStyle.h:22:45:
error: cannot initialize a parameter of type 'blink::Node *' with an lvalue of
type 'blink::Element *'
        CSSComputedStyleDeclaration::create(element, false, pseudoElement);
                                            ^~~~~~~
../../third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.h:52:13:
note: passing argument to parameter 'node' here
      Node* node,

It seems using Window as a union causes this error somehow. Testing with other
combinations of types in unions complile successfully.

bashi@ do you have any idea about this? Is there any history why the union type
couldn't be done with Window?

Also, even if we make this work, ServiceWorker class should be able to be
accessed from within core, which sounds like an unwanted layer violation? The
interfaces of the other workers are defined under core, but SW has its code
under modules. @nhiroki, could you comment on it?

[Rick Byers, 2016-10-31 13:55:35]

rbyers@chromium.org changed reviewers:
- rbyers@chromium.org

[nhiroki, 2016-10-31 23:28:25]

nhiroki@chromium.org changed reviewers:
+ haraken@chromium.org

[nhiroki, 2016-10-31 23:31:36]

+haraken@

On 2016/10/31 12:12:57, jungkees wrote:
> Also, even if we make this work, ServiceWorker class should be able to be
> accessed from within core, which sounds like an unwanted layer violation? The
> interfaces of the other workers are defined under core, but SW has its code
> under modules. @nhiroki, could you comment on it?

Good point. I have no ideas other than moving service worker components from
modules/ to core/. haraken@ who is an expert of Blink architecture may have
better ideas.

haraken@, any thoughts on this?

[bashi, 2016-11-01 00:06:10]

On 2016/10/31 12:12:57, jungkees wrote:
> While having tried this with idl-generated WindowOrMessagePortOrServiceWorker
> class, I encountered some irrelevant errors as follows:
> 
> In file included from
> gen/blink/bindings/core/v8/WindowOrMessagePortOrServiceWorker.cpp:15:
> ../../third_party/WebKit/Source/core/css/cssom/WindowGetComputedStyle.h:22:45:
> error: cannot initialize a parameter of type 'blink::Node *' with an lvalue of
> type 'blink::Element *'
>         CSSComputedStyleDeclaration::create(element, false, pseudoElement);
>                                             ^~~~~~~
> ../../third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.h:52:13:
> note: passing argument to parameter 'node' here
>       Node* node,
> 
> It seems using Window as a union causes this error somehow. Testing with other
> combinations of types in unions complile successfully.
> 
> bashi@ do you have any idea about this? Is there any history why the union
type
> couldn't be done with Window?
The problem is that WindowGetComputedStyle::getComputedStyleMap() is defined in
the header file. This means that WindowGetComputedStyle.h needs to include
Node.h and Element.h but I guess it won't work. A right solution would be moving
getComputedStyleMap() in .cpp file.
> 
> Also, even if we make this work, ServiceWorker class should be able to be
> accessed from within core, which sounds like an unwanted layer violation? The
> interfaces of the other workers are defined under core, but SW has its code
> under modules. @nhiroki, could you comment on it?

Unfortunately I don't think we have a solution of this kind of layer violation
for now :( In the current form of core/modules separation model, it's very hard
to support union type which have to be defined in core/ but have to refer a type
defined in modules/.

[haraken, 2016-11-01 02:42:44]

On 2016/11/01 00:06:10, bashi1 (slow) wrote:
> On 2016/10/31 12:12:57, jungkees wrote:
> > While having tried this with idl-generated
WindowOrMessagePortOrServiceWorker
> > class, I encountered some irrelevant errors as follows:
> > 
> > In file included from
> > gen/blink/bindings/core/v8/WindowOrMessagePortOrServiceWorker.cpp:15:
> >
../../third_party/WebKit/Source/core/css/cssom/WindowGetComputedStyle.h:22:45:
> > error: cannot initialize a parameter of type 'blink::Node *' with an lvalue
of
> > type 'blink::Element *'
> >         CSSComputedStyleDeclaration::create(element, false, pseudoElement);
> >                                             ^~~~~~~
> >
../../third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.h:52:13:
> > note: passing argument to parameter 'node' here
> >       Node* node,
> > 
> > It seems using Window as a union causes this error somehow. Testing with
other
> > combinations of types in unions complile successfully.
> > 
> > bashi@ do you have any idea about this? Is there any history why the union
> type
> > couldn't be done with Window?
> The problem is that WindowGetComputedStyle::getComputedStyleMap() is defined
in
> the header file. This means that WindowGetComputedStyle.h needs to include
> Node.h and Element.h but I guess it won't work. A right solution would be
moving
> getComputedStyleMap() in .cpp file.
> > 
> > Also, even if we make this work, ServiceWorker class should be able to be
> > accessed from within core, which sounds like an unwanted layer violation?
The
> > interfaces of the other workers are defined under core, but SW has its code
> > under modules. @nhiroki, could you comment on it?
> 
> Unfortunately I don't think we have a solution of this kind of layer violation
> for now :( In the current form of core/modules separation model, it's very
hard
> to support union type which have to be defined in core/ but have to refer a
type
> defined in modules/.

Yeah, currently we don't support union types that are defined in core/ but use
modules/. And it's really hard to support it.

Unfortunately the only viable solution would be to move serviceworker/ to core/.

[nhiroki, 2016-11-01 04:12:18]

On 2016/11/01 02:42:44, haraken wrote:
> On 2016/11/01 00:06:10, bashi1 (slow) wrote:
> > On 2016/10/31 12:12:57, jungkees wrote:
> > > While having tried this with idl-generated
> WindowOrMessagePortOrServiceWorker
> > > class, I encountered some irrelevant errors as follows:
> > > 
> > > In file included from
> > > gen/blink/bindings/core/v8/WindowOrMessagePortOrServiceWorker.cpp:15:
> > >
> ../../third_party/WebKit/Source/core/css/cssom/WindowGetComputedStyle.h:22:45:
> > > error: cannot initialize a parameter of type 'blink::Node *' with an
lvalue
> of
> > > type 'blink::Element *'
> > >         CSSComputedStyleDeclaration::create(element, false,
pseudoElement);
> > >                                             ^~~~~~~
> > >
> ../../third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.h:52:13:
> > > note: passing argument to parameter 'node' here
> > >       Node* node,
> > > 
> > > It seems using Window as a union causes this error somehow. Testing with
> other
> > > combinations of types in unions complile successfully.
> > > 
> > > bashi@ do you have any idea about this? Is there any history why the union
> > type
> > > couldn't be done with Window?
> > The problem is that WindowGetComputedStyle::getComputedStyleMap() is defined
> in
> > the header file. This means that WindowGetComputedStyle.h needs to include
> > Node.h and Element.h but I guess it won't work. A right solution would be
> moving
> > getComputedStyleMap() in .cpp file.
> > > 
> > > Also, even if we make this work, ServiceWorker class should be able to be
> > > accessed from within core, which sounds like an unwanted layer violation?
> The
> > > interfaces of the other workers are defined under core, but SW has its
code
> > > under modules. @nhiroki, could you comment on it?
> > 
> > Unfortunately I don't think we have a solution of this kind of layer
violation
> > for now :( In the current form of core/modules separation model, it's very
> hard
> > to support union type which have to be defined in core/ but have to refer a
> type
> > defined in modules/.
> 
> Yeah, currently we don't support union types that are defined in core/ but use
> modules/. And it's really hard to support it.
> 
> Unfortunately the only viable solution would be to move serviceworker/ to
core/.

I see... Thank you for the follow-up.

Moving all modules/serviceworker/ wouldn't be easy because they rely on
modules/fetch/. Moving a part of service worker components could be a feasible
option, but I'm not sure whether it's justifiable only for this change. As
jungkees@'s comment on the issue[1], just using EventTarget as is and replacing
it with union syntax in a separate issue could be a better workaround.

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=659074#c1

[jungkees, 2016-11-01 05:44:56]

On 2016/11/01 04:12:18, nhiroki wrote:
> On 2016/11/01 02:42:44, haraken wrote:
> > On 2016/11/01 00:06:10, bashi1 (slow) wrote:
> > > On 2016/10/31 12:12:57, jungkees wrote:
> > > > While having tried this with idl-generated
> > WindowOrMessagePortOrServiceWorker
> > > > class, I encountered some irrelevant errors as follows:
> > > > 
> > > > In file included from
> > > > gen/blink/bindings/core/v8/WindowOrMessagePortOrServiceWorker.cpp:15:
> > > >
> >
../../third_party/WebKit/Source/core/css/cssom/WindowGetComputedStyle.h:22:45:
> > > > error: cannot initialize a parameter of type 'blink::Node *' with an
> lvalue
> > of
> > > > type 'blink::Element *'
> > > >         CSSComputedStyleDeclaration::create(element, false,
> pseudoElement);
> > > >                                             ^~~~~~~
> > > >
> >
../../third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.h:52:13:
> > > > note: passing argument to parameter 'node' here
> > > >       Node* node,
> > > > 
> > > > It seems using Window as a union causes this error somehow. Testing with
> > other
> > > > combinations of types in unions complile successfully.
> > > > 
> > > > bashi@ do you have any idea about this? Is there any history why the
union
> > > type
> > > > couldn't be done with Window?
> > > The problem is that WindowGetComputedStyle::getComputedStyleMap() is
defined
> > in
> > > the header file. This means that WindowGetComputedStyle.h needs to include
> > > Node.h and Element.h but I guess it won't work. A right solution would be
> > moving
> > > getComputedStyleMap() in .cpp file.
> > > > 
> > > > Also, even if we make this work, ServiceWorker class should be able to
be
> > > > accessed from within core, which sounds like an unwanted layer
violation?
> > The
> > > > interfaces of the other workers are defined under core, but SW has its
> code
> > > > under modules. @nhiroki, could you comment on it?
> > > 
> > > Unfortunately I don't think we have a solution of this kind of layer
> violation
> > > for now :( In the current form of core/modules separation model, it's very
> > hard
> > > to support union type which have to be defined in core/ but have to refer
a
> > type
> > > defined in modules/.
> > 
> > Yeah, currently we don't support union types that are defined in core/ but
use
> > modules/. And it's really hard to support it.
> > 
> > Unfortunately the only viable solution would be to move serviceworker/ to
> core/.
> 
> I see... Thank you for the follow-up.
> 
> Moving all modules/serviceworker/ wouldn't be easy because they rely on
> modules/fetch/. Moving a part of service worker components could be a feasible
> option, but I'm not sure whether it's justifiable only for this change. As
> jungkees@'s comment on the issue[1], just using EventTarget as is and
replacing
> it with union syntax in a separate issue could be a better workaround.
> 
> [1] https://bugs.chromium.org/p/chromium/issues/detail?id=659074#c1

Thanks for comments. I'll do it that way.

[haraken, 2016-11-01 06:25:15]

On 2016/11/01 05:44:56, jungkees wrote:
> On 2016/11/01 04:12:18, nhiroki wrote:
> > On 2016/11/01 02:42:44, haraken wrote:
> > > On 2016/11/01 00:06:10, bashi1 (slow) wrote:
> > > > On 2016/10/31 12:12:57, jungkees wrote:
> > > > > While having tried this with idl-generated
> > > WindowOrMessagePortOrServiceWorker
> > > > > class, I encountered some irrelevant errors as follows:
> > > > > 
> > > > > In file included from
> > > > > gen/blink/bindings/core/v8/WindowOrMessagePortOrServiceWorker.cpp:15:
> > > > >
> > >
> ../../third_party/WebKit/Source/core/css/cssom/WindowGetComputedStyle.h:22:45:
> > > > > error: cannot initialize a parameter of type 'blink::Node *' with an
> > lvalue
> > > of
> > > > > type 'blink::Element *'
> > > > >         CSSComputedStyleDeclaration::create(element, false,
> > pseudoElement);
> > > > >                                             ^~~~~~~
> > > > >
> > >
> ../../third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.h:52:13:
> > > > > note: passing argument to parameter 'node' here
> > > > >       Node* node,
> > > > > 
> > > > > It seems using Window as a union causes this error somehow. Testing
with
> > > other
> > > > > combinations of types in unions complile successfully.
> > > > > 
> > > > > bashi@ do you have any idea about this? Is there any history why the
> union
> > > > type
> > > > > couldn't be done with Window?
> > > > The problem is that WindowGetComputedStyle::getComputedStyleMap() is
> defined
> > > in
> > > > the header file. This means that WindowGetComputedStyle.h needs to
include
> > > > Node.h and Element.h but I guess it won't work. A right solution would
be
> > > moving
> > > > getComputedStyleMap() in .cpp file.
> > > > > 
> > > > > Also, even if we make this work, ServiceWorker class should be able to
> be
> > > > > accessed from within core, which sounds like an unwanted layer
> violation?
> > > The
> > > > > interfaces of the other workers are defined under core, but SW has its
> > code
> > > > > under modules. @nhiroki, could you comment on it?
> > > > 
> > > > Unfortunately I don't think we have a solution of this kind of layer
> > violation
> > > > for now :( In the current form of core/modules separation model, it's
very
> > > hard
> > > > to support union type which have to be defined in core/ but have to
refer
> a
> > > type
> > > > defined in modules/.
> > > 
> > > Yeah, currently we don't support union types that are defined in core/ but
> use
> > > modules/. And it's really hard to support it.
> > > 
> > > Unfortunately the only viable solution would be to move serviceworker/ to
> > core/.
> > 
> > I see... Thank you for the follow-up.
> > 
> > Moving all modules/serviceworker/ wouldn't be easy because they rely on
> > modules/fetch/. Moving a part of service worker components could be a
feasible
> > option, but I'm not sure whether it's justifiable only for this change. As
> > jungkees@'s comment on the issue[1], just using EventTarget as is and
> replacing
> > it with union syntax in a separate issue could be a better workaround.
> > 
> > [1] https://bugs.chromium.org/p/chromium/issues/detail?id=659074#c1
> 
> Thanks for comments. I'll do it that way.

Yeah, sorry about the inconvenience. This is clearly a bug of the core-module
architecture, but I cannot come up with an easy way to solve it :/

[jungkees, 2016-11-01 07:04:19]

On 2016/11/01 06:25:15, haraken wrote:
> On 2016/11/01 05:44:56, jungkees wrote:
> > On 2016/11/01 04:12:18, nhiroki wrote:
> > > On 2016/11/01 02:42:44, haraken wrote:
> > > > On 2016/11/01 00:06:10, bashi1 (slow) wrote:
> > > > > On 2016/10/31 12:12:57, jungkees wrote:
> > > > > > While having tried this with idl-generated
> > > > WindowOrMessagePortOrServiceWorker
> > > > > > class, I encountered some irrelevant errors as follows:
> > > > > > 
> > > > > > In file included from
> > > > > >
gen/blink/bindings/core/v8/WindowOrMessagePortOrServiceWorker.cpp:15:
> > > > > >
> > > >
> >
../../third_party/WebKit/Source/core/css/cssom/WindowGetComputedStyle.h:22:45:
> > > > > > error: cannot initialize a parameter of type 'blink::Node *' with an
> > > lvalue
> > > > of
> > > > > > type 'blink::Element *'
> > > > > >         CSSComputedStyleDeclaration::create(element, false,
> > > pseudoElement);
> > > > > >                                             ^~~~~~~
> > > > > >
> > > >
> >
../../third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.h:52:13:
> > > > > > note: passing argument to parameter 'node' here
> > > > > >       Node* node,
> > > > > > 
> > > > > > It seems using Window as a union causes this error somehow. Testing
> with
> > > > other
> > > > > > combinations of types in unions complile successfully.
> > > > > > 
> > > > > > bashi@ do you have any idea about this? Is there any history why the
> > union
> > > > > type
> > > > > > couldn't be done with Window?
> > > > > The problem is that WindowGetComputedStyle::getComputedStyleMap() is
> > defined
> > > > in
> > > > > the header file. This means that WindowGetComputedStyle.h needs to
> include
> > > > > Node.h and Element.h but I guess it won't work. A right solution would
> be
> > > > moving
> > > > > getComputedStyleMap() in .cpp file.
> > > > > > 
> > > > > > Also, even if we make this work, ServiceWorker class should be able
to
> > be
> > > > > > accessed from within core, which sounds like an unwanted layer
> > violation?
> > > > The
> > > > > > interfaces of the other workers are defined under core, but SW has
its
> > > code
> > > > > > under modules. @nhiroki, could you comment on it?
> > > > > 
> > > > > Unfortunately I don't think we have a solution of this kind of layer
> > > violation
> > > > > for now :( In the current form of core/modules separation model, it's
> very
> > > > hard
> > > > > to support union type which have to be defined in core/ but have to
> refer
> > a
> > > > type
> > > > > defined in modules/.
> > > > 
> > > > Yeah, currently we don't support union types that are defined in core/
but
> > use
> > > > modules/. And it's really hard to support it.
> > > > 
> > > > Unfortunately the only viable solution would be to move serviceworker/
to
> > > core/.
> > > 
> > > I see... Thank you for the follow-up.
> > > 
> > > Moving all modules/serviceworker/ wouldn't be easy because they rely on
> > > modules/fetch/. Moving a part of service worker components could be a
> feasible
> > > option, but I'm not sure whether it's justifiable only for this change. As
> > > jungkees@'s comment on the issue[1], just using EventTarget as is and
> > replacing
> > > it with union syntax in a separate issue could be a better workaround.
> > > 
> > > [1] https://bugs.chromium.org/p/chromium/issues/detail?id=659074#c1
> > 
> > Thanks for comments. I'll do it that way.
> 
> Yeah, sorry about the inconvenience. This is clearly a bug of the core-module
> architecture, but I cannot come up with an easy way to solve it :/

Understood. Thanks for the confirmation.

[jungkees, 2016-11-04 11:44:37]

Description was changed from

==========
(WIP) Extend Message Event

HTML extends MessageEvent to allow ServiceWorker as a type of source attribute.
Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent. This patch incorporates these spec changes.

BUG=659074

R=nhiroki@chromium.org, jinho.bang@samsung.com
==========

to

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent.

This is a WIP. I still need to work on layout tests for the changes. Do
NOT spend time on reviewing yet.

BUG=659074
==========

[foolip, 2016-11-09 22:09:12]

For the srcObject case, the fix was to move just that attribute to its own
module.

[jungkees, 2016-11-10 01:09:49]

On 2016/11/09 22:09:12, foolip wrote:
> For the srcObject case, the fix was to move just that attribute to its own
> module.

foolip@, I'm not sure I understood. Could you get me some more info about your
point?

[jungkees, 2016-11-10 06:19:21]

Description was changed from

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent.

This is a WIP. I still need to work on layout tests for the changes. Do
NOT spend time on reviewing yet.

BUG=659074
==========

to

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent when the spec changes are landed in HTML. Landing this CL
should wait until both HTML and SW incorporate the changes.

BUG=659074

Related spec issue: https://github.com/w3c/ServiceWorker/issues/989
Related spec change: https://github.com/whatwg/html/pull/1944 (SW changes are
WIP.)
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186
==========

[jungkees, 2016-11-10 06:19:41]

Description was changed from

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent when the spec changes are landed in HTML. Landing this CL
should wait until both HTML and SW incorporate the changes.

BUG=659074

Related spec issue: https://github.com/w3c/ServiceWorker/issues/989
Related spec change: https://github.com/whatwg/html/pull/1944 (SW changes are
WIP.)
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186
==========

to

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent when the spec changes are landed in HTML. Landing this CL
should wait until both HTML and SW incorporate the changes.

BUG=659074


Related spec issue: https://github.com/w3c/ServiceWorker/issues/989
Related spec change: https://github.com/whatwg/html/pull/1944 (SW changes are
WIP.)
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186
==========

[jungkees, 2016-11-10 06:20:10]

Description was changed from

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent when the spec changes are landed in HTML. Landing this CL
should wait until both HTML and SW incorporate the changes.

BUG=659074


Related spec issue: https://github.com/w3c/ServiceWorker/issues/989
Related spec change: https://github.com/whatwg/html/pull/1944 (SW changes are
WIP.)
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186
==========

to

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent when the spec changes are landed in HTML. Landing this CL
should wait until both HTML and SW incorporate the changes.

Related spec issue: https://github.com/w3c/ServiceWorker/issues/989
Related spec change: https://github.com/whatwg/html/pull/1944 (SW changes are
WIP.)
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186

BUG=659074
==========

[jungkees, 2016-11-10 06:27:35]

nhiroki@, zino@, foolip@, I uploaded a snapshot for review. PTAL. (I encountered
a 403 error while uploading the files as it sometimes occurred from my work
environment, but it seems the files are uploaded as usual. Let me know if you
have any difficulties in reviewing them.)

[foolip, 2016-11-10 09:33:12]

On 2016/11/10 01:09:49, jungkees wrote:
> On 2016/11/09 22:09:12, foolip wrote:
> > For the srcObject case, the fix was to move just that attribute to its own
> > module.
> 
> foolip@, I'm not sure I understood. Could you get me some more info about your
> point?

In HTMLMediaElementSrcObject.idl, there's a partial interface HTMLMediaElement,
and the only reason it's not in core/ is that the MediaStream type is involved.
It might be possible to do something similar here, but I don't know. It'd sure
be nice to not have to write IDL that's different from the spec for cases like
this.

[zino, 2016-11-10 10:54:33]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-10 10:54:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jungkees, 2016-11-10 11:17:59]

On 2016/11/10 09:33:12, foolip wrote:
> On 2016/11/10 01:09:49, jungkees wrote:
> > On 2016/11/09 22:09:12, foolip wrote:
> > > For the srcObject case, the fix was to move just that attribute to its own
> > > module.
> > 
> > foolip@, I'm not sure I understood. Could you get me some more info about
your
> > point?
> 
> In HTMLMediaElementSrcObject.idl, there's a partial interface
HTMLMediaElement,
> and the only reason it's not in core/ is that the MediaStream type is
involved.
> It might be possible to do something similar here, but I don't know. It'd sure
> be nice to not have to write IDL that's different from the spec for cases like
> this.

In the previous attempt with defining a union (even with WindowOrMessagePort,
i.e, without ServiceWorker included), I'd had some series of errors after
https://codereview.chromium.org/2466513002/#msg7 (including macro separation
issues, etc.) Before trying to fix them as part of this CL, I'd like to have
more comments on the direction.

[commit-bot: I haz the power, 2016-11-10 12:32:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-10 12:32:10]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[zino, 2016-11-10 14:31:13]

On 2016/11/10 11:17:59, jungkees wrote:
> On 2016/11/10 09:33:12, foolip wrote:
> > On 2016/11/10 01:09:49, jungkees wrote:
> > > On 2016/11/09 22:09:12, foolip wrote:
> > > > For the srcObject case, the fix was to move just that attribute to its
own
> > > > module.
> > > 
> > > foolip@, I'm not sure I understood. Could you get me some more info about
> your
> > > point?
> > 
> > In HTMLMediaElementSrcObject.idl, there's a partial interface
> HTMLMediaElement,
> > and the only reason it's not in core/ is that the MediaStream type is
> involved.
> > It might be possible to do something similar here, but I don't know. It'd
sure
> > be nice to not have to write IDL that's different from the spec for cases
like
> > this.
> 
> In the previous attempt with defining a union (even with WindowOrMessagePort,
> i.e, without ServiceWorker included), I'd had some series of errors after
> https://codereview.chromium.org/2466513002/#msg7 (including macro separation
> issues, etc.) Before trying to fix them as part of this CL, I'd like to have
> more comments on the direction.

I'm not sure but Philip's idea seems that we can use partial interface instead
of defining the union type in MessageEvent.idl directly.
The original problem was that it's impossible to refer to ServiceWorker class in
core directory due to layer violation.
So, if we use partial interface and define the union type in module directory,
then it will not layer violation.

[zino, 2016-11-10 14:35:38]

Hmm, I'm not expert in this part but it looks very good idea.
Other reviewers, what do you think?

[jungkees, 2016-11-11 08:54:51]

Description was changed from

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent when the spec changes are landed in HTML. Landing this CL
should wait until both HTML and SW incorporate the changes.

Related spec issue: https://github.com/w3c/ServiceWorker/issues/989
Related spec change: https://github.com/whatwg/html/pull/1944 (SW changes are
WIP.)
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186

BUG=659074
==========

to

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent when the spec changes are landed in HTML. Landing this CL
should wait until both HTML and SW incorporate the changes.

Related spec issue: https://github.com/w3c/ServiceWorker/issues/989
Related spec change:
 - https://github.com/whatwg/html/pull/1944
 -
https://github.com/w3c/ServiceWorker/commit/6c68bd87bdfd153186a5b9904e31676c5...
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186

BUG=659074
==========

[jungkees, 2016-11-11 08:58:23]

Description was changed from

==========
This extends MessageEvent to allow ServiceWorker as a type of source
attribute according to the changes in HTML spec:
https://github.com/whatwg/html/pull/1944

Service worker will replace ServiceWorkerMessageEvent with this extended
MessageEvent when the spec changes are landed in HTML. Landing this CL
should wait until both HTML and SW incorporate the changes.

Related spec issue: https://github.com/w3c/ServiceWorker/issues/989
Related spec change:
 - https://github.com/whatwg/html/pull/1944
 -
https://github.com/w3c/ServiceWorker/commit/6c68bd87bdfd153186a5b9904e31676c5...
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186

BUG=659074
==========

to

==========
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/6c68bd87bdfd153186a5b9904e31676c5...
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186

BUG=659074
==========

[nhiroki, 2016-11-11 11:34:36]

Sorry, I couldn't have time to review this CL today. I'll take a look early next
week.

On 2016/11/10 14:31:13, zino wrote:
> On 2016/11/10 11:17:59, jungkees wrote:
> > On 2016/11/10 09:33:12, foolip wrote:
> > > On 2016/11/10 01:09:49, jungkees wrote:
> > > > On 2016/11/09 22:09:12, foolip wrote:
> > > > > For the srcObject case, the fix was to move just that attribute to its
> own
> > > > > module.
> > > > 
> > > > foolip@, I'm not sure I understood. Could you get me some more info
about
> > your
> > > > point?
> > > 
> > > In HTMLMediaElementSrcObject.idl, there's a partial interface
> > HTMLMediaElement,
> > > and the only reason it's not in core/ is that the MediaStream type is
> > involved.
> > > It might be possible to do something similar here, but I don't know. It'd
> sure
> > > be nice to not have to write IDL that's different from the spec for cases
> like
> > > this.
> > 
> > In the previous attempt with defining a union (even with
WindowOrMessagePort,
> > i.e, without ServiceWorker included), I'd had some series of errors after
> > https://codereview.chromium.org/2466513002/#msg7 (including macro separation
> > issues, etc.) Before trying to fix them as part of this CL, I'd like to have
> > more comments on the direction.
> 
> I'm not sure but Philip's idea seems that we can use partial interface instead
> of defining the union type in MessageEvent.idl directly.
> The original problem was that it's impossible to refer to ServiceWorker class
in
> core directory due to layer violation.
> So, if we use partial interface and define the union type in module directory,
> then it will not layer violation.

I'd prefer the current approach because (1) it'd be easier to add a new source
type when we want it, and (2) implementing the whole MessageEvent.source under
modules/serviceworker seems strange.

[nhiroki, 2016-11-11 11:40:31]

On 2016/11/10 06:27:35, jungkees wrote:
> nhiroki@, zino@, foolip@, I uploaded a snapshot for review. PTAL. (I
encountered
> a 403 error while uploading the files as it sometimes occurred from my work
> environment, but it seems the files are uploaded as usual. Let me know if you
> have any difficulties in reviewing them.)

Maybe you failed to upload
third_party/WebKit/Source/modules/modules_idl_files.gni. I cannot see it :)

[jungkees, 2016-11-11 14:37:21]

On 2016/11/11 11:34:36, nhiroki wrote:
> Sorry, I couldn't have time to review this CL today. I'll take a look early
next
> week.
>

Thanks!

> 
> I'd prefer the current approach because (1) it'd be easier to add a new source
> type when we want it, and (2) implementing the whole MessageEvent.source under
> modules/serviceworker seems strange.

Right. As I discussed with zino@, partial dictionary seems to be not supported
yet. So, moving the whole dictionary MessageEventInit to module would not be
plausible either I guess.

[jungkees, 2016-11-11 14:43:52]

On 2016/11/11 11:40:31, nhiroki wrote:
> On 2016/11/10 06:27:35, jungkees wrote:
> > nhiroki@, zino@, foolip@, I uploaded a snapshot for review. PTAL. (I
> encountered
> > a 403 error while uploading the files as it sometimes occurred from my work
> > environment, but it seems the files are uploaded as usual. Let me know if
you
> > have any difficulties in reviewing them.)
> 
> Maybe you failed to upload
> third_party/WebKit/Source/modules/modules_idl_files.gni. I cannot see it :)

Sorry about the inconvenience. The side-by-side diff column doesn't show the
patch, but unified diffs column shows the patch. Getting home, I succeeded with
"git cl patch *this_CL*".

[jungkees, 2016-11-14 04:02:33]

The CQ bit was checked by jungkee.song@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 04:02:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jungkees, 2016-11-14 04:13:30]

The CQ bit was unchecked by jungkee.song@samsung.com

[nhiroki, 2016-11-14 05:48:55]

Looks good overall.

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html
(right):

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html:36:
'source should be ServiceWorker.');
Can you check assert_equals(e.ports.length, 0); here?

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/serviceworker-message-event-historical.html
(right):

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/serviceworker-message-event-historical.html:34:
resolve();
Can we merge this test into postmessage-to-client.html?

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainer.cpp
(right):

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainer.cpp:515:
String(), source, String()));
nit:

Adding arg names would be a bit more readable.

  String() /* lastEventId */, source, String() /* suborigin */));

[jungkees, 2016-11-14 14:17:11]

I uploaded a new snapshot having addressed the comments. PTAL.

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html
(right):

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html:36:
'source should be ServiceWorker.');
On 2016/11/14 05:48:55, nhiroki wrote:
> Can you check assert_equals(e.ports.length, 0); here?

Done.

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/serviceworker-message-event-historical.html
(right):

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/serviceworker-message-event-historical.html:34:
resolve();
On 2016/11/14 05:48:55, nhiroki wrote:
> Can we merge this test into postmessage-to-client.html?

I merged the relevant asserts into postmessage-to-client.html and removed this
file.

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainer.cpp
(right):

https://codereview.chromium.org/2466513002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainer.cpp:515:
String(), source, String()));
On 2016/11/14 05:48:55, nhiroki wrote:
> nit:
> 
> Adding arg names would be a bit more readable.
> 
>   String() /* lastEventId */, source, String() /* suborigin */));

Yes. Addressed.

[jungkees, 2016-11-14 14:17:49]

The CQ bit was checked by jungkee.song@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 14:18:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-14 15:53:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-14 15:53:29]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[nhiroki, 2016-11-15 03:42:15]

lgtm

https://codereview.chromium.org/2466513002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html
(right):

https://codereview.chromium.org/2466513002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html:29:
'ServiceWorkerMessageEvent should not be defined.');
This assertion wouldn't be necessary because it's covered by global-interface
tests.

[haraken, 2016-11-15 05:00:42]

Implementation-wise LGTM.

You'll need to get an approval of an API owner.

[jungkees, 2016-11-15 05:08:00]

On 2016/11/15 05:00:42, haraken wrote:
> Implementation-wise LGTM.
> 
> You'll need to get an approval of an API owner.

Thanks for the review. I'll address nhiroki@'s comment and remove
ServiceWorkerMessageEvent interface listings from the following files (I can do
later today or early tomorrow):

src/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
src/third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt

foolip@, could you advise me with what you expect to land this CL (any intent or
required procedure?)

[jungkees, 2016-11-15 16:06:49]

I uploaded a new snapshot having addressed nhiroki@'s comment and removed
ServiceWorkerMessageEvent interfaces from global-interface-listing-expected
files for linux/win.

foolip@, PTAL.

https://codereview.chromium.org/2466513002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html
(right):

https://codereview.chromium.org/2466513002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-to-client.html:29:
'ServiceWorkerMessageEvent should not be defined.');
On 2016/11/15 03:42:15, nhiroki wrote:
> This assertion wouldn't be necessary because it's covered by global-interface
> tests.

Removed this assertion.

[jungkees, 2016-11-15 16:07:40]

The CQ bit was checked by jungkee.song@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-15 16:07:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[foolip, 2016-11-15 16:16:43]

On 2016/11/15 05:08:00, jungkees wrote:
> On 2016/11/15 05:00:42, haraken wrote:
> > Implementation-wise LGTM.
> > 
> > You'll need to get an approval of an API owner.
> 
> Thanks for the review. I'll address nhiroki@'s comment and remove
> ServiceWorkerMessageEvent interface listings from the following files (I can
do
> later today or early tomorrow):
> 
>
src/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
>
src/third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt
> 
> foolip@, could you advise me with what you expect to land this CL (any intent
or
> required procedure?)

This does delete an interface that had a constructor, so an Intent to Deprecate
and Remove seems appropriate. Given
https://github.com/w3c/ServiceWorker/issues/989#issuecomment-255337272 it seems
very safe and a deprecation period wouldn't help, direct removal seems OK.

[commit-bot: I haz the power, 2016-11-15 17:18:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-15 17:18:48]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jungkees, 2016-11-16 05:10:58]

On 2016/11/15 16:16:43, foolip wrote:
> On 2016/11/15 05:08:00, jungkees wrote:
> > On 2016/11/15 05:00:42, haraken wrote:
> > > Implementation-wise LGTM.
> > > 
> > > You'll need to get an approval of an API owner.
> > 
> > Thanks for the review. I'll address nhiroki@'s comment and remove
> > ServiceWorkerMessageEvent interface listings from the following files (I can
> do
> > later today or early tomorrow):
> > 
> >
>
src/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
> >
>
src/third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt
> > 
> > foolip@, could you advise me with what you expect to land this CL (any
intent
> or
> > required procedure?)
> 
> This does delete an interface that had a constructor, so an Intent to
Deprecate
> and Remove seems appropriate. Given
> https://github.com/w3c/ServiceWorker/issues/989#issuecomment-255337272 it
seems
> very safe and a deprecation period wouldn't help, direct removal seems OK.

Posted Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Xp9hmKyuOrI

[jungkees, 2016-11-16 05:11:36]

Description was changed from

==========
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/6c68bd87bdfd153186a5b9904e31676c5...
Related WPT: https://github.com/w3c/web-platform-tests/pull/4186

BUG=659074
==========

to

==========
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/6c68bd87bdfd153186a5b9904e31676c5...
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
==========

[jungkees, 2016-12-08 06:06:11]

On 2016/11/16 05:10:58, jungkees wrote:
> On 2016/11/15 16:16:43, foolip wrote:
> > On 2016/11/15 05:08:00, jungkees wrote:
> > > On 2016/11/15 05:00:42, haraken wrote:
> > > > Implementation-wise LGTM.
> > > > 
> > > > You'll need to get an approval of an API owner.
> > > 
> > > Thanks for the review. I'll address nhiroki@'s comment and remove
> > > ServiceWorkerMessageEvent interface listings from the following files (I
can
> > do
> > > later today or early tomorrow):
> > > 
> > >
> >
>
src/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
> > >
> >
>
src/third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt
> > > 
> > > foolip@, could you advise me with what you expect to land this CL (any
> intent
> > or
> > > required procedure?)
> > 
> > This does delete an interface that had a constructor, so an Intent to
> Deprecate
> > and Remove seems appropriate. Given
> > https://github.com/w3c/ServiceWorker/issues/989#issuecomment-255337272 it
> seems
> > very safe and a deprecation period wouldn't help, direct removal seems OK.
> 
> Posted Intent to Deprecate and Remove:
> https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Xp9hmKyuOrI

Now I've had three l-g-t-m's for
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Xp9hmKyuOrI Can
I proceed with commit attempt now? Or should I get another API owner's l-g-t-m
for this CL?

[foolip, 2016-12-08 11:48:49]

On 2016/12/08 06:06:11, jungkees wrote:
> On 2016/11/16 05:10:58, jungkees wrote:
> > On 2016/11/15 16:16:43, foolip wrote:
> > > On 2016/11/15 05:08:00, jungkees wrote:
> > > > On 2016/11/15 05:00:42, haraken wrote:
> > > > > Implementation-wise LGTM.
> > > > > 
> > > > > You'll need to get an approval of an API owner.
> > > > 
> > > > Thanks for the review. I'll address nhiroki@'s comment and remove
> > > > ServiceWorkerMessageEvent interface listings from the following files (I
> can
> > > do
> > > > later today or early tomorrow):
> > > > 
> > > >
> > >
> >
>
src/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
> > > >
> > >
> >
>
src/third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt
> > > > 
> > > > foolip@, could you advise me with what you expect to land this CL (any
> > intent
> > > or
> > > > required procedure?)
> > > 
> > > This does delete an interface that had a constructor, so an Intent to
> > Deprecate
> > > and Remove seems appropriate. Given
> > > https://github.com/w3c/ServiceWorker/issues/989#issuecomment-255337272 it
> > seems
> > > very safe and a deprecation period wouldn't help, direct removal seems OK.
> > 
> > Posted Intent to Deprecate and Remove:
> > https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Xp9hmKyuOrI
> 
> Now I've had three l-g-t-m's for
> https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Xp9hmKyuOrI
Can
> I proceed with commit attempt now? Or should I get another API owner's l-g-t-m
> for this CL?

In general API owners don't need to LGTM the actual CLs, but
virtual/stable/webexposed/global-interface-listing-expected.txt does, so LGTM
for that and leaving everything else to other reviewers.

[jungkees, 2016-12-09 00:17:01]

The CQ bit was checked by jungkee.song@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 00:17:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 00:20:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 00:20:05]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[jungkees, 2016-12-09 14:10:08]

The CQ bit was checked by jungkee.song@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 14:10:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 15:59:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 15:59:51]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jungkees, 2016-12-09 19:36:04]

I uploaded a new snapshot having rebased it. PTAL.

[jungkees, 2016-12-09 19:36:22]

The CQ bit was checked by jungkee.song@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 19:37:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[zino, 2016-12-09 19:37:18]

On 2016/12/09 19:36:04, jungkees wrote:
> I uploaded a new snapshot having rebased it. PTAL.

Oh, you looks hard worker. ;)

[commit-bot: I haz the power, 2016-12-09 22:27:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 22:27:59]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jungkees, 2016-12-12 02:24:45]

> Oh, you looks hard worker. ;)

zino@, you always stay up until that hour? ;)

I found a place I should additionally fix:

In
imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions-expected.txt

"FAIL [[OwnPropertyKeys]] should return all properties from cross-origin objects
assert_array_equals: Object.getOwnPropertyNames() gives the right answer for
cross-origin Window lengths differ, expected 863 got 13"

s/863/862/ is needed to address the deleted ServiceWorkerMessageEvent interface.

I'll update this later today.

[jungkees, 2016-12-12 14:41:44]

The CQ bit was checked by jungkee.song@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-12 14:41:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-12 16:47:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-12 16:47:40]

Dry run: This issue passed the CQ dry run.

[jungkees, 2016-12-13 01:14:47]

On 2016/12/12 02:24:45, jungkees wrote:
> > Oh, you looks hard worker. ;)
> 
> zino@, you always stay up until that hour? ;)
> 
> I found a place I should additionally fix:
> 
> In
>
imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions-expected.txt
> 
> "FAIL [[OwnPropertyKeys]] should return all properties from cross-origin
objects
> assert_array_equals: Object.getOwnPropertyNames() gives the right answer for
> cross-origin Window lengths differ, expected 863 got 13"
> 
> s/863/862/ is needed to address the deleted ServiceWorkerMessageEvent
interface.
> 
> I'll update this later today.

I uploaded a new snapshot having addressed the
cross-origin-objects-exceptions.html issue. PTAL.

[nhiroki, 2016-12-13 02:22:22]

On 2016/12/13 01:14:47, jungkees wrote:
> On 2016/12/12 02:24:45, jungkees wrote:
> > > Oh, you looks hard worker. ;)
> > 
> > zino@, you always stay up until that hour? ;)
> > 
> > I found a place I should additionally fix:
> > 
> > In
> >
>
imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions-expected.txt
> > 
> > "FAIL [[OwnPropertyKeys]] should return all properties from cross-origin
> objects
> > assert_array_equals: Object.getOwnPropertyNames() gives the right answer for
> > cross-origin Window lengths differ, expected 863 got 13"
> > 
> > s/863/862/ is needed to address the deleted ServiceWorkerMessageEvent
> interface.
> > 
> > I'll update this later today.
> 
> I uploaded a new snapshot having addressed the
> cross-origin-objects-exceptions.html issue. PTAL.

LGTM.

Regarding the CL description, "Extend MessageEvent" would be somewhat unclear.
Can you make it more informative, for example, "Deprecate
ServiceWorkerMessageEvent in favor of MessageEvent", "Extend MessageEvent to
allow ServiceWorker as a type of source
attribute"?

[nhiroki, 2016-12-13 02:26:15]

On 2016/12/13 02:22:22, nhiroki wrote:
> On 2016/12/13 01:14:47, jungkees wrote:
> > On 2016/12/12 02:24:45, jungkees wrote:
> > > > Oh, you looks hard worker. ;)
> > > 
> > > zino@, you always stay up until that hour? ;)
> > > 
> > > I found a place I should additionally fix:
> > > 
> > > In
> > >
> >
>
imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions-expected.txt
> > > 
> > > "FAIL [[OwnPropertyKeys]] should return all properties from cross-origin
> > objects
> > > assert_array_equals: Object.getOwnPropertyNames() gives the right answer
for
> > > cross-origin Window lengths differ, expected 863 got 13"
> > > 
> > > s/863/862/ is needed to address the deleted ServiceWorkerMessageEvent
> > interface.
> > > 
> > > I'll update this later today.
> > 
> > I uploaded a new snapshot having addressed the
> > cross-origin-objects-exceptions.html issue. PTAL.
> 
> LGTM.
> 
> Regarding the CL description, "Extend MessageEvent" would be somewhat unclear.
> Can you make it more informative, for example, "Deprecate
> ServiceWorkerMessageEvent in favor of MessageEvent", "Extend MessageEvent to
> allow ServiceWorker as a type of source
> attribute"?

^^^ s/CL description/CL subject

In addition, CL description should include CL subject at the first line because
a commit log doesn't explicitly include the CL subject.

[jungkees, 2016-12-13 02:34:22]

Description was changed from

==========
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/6c68bd87bdfd153186a5b9904e31676c5...
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
==========

to

==========
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/6c68bd87bdfd153186a5b9904e31676c5...
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
==========

[jungkees, 2016-12-13 02:35:54]

On 2016/12/13 02:26:15, nhiroki wrote:
> On 2016/12/13 02:22:22, nhiroki wrote:
> > On 2016/12/13 01:14:47, jungkees wrote:
> > > On 2016/12/12 02:24:45, jungkees wrote:
> > > > > Oh, you looks hard worker. ;)
> > > > 
> > > > zino@, you always stay up until that hour? ;)
> > > > 
> > > > I found a place I should additionally fix:
> > > > 
> > > > In
> > > >
> > >
> >
>
imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions-expected.txt
> > > > 
> > > > "FAIL [[OwnPropertyKeys]] should return all properties from cross-origin
> > > objects
> > > > assert_array_equals: Object.getOwnPropertyNames() gives the right answer
> for
> > > > cross-origin Window lengths differ, expected 863 got 13"
> > > > 
> > > > s/863/862/ is needed to address the deleted ServiceWorkerMessageEvent
> > > interface.
> > > > 
> > > > I'll update this later today.
> > > 
> > > I uploaded a new snapshot having addressed the
> > > cross-origin-objects-exceptions.html issue. PTAL.
> > 
> > LGTM.
> > 
> > Regarding the CL description, "Extend MessageEvent" would be somewhat
unclear.
> > Can you make it more informative, for example, "Deprecate
> > ServiceWorkerMessageEvent in favor of MessageEvent", "Extend MessageEvent to
> > allow ServiceWorker as a type of source
> > attribute"?
> 
> ^^^ s/CL description/CL subject
> 
> In addition, CL description should include CL subject at the first line
because
> a commit log doesn't explicitly include the CL subject.

nhiroki@, thanks for the review! Done with the comments.

[jungkees, 2016-12-13 02:36:35]

The CQ bit was checked by jungkee.song@samsung.com

[jungkees, 2016-12-13 02:36:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, foolip@chromium.org
Link to the patchset: https://codereview.chromium.org/2466513002/#ps120001
(title: "Update cross-origin-objects-exceptions-expected.txt")

[commit-bot: I haz the power, 2016-12-13 02:37:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 02:47:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 02:47:49]

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_presub...)

[jungkees, 2016-12-13 05:47:09]

jungkee.song@samsung.com changed reviewers:
+ boliu@chromium.org, timvolodine@chromium.org

[jungkees, 2016-12-13 05:49:15]

On 2016/12/13 02:47:49, commit-bot: I haz the power wrote:
> 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_presub...)

timvolodine@, boliu@, sorry to have missed to request a review for
android_webview/tools/system_webview_shell/test/data/webexposed/global-interface-listing-expected.txt

PTAL.

[boliu, 2016-12-13 16:05:01]

On 2016/12/13 05:49:15, jungkees wrote:
> On 2016/12/13 02:47:49, commit-bot: I haz the power wrote:
> > 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_presub...)
> 
> timvolodine@, boliu@, sorry to have missed to request a review for
>
android_webview/tools/system_webview_shell/test/data/webexposed/global-interface-listing-expected.txt
> 
> PTAL.

rs lgtm

[jungkees, 2016-12-14 01:21:12]

On 2016/12/13 16:05:01, boliu wrote:
> On 2016/12/13 05:49:15, jungkees wrote:
> > On 2016/12/13 02:47:49, commit-bot: I haz the power wrote:
> > > 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_presub...)
> > 
> > timvolodine@, boliu@, sorry to have missed to request a review for
> >
>
android_webview/tools/system_webview_shell/test/data/webexposed/global-interface-listing-expected.txt
> > 
> > PTAL.
> 
> rs lgtm

Thanks!

[jungkees, 2016-12-14 01:21:23]

The CQ bit was checked by jungkee.song@samsung.com

[commit-bot: I haz the power, 2016-12-14 01:22:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 03:11:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-14 03:11:57]

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_...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jungkees, 2016-12-14 16:09:28]

On 2016/12/14 03:11:57, commit-bot: I haz the power wrote:
> 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_...)
>   win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

https://crrev.com/f63d67df608751f83eaa15a67df069a38714f171 removed
SVGCursorElement and required a change to
third_party/WebKit/LayoutTests/imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions-expected.txt

Rebased and updated cross-origin-objects-exceptions-expected.txt as such.

[jungkees, 2016-12-14 16:09:44]

The CQ bit was checked by jungkee.song@samsung.com

[jungkees, 2016-12-14 16:09:44]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org,
nhiroki@chromium.org, foolip@chromium.org, haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2466513002/#ps140001
(title: "Rebase and update cross-origin-objects-exceptions-expected.txt")

[commit-bot: I haz the power, 2016-12-14 16:09:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 18:38:22]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1481731784296050,
"parent_rev": "1141d05c6719e5a78b33782d2fe62e523ccd92cb", "commit_rev":
"e144ba694c5061bdd9e4debfe6102045804d5ca8"}

[commit-bot: I haz the power, 2016-12-14 18:39:03]

Description was changed from

==========
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/6c68bd87bdfd153186a5b9904e31676c5...
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
==========

to

==========
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/6c68bd87bdfd153186a5b9904e31676c5...
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

Review-Url: https://codereview.chromium.org/2466513002
==========

[commit-bot: I haz the power, 2016-12-14 18:39:05]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-12-14 18:43:20]

Description was changed from

==========
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/6c68bd87bdfd153186a5b9904e31676c5...
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

Review-Url: https://codereview.chromium.org/2466513002
==========

to

==========
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/6c68bd87bdfd153186a5b9904e31676c5...
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}
==========

[commit-bot: I haz the power, 2016-12-14 18:43:22]

Patchset 8 (id:??) landed as
https://crrev.com/faa6203e513b171743c9d9fd7121501e681186af
Cr-Commit-Position: refs/heads/master@{#438545}