Freeze global prototype chain per WebIDL

Freeze global prototype chain per WebIDL

There are three places where WebIDL freezes the global object prototype chain:
- Named properties objects have an immutable prototype
- Prototypes of interfaces declared with [Global] or [PrimaryGlobal], and
  classes which are inherited from by those. For the latter, this patch
  introduces an [ImmutablePrototype] synthetic extended attribute.
- Instances of [Global] or [PrimaryGlobal]

This patch causes all three types of objects to be immutable prototype exotic
objects.

A number of tests previously checked what behavior occurs when the prototype
chain of the global object is mutated. This patch changes the tests to just
check that the prototype chain of the global object is immutable; tests which
check what happens after mutating are deleted as the path is unreachable.

BUG=v8:5149
Committed: https://crrev.com/c65700010f3c557867e23b24373499170959f845
Cr-Commit-Position: refs/heads/master@{#431204}

[Dan Ehrenberg, 2016-10-26 20:38:06]

Description was changed from

==========
Make global object prototype chain immutable

BUG=v8:5149
==========

to

==========
Freeze part of global prototype chain per WebIDL

The WebIDL spec says,

> If the interface is declared with the [Global] or [PrimaryGlobal] extended
attribute, or the interface is in the set of inherited interfaces for any other
interface that is declared with one of these attributes, then the interface
prototype object must be an immutable prototype exotic object.

From my read, this means that objects like Window.prototype,
EventTarget.prototype
and SharedWorkerGlobalScope.prototype need to be immutable prototype exotic
objects. That's what this patch does.

However, the included test demonstrates that there are still two links in the
chain
which are mutable, so more work is needed.

BUG=v8:5149
==========

[Dan Ehrenberg, 2016-10-26 20:38:20]

Description was changed from

==========
Freeze part of global prototype chain per WebIDL

The WebIDL spec says,

> If the interface is declared with the [Global] or [PrimaryGlobal] extended
attribute, or the interface is in the set of inherited interfaces for any other
interface that is declared with one of these attributes, then the interface
prototype object must be an immutable prototype exotic object.

From my read, this means that objects like Window.prototype,
EventTarget.prototype
and SharedWorkerGlobalScope.prototype need to be immutable prototype exotic
objects. That's what this patch does.

However, the included test demonstrates that there are still two links in the
chain
which are mutable, so more work is needed.

BUG=v8:5149
==========

to

==========
Freeze part of global prototype chain per WebIDL

The WebIDL spec says,

> If the interface is declared with the [Global] or [PrimaryGlobal] extended
> attribute, or the interface is in the set of inherited interfaces for any
other
> interface that is declared with one of these attributes, then the interface
> prototype object must be an immutable prototype exotic object.

From my read, this means that objects like Window.prototype,
EventTarget.prototype
and SharedWorkerGlobalScope.prototype need to be immutable prototype exotic
objects. That's what this patch does.

However, the included test demonstrates that there are still two links in the
chain
which are mutable, so more work is needed.

BUG=v8:5149
==========

[Dan Ehrenberg, 2016-10-26 20:40:02]

littledan@chromium.org changed reviewers:
+ domenic@chromium.org, haraken@chromium.org

[domenic, 2016-10-26 20:46:32]

> From my read, this means that objects like Window.prototype,
EventTarget.prototype
and SharedWorkerGlobalScope.prototype need to be immutable prototype exotic
objects. That's what this patch does.

This missed a few: WorkerGlobalScope, DedicatedWorkerGlobalScope,
ServiceWorkerGlobalScope, and I think WorkletGlobalScope. I'm not sure how that
maps to the code, but at least we should write some tests.

Speaking of tests, if you write them using testharness.js instead of
testRunner.*, we can upstream them to web-platform-tests.

[Dan Ehrenberg, 2016-10-26 20:53:10]

On 2016/10/26 at 20:46:32, domenic wrote:
> > From my read, this means that objects like Window.prototype,
EventTarget.prototype
> and SharedWorkerGlobalScope.prototype need to be immutable prototype exotic
> objects. That's what this patch does.
> 
> This missed a few: WorkerGlobalScope, DedicatedWorkerGlobalScope,
ServiceWorkerGlobalScope, and I think WorkletGlobalScope. I'm not sure how that
maps to the code, but at least we should write some tests.

I think those should be covered, since the logic is EventTarget + anything with
a Global/PrimaryGlobal property. Looks like there's some more specialized
worklet scopes with idl files checked in as well, FWIW.
> 
> Speaking of tests, if you write them using testharness.js instead of
testRunner.*, we can upstream them to web-platform-tests.

Wasn't aware of that, will do.

[Dan Ehrenberg, 2016-10-26 21:27:28]

Description was changed from

==========
Freeze part of global prototype chain per WebIDL

The WebIDL spec says,

> If the interface is declared with the [Global] or [PrimaryGlobal] extended
> attribute, or the interface is in the set of inherited interfaces for any
other
> interface that is declared with one of these attributes, then the interface
> prototype object must be an immutable prototype exotic object.

From my read, this means that objects like Window.prototype,
EventTarget.prototype
and SharedWorkerGlobalScope.prototype need to be immutable prototype exotic
objects. That's what this patch does.

However, the included test demonstrates that there are still two links in the
chain
which are mutable, so more work is needed.

BUG=v8:5149
==========

to

==========
Freeze part of global prototype chain per WebIDL

The WebIDL spec says,

> If the interface is declared with the [Global] or [PrimaryGlobal] extended
> attribute, or the interface is in the set of inherited interfaces for any
other
> interface that is declared with one of these attributes, then the interface
> prototype object must be an immutable prototype exotic object.

From my read, this means that objects like Window.prototype,
EventTarget.prototype
and SharedWorkerGlobalScope.prototype need to be immutable prototype exotic
objects. That's the first part of what this patch does.

This patch also freezes the prototype chain of named properties objects, as
specified in WebIDL.

However, the included test demonstrates that the global object itself still has
a mutable prototype.

BUG=v8:5149
==========

[haraken, 2016-10-27 08:47:06]

https://codereview.chromium.org/2452073002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/2452073002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:204:
is_immutable_prototype = is_global or 'ImmutablePrototype' in
extended_attributes

Why is is_global not enough? You want to set the immutable prototype only on
[Global] or [PrimaryGlobal] interfaces, right?

https://codereview.chromium.org/2452073002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/events/EventTarget.idl (right):

https://codereview.chromium.org/2452073002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/events/EventTarget.idl:26: ImmutablePrototype,

Why do you want to have [ImmutablePrototype] on EventTarget?

[Dan Ehrenberg, 2016-10-27 09:03:45]

https://codereview.chromium.org/2452073002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/2452073002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:204:
is_immutable_prototype = is_global or 'ImmutablePrototype' in
extended_attributes
On 2016/10/27 at 08:47:05, haraken wrote:
> Why is is_global not enough? You want to set the immutable prototype only on
[Global] or [PrimaryGlobal] interfaces, right?

Also EventTarget

https://codereview.chromium.org/2452073002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/events/EventTarget.idl (right):

https://codereview.chromium.org/2452073002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/events/EventTarget.idl:26: ImmutablePrototype,
On 2016/10/27 at 08:47:05, haraken wrote:
> Why do you want to have [ImmutablePrototype] on EventTarget?

Because EventTarget.prototype is in the prototype chain of the global object,
and that whole prototype chain has to be immutable.

[Yuki, 2016-10-27 09:45:54]

yukishiino@chromium.org changed reviewers:
+ yukishiino@chromium.org

[Yuki, 2016-10-27 09:45:54]

lgtm with domenic@'s comments.

[Dan Ehrenberg, 2016-11-03 19:02:57]

I changed the tests per comments (demonstrating further remaining issues); PTAL

[domenic, 2016-11-03 19:13:04]

On 2016/11/03 at 19:02:57, littledan wrote:
> I changed the tests per comments (demonstrating further remaining issues);
PTAL

Tests LGTM. To fix the extra mutable link below DedicatedWorkerGlobalScope, add
[ImmutablePrototype] to WorkerGlobalScope.

[Dan Ehrenberg, 2016-11-04 00:49:38]

Description was changed from

==========
Freeze part of global prototype chain per WebIDL

The WebIDL spec says,

> If the interface is declared with the [Global] or [PrimaryGlobal] extended
> attribute, or the interface is in the set of inherited interfaces for any
other
> interface that is declared with one of these attributes, then the interface
> prototype object must be an immutable prototype exotic object.

From my read, this means that objects like Window.prototype,
EventTarget.prototype
and SharedWorkerGlobalScope.prototype need to be immutable prototype exotic
objects. That's the first part of what this patch does.

This patch also freezes the prototype chain of named properties objects, as
specified in WebIDL.

However, the included test demonstrates that the global object itself still has
a mutable prototype.

BUG=v8:5149
==========

to

==========
Freeze global prototype chain per WebIDL

There are three places where WebIDL freezes the global object prototype chain:
- Named properties objects have an immutable prototype
- Prototypes of interfaces declared with [Global] or [PrimaryGlobal], and
  classes which are inherited from by those. For the latter, this patch
  introduces an [ImmutablePrototype] synthetic extended attribute.
- Instances of [Global] or [PrimaryGlobal]

This patch causes all three types of objects to be immutable prototype exotic
objects.

BUG=v8:5149
==========

[Dan Ehrenberg, 2016-11-04 00:50:10]

The new version of this patch actually makes the whole chain immutable, once the
V8 patch in https://codereview.chromium.org/2474843003/ goes in

[haraken, 2016-11-04 00:57:08]

https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:204:
is_immutable_prototype = is_global or 'ImmutablePrototype' in
extended_attributes

Won't this make Node.__proto__ immutable as well?

IIUC, we want to make Window.__proto__.__proto__.__proto__ immutable but don't
want to make Node.__proto__ immutable.

[domenic, 2016-11-04 01:14:27]

On 2016/11/04 at 00:57:08, haraken wrote:
>
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):
> 
>
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> third_party/WebKit/Source/bindings/scripts/v8_interface.py:204:
is_immutable_prototype = is_global or 'ImmutablePrototype' in
extended_attributes
> 
> Won't this make Node.__proto__ immutable as well?
> 
> IIUC, we want to make Window.__proto__.__proto__.__proto__ immutable but don't
want to make Node.__proto__ immutable.

