binding: Refactors bindings/templates/attributes.cpp.

binding: Refactors bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable.

BUG=

Committed: https://crrev.com/70f76152ff54f603de5d51eb605d47dfad97e2ae
Cr-Commit-Position: refs/heads/master@{#421161}

[Yuki, 2016-09-15 14:29:05]

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

[commit-bot: I haz the power, 2016-09-15 14:29:20]

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-09-15 15:30:42]

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

[commit-bot: I haz the power, 2016-09-15 15:30:43]

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

[Yuki, 2016-09-16 15:17:01]

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

[commit-bot: I haz the power, 2016-09-16 15:17:22]

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-09-16 15:20:47]

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

[commit-bot: I haz the power, 2016-09-16 15:20:48]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Yuki, 2016-09-16 15:24:25]

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

[commit-bot: I haz the power, 2016-09-16 15:24:43]

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-09-16 16:30:30]

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

[commit-bot: I haz the power, 2016-09-16 16:30:32]

Dry run: This issue passed the CQ dry run.

[Yuki, 2016-09-20 13:22:33]

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

[commit-bot: I haz the power, 2016-09-20 13:22:45]

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

[Yuki, 2016-09-20 14:25:40]

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

[Yuki, 2016-09-20 14:25:41]

Could you review this CL?

[Yuki, 2016-09-20 14:27:33]

Description was changed from

==========
binding: Minor refactoring of bindings/templates/attributes.cpp.

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

to

==========
binding: Minor refactoring of bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable, switching from
hidden values to private properties.

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

[Yuki, 2016-09-20 14:30:33]

Description was changed from

==========
binding: Minor refactoring of bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable, switching from
hidden values to private properties.

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

to

==========
binding: Minor refactoring of bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable,
switching from hidden values to private properties.

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

[commit-bot: I haz the power, 2016-09-20 14:46:36]

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

[commit-bot: I haz the power, 2016-09-20 14:46:37]

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

[haraken, 2016-09-20 14:52:31]

As mentioned in a previous CL, consider making one change per CL; i.e., split
the refactoring part from the HiddenValue => PrivateProperty replacement.

Regarding the HiddenValue => PrivateProperty replacement, I'm not sure if it's
worth doing at this point because the attribute caching will be available very
soon. Then we should just use the attribute caching.

https://codereview.chromium.org/2342053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h (right):

https://codereview.chromium.org/2342053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h:112: return
v8CallBoolean(object->DeletePrivate(context, m_privateSymbol));

Don't use v8Call macros any more :)

[Yuki, 2016-09-20 14:57:15]

Done.  Could you take another look?

[Yuki, 2016-09-20 14:57:20]

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

[commit-bot: I haz the power, 2016-09-20 14:57:42]

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

[haraken, 2016-09-20 15:17:26]

This is never a minor refactoring... Please consider making changes more
incrementally from next time.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_attributes.py (right):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_attributes.py:107:
includes.add('bindings/core/v8/V8PrivateProperty.h')

Revert this change.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (left):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:319:
exceptionState.clearException();

Why can we remove clearException?

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (right):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:137:
v8SetReturnValueNull(info);

What is this change for?

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:145:
privateKeepAlive.set(info.GetIsolate()->GetCurrentContext(), holder, v8Value);

Revert this change.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:267: {% set
define_exception_state -%}

Nit: I'd prefer not introducing yet another macro whose contents will be removed
by format_remove_duplicates; i.e., I'd prefer just writing the ExceptionState
definition multiple times.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:336:
TrackExceptionState trackExceptionState;

Why did you change NonThrowableExceptionState to TrackExceptionState?

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
(right):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp:164: 

Is it worth inserting these empty lines?

[commit-bot: I haz the power, 2016-09-20 16:35:37]

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

[commit-bot: I haz the power, 2016-09-20 16:35:38]

Dry run: This issue passed the CQ dry run.

[peria, 2016-09-21 02:27:10]

I still feel current change is complicated, and am not sure it does only
refactoring.

Plus, please update the description.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (right):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:103: {% if
attribute.use_output_parameter_for_result %}
Do we need to move this part?

[Yuki, 2016-09-21 10:29:05]

