Slim V8DOMConfiguration::MethodConfiguration by refactoring per-world bindings.

Slim V8DOMConfiguration::MethodConfiguration by refactoring per-world bindings.

This represents the main world version as a separate entry in the configuration list.
While this is larger for methods with PerWorldBindings, it's smaller for those without,
which are the vast majority.

Effect is a change of 10-15 KiB in the .data.rel.ro section on Linux x64.
Android Monochrome (32-bit ARM) APK size shrinks by 8 KiB.

Review-Url: https://codereview.chromium.org/2717183002
Cr-Commit-Position: refs/heads/master@{#457922}
Committed: https://chromium.googlesource.com/chromium/src/+/4f43563b2d986dd686d4f20a14fc15fe02884489

[jbroman, 2017-02-26 21:40:00]

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

[commit-bot: I haz the power, 2017-02-26 21:40:06]

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, 2017-02-26 23:10:22]

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

[commit-bot: I haz the power, 2017-02-26 23:10:23]

Dry run: This issue passed the CQ dry run.

[jbroman, 2017-03-17 18:01:55]

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

[commit-bot: I haz the power, 2017-03-17 18:03:06]

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

[jbroman, 2017-03-17 18:05:11]

Description was changed from

==========
Slim V8DOMConfiguration::MethodConfiguration by refactoring per-world bindings.

This represents the main world version as a separate entry in the configuration
list.
While this is larger for methods with PerWorldBindings, it's smaller for those
without,
which are the vast majority.

Effect is a change of 10-15 KiB in the .data.rel.ro section.

BUG=
==========

to

==========
Slim V8DOMConfiguration::MethodConfiguration by refactoring per-world bindings.

This represents the main world version as a separate entry in the configuration
list.
While this is larger for methods with PerWorldBindings, it's smaller for those
without,
which are the vast majority.

Effect is a change of 10-15 KiB in the .data.rel.ro section on Linux x64.
Android Monochrome (32-bit ARM) APK size shrinks by 8 KiB.
==========

[jbroman, 2017-03-17 18:08:00]

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

[jbroman, 2017-03-17 18:08:01]

This just changes how we represent per-world bindings in the array. WDYT? This
approach would easily be extended to attributes/accessors in a followup if this
looks reasonable to you.

[haraken, 2017-03-17 18:16:15]

LGTM

Maybe can we implement this on DOM attribute callbacks as well?

https://codereview.chromium.org/2717183002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.h (right):

https://codereview.chromium.org/2717183002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.h:71: OtherWorlds
= 1 << 1,

NonMainWorlds ?

https://codereview.chromium.org/2717183002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl (right):

https://codereview.chromium.org/2717183002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl:609:
{"{{method.name}}", {{method_callback}}, {{method.length}},
{{property_attribute}}, {{property_location(method)}}, {{holder_check}},
{{access_check}}, V8DOMConfiguration::AllWorlds &
~V8DOMConfiguration::MainWorld}

Isn't this equivalent to V8DOMConfiguration::NonMainWorlds?

[jbroman, 2017-03-17 18:31:46]

The CQ bit was unchecked by jbroman@chromium.org

[jbroman, 2017-03-17 18:32:43]

On 2017/03/17 at 18:16:15, haraken wrote:
> LGTM
> 
> Maybe can we implement this on DOM attribute callbacks as well?

Yup, absolutely. Just wanted to do it in smaller separate patches.

https://codereview.chromium.org/2717183002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.h (right):

https://codereview.chromium.org/2717183002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.h:71: OtherWorlds
= 1 << 1,
On 2017/03/17 at 18:16:14, haraken wrote:
> NonMainWorlds ?

Done. I'd named it for the hypothetical possibility of us adding other kinds of
per-world bindings, but we can rename in the unlikely event that ever happens.

https://codereview.chromium.org/2717183002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl (right):

https://codereview.chromium.org/2717183002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl:609:
{"{{method.name}}", {{method_callback}}, {{method.length}},
{{property_attribute}}, {{property_location(method)}}, {{holder_check}},
{{access_check}}, V8DOMConfiguration::AllWorlds &
~V8DOMConfiguration::MainWorld}
On 2017/03/17 at 18:16:14, haraken wrote:
> Isn't this equivalent to V8DOMConfiguration::NonMainWorlds?

Yup. It just seemed to me to be saying "all worlds but the main world". With the
rename you suggested, that's already obvious. Done.

[jbroman, 2017-03-17 18:32:47]

The CQ bit was checked by jbroman@chromium.org

[jbroman, 2017-03-17 18:32:47]

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

[commit-bot: I haz the power, 2017-03-17 18:33:28]

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, 2017-03-17 21:30:48]

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

[commit-bot: I haz the power, 2017-03-17 21:30:49]

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

[jbroman, 2017-03-18 00:38:34]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2017-03-18 00:39:07]

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, 2017-03-18 01:51:47]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489797514694650,
"parent_rev": "d2c24342da278481fc6041e235f8690828e6760d", "commit_rev":
"4f43563b2d986dd686d4f20a14fc15fe02884489"}

[commit-bot: I haz the power, 2017-03-18 01:52:32]

Description was changed from

==========
Slim V8DOMConfiguration::MethodConfiguration by refactoring per-world bindings.

This represents the main world version as a separate entry in the configuration
list.
While this is larger for methods with PerWorldBindings, it's smaller for those
without,
which are the vast majority.

Effect is a change of 10-15 KiB in the .data.rel.ro section on Linux x64.
Android Monochrome (32-bit ARM) APK size shrinks by 8 KiB.
==========

to

==========
Slim V8DOMConfiguration::MethodConfiguration by refactoring per-world bindings.

This represents the main world version as a separate entry in the configuration
list.
While this is larger for methods with PerWorldBindings, it's smaller for those
without,
which are the vast majority.

Effect is a change of 10-15 KiB in the .data.rel.ro section on Linux x64.
Android Monochrome (32-bit ARM) APK size shrinks by 8 KiB.

Review-Url: https://codereview.chromium.org/2717183002
Cr-Commit-Position: refs/heads/master@{#457922}
Committed:
https://chromium.googlesource.com/chromium/src/+/4f43563b2d986dd686d4f20a14fc...
==========

[commit-bot: I haz the power, 2017-03-18 01:52:33]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/4f43563b2d986dd686d4f20a14fc...

[Yuki, 2017-03-21 10:05:47]

LGTM.