Why would it make Node.__proto__ (=== EventTarget) immutable?

[Dan Ehrenberg, 2016-11-04 01:23:52]

On 2016/11/04 at 01:14:27, domenic wrote:
> On 2016/11/04 at 00:57:08, haraken wrote:
> >
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):
> > 
> >
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/bindings/scripts/v8_interface.py:204:
is_immutable_prototype = is_global or 'ImmutablePrototype' in
extended_attributes
> > 
> > Won't this make Node.__proto__ immutable as well?
> > 
> > IIUC, we want to make Window.__proto__.__proto__.__proto__ immutable but
don't want to make Node.__proto__ immutable.
> 
> Why would it make Node.__proto__ (=== EventTarget) immutable?

Node.__proto__ will have an immutable prototype, that is,
Node.__proto__.__proto__ = {} should fail. However, I don't see why this should
make Node.__proto__ = {} fail. Seems like I should write tests for these
things...

[haraken, 2016-11-04 01:28:28]

On 2016/11/04 01:23:52, Dan Ehrenberg wrote:
> On 2016/11/04 at 01:14:27, domenic wrote:
> > On 2016/11/04 at 00:57:08, haraken wrote:
> > >
>
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> > > File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):
> > > 
> > >
>
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> > > third_party/WebKit/Source/bindings/scripts/v8_interface.py:204:
> is_immutable_prototype = is_global or 'ImmutablePrototype' in
> extended_attributes
> > > 
> > > Won't this make Node.__proto__ immutable as well?
> > > 
> > > IIUC, we want to make Window.__proto__.__proto__.__proto__ immutable but
> don't want to make Node.__proto__ immutable.
> > 
> > Why would it make Node.__proto__ (=== EventTarget) immutable?
> 
> Node.__proto__ will have an immutable prototype, that is,
> Node.__proto__.__proto__ = {} should fail. However, I don't see why this
should
> make Node.__proto__ = {} fail. Seems like I should write tests for these
> things...