Description was changed from

==========
binding: Minor refactoring of bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable,
switching from hidden values to private properties.

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

to

==========
binding: Refactors bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable.

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

[Yuki, 2016-09-21 10:29:31]

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_attributes.py (right):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_attributes.py:107:
includes.add('bindings/core/v8/V8PrivateProperty.h')
On 2016/09/20 15:17:25, haraken wrote:
> 
> Revert this change.

Done.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (left):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:319:
exceptionState.clearException();
On 2016/09/20 15:17:25, haraken wrote:
> 
> Why can we remove clearException?

TrackExceptionState never throws an exception.
TrackExceptionState's dtor clears an exception if any.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (right):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:103: {% if
attribute.use_output_parameter_for_result %}
On 2016/09/21 02:27:10, peria wrote:
> Do we need to move this part?

If attribute.cpp_value were using exceptionState, this should be placed before
we check exceptionState.hadException().

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:137:
v8SetReturnValueNull(info);
On 2016/09/20 15:17:25, haraken wrote:
> 
> What is this change for?

I think returning |undefined| is wrong.  is_keep_alive_for_gc is used only for
Node, then the return type is likely Node or Node? or its inherited interface. 
It's unlikely to be (Node or undefined).  Then returning |undefined| is wrong. 
We should return |null| instead.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:145:
privateKeepAlive.set(info.GetIsolate()->GetCurrentContext(), holder, v8Value);
On 2016/09/20 15:17:26, haraken wrote:
> 
> Revert this change.

Done.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:267: {% set
define_exception_state -%}
On 2016/09/20 15:17:25, haraken wrote:
> 
> Nit: I'd prefer not introducing yet another macro whose contents will be
removed
> by format_remove_duplicates; i.e., I'd prefer just writing the ExceptionState
> definition multiple times.

IMHO, copy&pasting is a cause of bugs.  How do we maintain?  How to keep all of
them the same?

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:336:
TrackExceptionState trackExceptionState;
On 2016/09/20 15:17:25, haraken wrote:
> 
> Why did you change NonThrowableExceptionState to TrackExceptionState?

NonThrowableExceptionState's implementation is
    NOTREACHED();
It's simply wrong.

TrackExceptionState never throws an exception, but it helps detect an exception.

[Yuki, 2016-09-21 10:32:40]

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

[commit-bot: I haz the power, 2016-09-21 10:32:50]

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-09-21 11:54:08]

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

[commit-bot: I haz the power, 2016-09-21 11:54:09]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-09-21 12:36:37]

LGTM

As peria-san mentioned, you should describe all non-trivial changes in the CL
description.

Also please don't land this kind of non-trivial refactoring from next time.

[haraken, 2016-09-21 12:51:32]

>
https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/bindings/templates/attributes.cpp:267: {% set
> define_exception_state -%}
> On 2016/09/20 15:17:25, haraken wrote:
> > 
> > Nit: I'd prefer not introducing yet another macro whose contents will be
> removed
> > by format_remove_duplicates; i.e., I'd prefer just writing the
ExceptionState
> > definition multiple times.
> 
> IMHO, copy&pasting is a cause of bugs.  How do we maintain?  How to keep all
of
> them the same?

On the other hand, too many macros make the code more magical and less readable.

[Yuki, 2016-09-21 13:41:16]

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

[commit-bot: I haz the power, 2016-09-21 13:41:28]

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

[Yuki, 2016-09-21 14:12:42]

I split a bug fix out to https://crrev.com/2357243002 .

Any other ideas to make this CL small are welcome.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (right):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:137:
v8SetReturnValueNull(info);
On 2016/09/21 10:29:31, Yuki wrote:
> On 2016/09/20 15:17:25, haraken wrote:
> > 
> > What is this change for?
> 
> I think returning |undefined| is wrong.  is_keep_alive_for_gc is used only for
> Node, then the return type is likely Node or Node? or its inherited interface.

> It's unlikely to be (Node or undefined).  Then returning |undefined| is wrong.

> We should return |null| instead.

I was wrong.  The original code returns |null|.  I reverted this part.

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
(right):

