Make code generator understand FrozenArray

Make code generator understand FrozenArray

Currently FrozenArray is just an alias of Array. This means that:
- Converted V8 arrays are not frozen.
- Returned values are always newly created.

The latter isn't actually a spec violence.

BUG=515920
Committed: https://crrev.com/811a7ec0a438e4be75620f2a0c20c08deb5e1773
Cr-Commit-Position: refs/heads/master@{#387507}

[bashi, 2016-04-14 05:12:11]

Patchset #1 (id:1) has been deleted

[bashi, 2016-04-14 05:27:48]

bashi@chromium.org changed reviewers:
+ haraken@chromium.org, yukishiino@chromium.org

[bashi, 2016-04-14 05:27:49]

PTAL?

We might want to address the problems described in the description first...

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
(right):

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp:1597:
v8SetReturnValue(info, toV8(impl->stringFrozenArrayAttribute(), info.Holder(),
info.GetIsolate()));
An easy option for making FrozenArrays references would be storing the result of
toV8() to info.Holder() as a hidden value (like 'any'). Any other ideas?

[Yuki, 2016-04-14 05:54:41]

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/idl_definitions.py (right):

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/idl_definitions.py:1043: else:
I'd prefer
  elif class_name == 'ForzenArray':
    ...
  else
    raise an exception
for possible future expansions.

FrozenArray by default seems a bit weird.

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
(right):

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp:1597:
v8SetReturnValue(info, toV8(impl->stringFrozenArrayAttribute(), info.Holder(),
info.GetIsolate()));
On 2016/04/14 05:27:49, bashi1 wrote:
> An easy option for making FrozenArrays references would be storing the result
of
> toV8() to info.Holder() as a hidden value (like 'any'). Any other ideas?

I was thinking the same thing, but my question is whether or not FrozenArray
implies [SameObject] or not.  It's not clear for me.  If not [SameObject],
storing the result in a hidden value doesn't make much sense.

[bashi, 2016-04-14 07:33:13]

Description was changed from

==========
Make code generator understand FrozenArray

Currently FrozenArray is just an alias of Array. This means that:
- Converted V8 arrays are not frozen.
- Returned values are not references.

BUG=515920
==========

to

==========
Make code generator understand FrozenArray

Currently FrozenArray is just an alias of Array. This means that:
- Converted V8 arrays are not frozen.
- Returned values are always newly created.

The latter isn't actually a spec violence.

BUG=515920
==========

[bashi, 2016-04-14 07:34:30]

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/idl_definitions.py (right):

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/idl_definitions.py:1043: else:
On 2016/04/14 05:54:41, Yuki wrote:
> I'd prefer
>   elif class_name == 'ForzenArray':
>     ...
>   else
>     raise an exception
> for possible future expansions.
> 
> FrozenArray by default seems a bit weird.

Done.

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
(right):

https://codereview.chromium.org/1890733002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp:1597:
v8SetReturnValue(info, toV8(impl->stringFrozenArrayAttribute(), info.Holder(),
info.GetIsolate()));
On 2016/04/14 05:54:41, Yuki wrote:
> On 2016/04/14 05:27:49, bashi1 wrote:
> > An easy option for making FrozenArrays references would be storing the
result
> of
> > toV8() to info.Holder() as a hidden value (like 'any'). Any other ideas?
> 
> I was thinking the same thing, but my question is whether or not FrozenArray
> implies [SameObject] or not.  It's not clear for me.  If not [SameObject],
> storing the result in a hidden value doesn't make much sense.

Chatted offline. Conclusion is that we don't need to do that. Per
https://github.com/whatwg/notifications/issues/66#issuecomment-195466398,
[SameObject] and FrozenArray are orthogonal.

In some cases, it'd be better to return the same object when the values in the
array are unchanged, but it's not mandatory unless I miss something.

[Yuki, 2016-04-14 07:43:14]

lgtm

[haraken, 2016-04-14 07:52:27]

LGTM

[bashi, 2016-04-14 08:22:42]

The CQ bit was checked by bashi@chromium.org

[commit-bot: I haz the power, 2016-04-14 08:23:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1890733002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1890733002/40001

[commit-bot: I haz the power, 2016-04-14 10:58:35]

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

[commit-bot: I haz the power, 2016-04-14 10:58:35]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[bashi, 2016-04-14 23:22:34]

The CQ bit was checked by bashi@chromium.org

[bashi, 2016-04-14 23:22:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/1890733002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-14 23:22:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1890733002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1890733002/60001

[commit-bot: I haz the power, 2016-04-15 01:15:00]

Description was changed from

==========
Make code generator understand FrozenArray

Currently FrozenArray is just an alias of Array. This means that:
- Converted V8 arrays are not frozen.
- Returned values are always newly created.

The latter isn't actually a spec violence.

BUG=515920
==========

to

==========
Make code generator understand FrozenArray

Currently FrozenArray is just an alias of Array. This means that:
- Converted V8 arrays are not frozen.
- Returned values are always newly created.

The latter isn't actually a spec violence.

BUG=515920
==========

[commit-bot: I haz the power, 2016-04-15 01:15:02]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-04-15 01:16:36]

Description was changed from

==========
Make code generator understand FrozenArray

Currently FrozenArray is just an alias of Array. This means that:
- Converted V8 arrays are not frozen.
- Returned values are always newly created.

The latter isn't actually a spec violence.

BUG=515920
==========

to

==========
Make code generator understand FrozenArray

Currently FrozenArray is just an alias of Array. This means that:
- Converted V8 arrays are not frozen.
- Returned values are always newly created.

The latter isn't actually a spec violence.

BUG=515920

Committed: https://crrev.com/811a7ec0a438e4be75620f2a0c20c08deb5e1773
Cr-Commit-Position: refs/heads/master@{#387507}
==========

[commit-bot: I haz the power, 2016-04-15 01:16:37]

Patchset 3 (id:??) landed as
https://crrev.com/811a7ec0a438e4be75620f2a0c20c08deb5e1773
Cr-Commit-Position: refs/heads/master@{#387507}