You're adding [ImmutablePrototype] to EventTarget.idl, which means that it will
affect all EventTargets, right?

[domenic, 2016-11-04 02:20:20]

On 2016/11/04 at 01:28:28, haraken wrote:
> On 2016/11/04 01:23:52, Dan Ehrenberg wrote:
> > On 2016/11/04 at 01:14:27, domenic wrote:
> > > On 2016/11/04 at 00:57:08, haraken wrote:
> > > >
> >
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> > > > File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):
> > > > 
> > > >
> >
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> > > > third_party/WebKit/Source/bindings/scripts/v8_interface.py:204:
> > is_immutable_prototype = is_global or 'ImmutablePrototype' in
> > extended_attributes
> > > > 
> > > > Won't this make Node.__proto__ immutable as well?
> > > > 
> > > > IIUC, we want to make Window.__proto__.__proto__.__proto__ immutable but
> > don't want to make Node.__proto__ immutable.
> > > 
> > > Why would it make Node.__proto__ (=== EventTarget) immutable?
> > 
> > Node.__proto__ will have an immutable prototype, that is,
> > Node.__proto__.__proto__ = {} should fail.

This just seems incorrect, right? Node.__proto__ === EventTarget, which is not
made immutable by this patch. EventTarget.prototype (===
Node.prototype.__proto__) is what is made immutable by this patch.