https://codereview.chromium.org/2342053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp:164: 
On 2016/09/20 15:17:26, haraken wrote:
> 
> Is it worth inserting these empty lines?

I'm making the template files more readable with empty lines.
Probably it would be better to remove these empty lines, but I think it's more
important to keep the source files (i.e. template files) cleaner than to remove
empty lines from generated files.  And I don't want to introduce another magic
to remove these empty lines.

[Yuki, 2016-09-23 14:32:17]

The CQ bit was checked by yukishiino@chromium.org

[Yuki, 2016-09-23 14:32:18]

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/2342053002/#ps180001
(title: "Synced.")

[commit-bot: I haz the power, 2016-09-23 14:32:29]

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-09-23 14:50:07]

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

[commit-bot: I haz the power, 2016-09-23 14:50:09]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Yuki, 2016-09-23 15:13:18]

The CQ bit was checked by yukishiino@chromium.org

[commit-bot: I haz the power, 2016-09-23 15:13:32]

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-09-23 15:29:48]

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

[commit-bot: I haz the power, 2016-09-23 15:29:50]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Yuki, 2016-09-26 07:10:17]

The CQ bit was checked by yukishiino@chromium.org

[Yuki, 2016-09-26 07:10:18]

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/2342053002/#ps200001
(title: "Synced.")

[commit-bot: I haz the power, 2016-09-26 07:10:38]

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-09-26 09:38:18]

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

[commit-bot: I haz the power, 2016-09-26 09:38:20]

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

[Yuki, 2016-09-26 09:49:44]

The CQ bit was checked by yukishiino@chromium.org

[commit-bot: I haz the power, 2016-09-26 09:50:06]

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-09-26 12:16:47]

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

[commit-bot: I haz the power, 2016-09-26 12:16:49]

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

[Yuki, 2016-09-27 08:38:09]

The CQ bit was checked by yukishiino@chromium.org

[Yuki, 2016-09-27 08:38:10]

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/2342053002/#ps220001
(title: "Synced.")

[commit-bot: I haz the power, 2016-09-27 08:38:23]

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-09-27 10:08:52]

Description was changed from

==========
binding: Refactors bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable.

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

to

==========
binding: Refactors bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable.

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

[commit-bot: I haz the power, 2016-09-27 10:08:54]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-09-27 10:11:02]

Description was changed from

==========
binding: Refactors bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable.

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

to

==========
binding: Refactors bindings/templates/attributes.cpp.

Makes FooAttribute{G,S}etter{,Callback} a little more readable.

BUG=

Committed: https://crrev.com/70f76152ff54f603de5d51eb605d47dfad97e2ae
Cr-Commit-Position: refs/heads/master@{#421161}
==========

[commit-bot: I haz the power, 2016-09-27 10:11:03]

Patchset 12 (id:??) landed as
https://crrev.com/70f76152ff54f603de5d51eb605d47dfad97e2ae
Cr-Commit-Position: refs/heads/master@{#421161}

[agrieve, 2016-09-27 14:21:29]

On 2016/09/27 10:11:03, commit-bot: I haz the power wrote:
> Patchset 12 (id:??) landed as
> https://crrev.com/70f76152ff54f603de5d51eb605d47dfad97e2ae
> Cr-Commit-Position: refs/heads/master@{#421161}

Not sure if this was expected, but wanted to make you aware that this change
increased libchrome.so on Android by almost 30kb:
https://chromeperf.appspot.com/report?sid=f8be4459943df7093c2c0cdfa83b9350c74...

[Yuki, 2016-09-28 05:00:07]

On 2016/09/27 14:21:29, agrieve wrote:
> On 2016/09/27 10:11:03, commit-bot: I haz the power wrote:
> > Patchset 12 (id:??) landed as
> > https://crrev.com/70f76152ff54f603de5d51eb605d47dfad97e2ae
> > Cr-Commit-Position: refs/heads/master@{#421161}
> 
> Not sure if this was expected, but wanted to make you aware that this change
> increased libchrome.so on Android by almost 30kb:
>
https://chromeperf.appspot.com/report?sid=f8be4459943df7093c2c0cdfa83b9350c74...

Thanks for letting me know.  Will look.