> You're adding [ImmutablePrototype] to EventTarget.idl, which means that it
will affect all EventTargets, right?

No, why would it? When you add [Constructor] or [Exposed] or [NoInterfaceObject]
to EventTarget, it doesn't affect Node, or any other EventTarget. It's the same
for [ImmutablePrototype].

[haraken, 2016-11-04 02:27:20]

On 2016/11/04 02:20:20, domenic wrote:
> On 2016/11/04 at 01:28:28, haraken wrote:
> > On 2016/11/04 01:23:52, Dan Ehrenberg wrote:
> > > On 2016/11/04 at 01:14:27, domenic wrote:
> > > > On 2016/11/04 at 00:57:08, haraken wrote:
> > > > >
> > >
>
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> > > > > File third_party/WebKit/Source/bindings/scripts/v8_interface.py
(right):
> > > > > 
> > > > >
> > >
>
https://codereview.chromium.org/2452073002/diff/220001/third_party/WebKit/Sou...
> > > > > third_party/WebKit/Source/bindings/scripts/v8_interface.py:204:
> > > is_immutable_prototype = is_global or 'ImmutablePrototype' in
> > > extended_attributes
> > > > > 
> > > > > Won't this make Node.__proto__ immutable as well?
> > > > > 
> > > > > IIUC, we want to make Window.__proto__.__proto__.__proto__ immutable
but
> > > don't want to make Node.__proto__ immutable.
> > > > 
> > > > Why would it make Node.__proto__ (=== EventTarget) immutable?
> > > 
> > > Node.__proto__ will have an immutable prototype, that is,
> > > Node.__proto__.__proto__ = {} should fail.
> 
> This just seems incorrect, right? Node.__proto__ === EventTarget, which is not
> made immutable by this patch. EventTarget.prototype (===
> Node.prototype.__proto__) is what is made immutable by this patch.

Ah, I was confused. You're right!
 
> > You're adding [ImmutablePrototype] to EventTarget.idl, which means that it
> will affect all EventTargets, right?
> 
> No, why would it? When you add [Constructor] or [Exposed] or
[NoInterfaceObject]
> to EventTarget, it doesn't affect Node, or any other EventTarget. It's the
same
> for [ImmutablePrototype].

[haraken, 2016-11-04 02:28:57]

LGTM

https://codereview.chromium.org/2452073002/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/2452073002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:203: # as in the
WebIDL spec?

Then this comment doesn't make sense, right? We just need to make
EventTarget.prototype immutable. Inheritance doesn't matter.

[Dan Ehrenberg, 2016-11-04 16:04:57]

https://codereview.chromium.org/2452073002/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/2452073002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:203: # as in the
WebIDL spec?
On 2016/11/04 at 02:28:56, haraken wrote:
> Then this comment doesn't make sense, right? We just need to make
EventTarget.prototype immutable. Inheritance doesn't matter.

There are a number of things that have ImmutablePrototype set. WebIDL doesn't
include this attribute; instead, WebIDL says that any class which is inherited
from by a Global or PrimaryGlobal acts like this.

[Dan Ehrenberg, 2016-11-04 18:35:43]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 18:36:34]

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-04 18:50:26]

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

[commit-bot: I haz the power, 2016-11-04 18:50:27]

Dry run: 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...)

[Dan Ehrenberg, 2016-11-04 20:10:37]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

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

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

[Dan Ehrenberg, 2016-11-05 02:54:57]

The CQ bit was checked by littledan@chromium.org

[Dan Ehrenberg, 2016-11-05 02:54:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
yukishiino@chromium.org, domenic@chromium.org, haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2452073002/#ps260001
(title: "Remove paint worklet tests")

[commit-bot: I haz the power, 2016-11-05 02:55:15]

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-05 04:06:58]

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

[commit-bot: I haz the power, 2016-11-05 04:06:59]

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

[Dan Ehrenberg, 2016-11-07 20:18:43]

The CQ bit was checked by littledan@chromium.org

[commit-bot: I haz the power, 2016-11-07 20:19: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-11-07 21:51:38]

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

[commit-bot: I haz the power, 2016-11-07 21:51:40]

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

[adamk, 2016-11-07 22:09:05]

adamk@chromium.org changed reviewers:
+ adamk@chromium.org

[adamk, 2016-11-07 22:09:05]

Looks like some actual test failures? E.g.,
https://storage.googleapis.com/chromium-layout-test-archives/linux_chromium_r...:

This is a testharness.js-based test.
PASS Named access test.  Window's members should have priority over named
properties. 
PASS WindowProperties object should exist. 
FAIL WindowProperties object should provide named access. Immutable prototype
object '#<Window>' cannot have their prototype set
PASS Window's members should be own members. 
Harness: the test ran to completion.

[Dan Ehrenberg, 2016-11-08 22:14:13]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-08 22:15:05]

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

[Dan Ehrenberg, 2016-11-09 01:01:14]

Changed some tests, PTAL

[Dan Ehrenberg, 2016-11-09 01:02:53]

Description was changed from

==========
Freeze global prototype chain per WebIDL

There are three places where WebIDL freezes the global object prototype chain:
- Named properties objects have an immutable prototype
- Prototypes of interfaces declared with [Global] or [PrimaryGlobal], and
  classes which are inherited from by those. For the latter, this patch
  introduces an [ImmutablePrototype] synthetic extended attribute.
- Instances of [Global] or [PrimaryGlobal]

This patch causes all three types of objects to be immutable prototype exotic
objects.

BUG=v8:5149
==========

to

==========
Freeze global prototype chain per WebIDL

There are three places where WebIDL freezes the global object prototype chain:
- Named properties objects have an immutable prototype
- Prototypes of interfaces declared with [Global] or [PrimaryGlobal], and
  classes which are inherited from by those. For the latter, this patch
  introduces an [ImmutablePrototype] synthetic extended attribute.
- Instances of [Global] or [PrimaryGlobal]

This patch causes all three types of objects to be immutable prototype exotic
objects.

A number of tests previously checked what behavior occurs when the prototype
chain of the global object is mutated. This patch changes the tests to just
check that the prototype chain of the global object is immutable; tests which
check what happens after mutating are deleted as the path is unreachable.

BUG=v8:5149
==========

[adamk, 2016-11-09 01:08:50]

https://codereview.chromium.org/2452073002/diff/300001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/Window/window-custom-prototype-crash.html
(right):

https://codereview.chromium.org/2452073002/diff/300001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/Window/window-custom-prototype-crash.html:10:
If this did crash the test has succeeded.
This test should just be deleted, it's a regression test for a crash, and if we
can't even go down that path it can die now.

https://codereview.chromium.org/2452073002/diff/300001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/security/xss-getownproperty.html
(left):

https://codereview.chromium.org/2452073002/diff/300001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/security/xss-getownproperty.html:3:
Test should log two "PASS" messages to the console.
This test looks unrelated, did it get accidentally caught up in the deletion?

[Dan Ehrenberg, 2016-11-09 01:12:16]

The CQ bit was checked by littledan@chromium.org

[Dan Ehrenberg, 2016-11-09 01:12:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, domenic@chromium.org, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2452073002/#ps320001
(title: "Remove useless test")

[Dan Ehrenberg, 2016-11-09 01:12:32]

https://codereview.chromium.org/2452073002/diff/300001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/Window/window-custom-prototype-crash.html
(right):

https://codereview.chromium.org/2452073002/diff/300001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/Window/window-custom-prototype-crash.html:10:
If this did crash the test has succeeded.
On 2016/11/09 at 01:08:50, adamk wrote:
> This test should just be deleted, it's a regression test for a crash, and if
we can't even go down that path it can die now.

Deleted

https://codereview.chromium.org/2452073002/diff/300001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/security/xss-getownproperty.html
(left):

https://codereview.chromium.org/2452073002/diff/300001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/security/xss-getownproperty.html:3:
Test should log two "PASS" messages to the console.
On 2016/11/09 at 01:08:50, adamk wrote:
> This test looks unrelated, did it get accidentally caught up in the deletion?

This is also a test for the mutation of the global object prototype chain; see
security/resources/doc-with-iframe.html

[commit-bot: I haz the power, 2016-11-09 01:12:52]

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

[Dan Ehrenberg, 2016-11-09 01:15:16]

The CQ bit was unchecked by littledan@chromium.org

[adamk, 2016-11-09 01:15:42]

lgtm, but I'd like yukishiino to take a last look at this (particularly make
sure that the test deletion sounds as reasonable as it did to me)

[Yuki, 2016-11-09 07:06:44]

LGTM.

I'm fine with deleting these tests.

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/Window/window-custom-prototype-expected.txt
(left):

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/Window/window-custom-prototype-expected.txt:6:
PASS __proto__ = window; __proto threw exception TypeError: Cyclic __proto__
value.
Do we have the same test for non-global object at another place?
I think it's better to have a test to check if it throws when creating a cyclic
prototype chain.

If there is no similar test, would you mind to add a test for a non-global
object?

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/security/xss-DENIED-htmlelelment-with-iframe-proto.html
(left):

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/security/xss-DENIED-htmlelelment-with-iframe-proto.html:6:
<iframe src="http://localhost:8000/security/resources/iframe-with-element.html"
style="">
It seems you can delete http/tests/security/resources/iframe-with-element.html,
too.

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/security/immutable-prototype-sharedworker.html
(right):

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/security/immutable-prototype-sharedworker.html:19:

nit: unnecessary empty lines.

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/security/immutable-prototype-worker.html
(right):

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/security/immutable-prototype-worker.html:18: 
nit: unnecessary empty lines.

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/security/immutable-prototype.html (right):

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/security/immutable-prototype.html:16: 
nit: unnecessary empty line.

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/security/immutable-prototype.js (right):

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/security/immutable-prototype.js:16: 
nit: unnecessary empty line.

[Dan Ehrenberg, 2016-11-09 23:51:12]

Made the cleanups suggested; I'd like a quick review of the new test.

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/Window/window-custom-prototype-expected.txt
(left):

https://codereview.chromium.org/2452073002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/Window/window-custom-prototype-expected.txt:6:
PASS __proto__ = window; __proto threw exception TypeError: Cyclic __proto__
value.
On 2016/11/09 at 07:06:43, Yuki wrote:
> Do we have the same test for non-global object at another place?
> I think it's better to have a test to check if it throws when creating a
cyclic prototype chain.
> 
> If there is no similar test, would you mind to add a test for a non-global
object?

Added a test at the end of this file for a similar circularity check. WDYT?

[Yuki, 2016-11-10 06:19:40]

LGTM.  Thanks for adding a test.

[Yuki, 2016-11-10 06:19:47]

The CQ bit was checked by yukishiino@chromium.org

[Yuki, 2016-11-10 06:19:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, domenic@chromium.org, adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2452073002/#ps340001
(title: "Test improvements")

[commit-bot: I haz the power, 2016-11-10 06:20:04]

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-10 07:47:27]

Committed patchset #18 (id:340001)

[commit-bot: I haz the power, 2016-11-10 07:49:36]

Description was changed from

==========
Freeze global prototype chain per WebIDL

There are three places where WebIDL freezes the global object prototype chain:
- Named properties objects have an immutable prototype
- Prototypes of interfaces declared with [Global] or [PrimaryGlobal], and
  classes which are inherited from by those. For the latter, this patch
  introduces an [ImmutablePrototype] synthetic extended attribute.
- Instances of [Global] or [PrimaryGlobal]

This patch causes all three types of objects to be immutable prototype exotic
objects.

A number of tests previously checked what behavior occurs when the prototype
chain of the global object is mutated. This patch changes the tests to just
check that the prototype chain of the global object is immutable; tests which
check what happens after mutating are deleted as the path is unreachable.

BUG=v8:5149
==========

to

==========
Freeze global prototype chain per WebIDL

There are three places where WebIDL freezes the global object prototype chain:
- Named properties objects have an immutable prototype
- Prototypes of interfaces declared with [Global] or [PrimaryGlobal], and
  classes which are inherited from by those. For the latter, this patch
  introduces an [ImmutablePrototype] synthetic extended attribute.
- Instances of [Global] or [PrimaryGlobal]

This patch causes all three types of objects to be immutable prototype exotic
objects.

A number of tests previously checked what behavior occurs when the prototype
chain of the global object is mutated. This patch changes the tests to just
check that the prototype chain of the global object is immutable; tests which
check what happens after mutating are deleted as the path is unreachable.

BUG=v8:5149

Committed: https://crrev.com/c65700010f3c557867e23b24373499170959f845
Cr-Commit-Position: refs/heads/master@{#431204}
==========

[commit-bot: I haz the power, 2016-11-10 07:49:37]

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