bindings: Use ExceptionMessages when an interface/callback conversion fails.

bindings: Use ExceptionMessages when an interface/callback conversion fails.

https://codereview.chromium.org/2802223002 ("bindings: Throw a type error if
converting an interface or callback function fails") introduced a 32kb size
regression on release Android builds (is_official_build=true), as the new
error strings being thrown as type errors could not be shared and all made
it into the .rodata section.

Start using ExceptionMessages::FailedToConstruct() in the error messages.
Not only do we use the existing error infrastructure, but it allows us to
separate the part that always changes (the callback or interface name) from
the actual details (the "Unable to convert value" bit).

With commit 07c092298a6243fc2c829e517cf2b19f4caea26c ("Fetch: Make Headers'
constructor match the current spec IDL") as a baseline, the following data
can be observed with //tools/binary_size/map2size.py and console.py:

* Reverting the changes from https://codereview.chromium.org/2802223002
  versus this commit:

  Section Sizes (Total=-168 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: 256 bytes (-152.4%)
    .text: -424 bytes (252.4%)
  0 symbols added (+), 27 changed (~), 3 removed (-), 313837 unchanged (not shown)

  Showing 30 symbols with total size: -160 bytes
  .text=-416 bytes .rodata=256 bytes  other=0 bytes    total=-160 bytes
  Number of object files: 28

* Commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 versus this commit:

  Section Sizes (Total=-33,704 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: -33,280 bytes (98.7%)
    .text: -424 bytes (1.3%)
  0 symbols added (+), 27 changed (~), 3 removed (-), 313837 unchanged (not shown)

  Showing 30 symbols with total size: -33696 bytes
  .text=-416 bytes .rodata=-32.5kb    other=0 bytes    total=-32.9kb
  Number of object files: 28

BUG=690428, 710035
R=agrieve@chromium.org,bashi@chromium.org,haraken@chromium.org,jbroman@chromium.org,yukishiino@chromium.org

Review-Url: https://codereview.chromium.org/2809003004
Cr-Commit-Position: refs/heads/master@{#463753}
Committed: https://chromium.googlesource.com/chromium/src/+/81c4e4ab9d81757ce3dcb49185be46c880f3f22a

[Raphael Kubo da Costa (rakuco), 2017-04-11 13:35:14]

PTAL

[Raphael Kubo da Costa (rakuco), 2017-04-11 13:35:41]

Description was changed from

==========
bindings: Use ExceptionMessages when an interface/callback conversion fails.

https://codereview.chromium.org/2802223002 ("bindings: Throw a type error if
converting an interface or callback function fails") introduced a 32kb size
regression on release Android builds (is_official_build=true), as the new
error strings being thrown as type errors could not be shared and all made
it into the .rodata section.

Start using ExceptionMessages::FailedToConstruct() in the error messages.
Not only do we use the existing error infrastructure, but it allows us to
separate the part that always changes (the callback or interface name) from
the actual details (the "Unable to convert value" bit).

With commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 ("Process TeamDrive
change in change list") as a baseline, the following data can be observed
with //tools/binary_size/map2size.py and console.py:

* Reverting the changes from https://codereview.chromium.org/2802223002
  versus this commit:

  Section Sizes (Total=-546 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: 0 bytes (-0.0%)
    .text: -546 bytes (100.0%)
  2 symbols added (+), 37 changed (~), 5 removed (-), 313766 unchanged (not
shown)

  Showing 44 symbols with total size: -560 bytes
  .text=-560 bytes .rodata=0 bytes    other=0 bytes    total=-560 bytes
  Number of object files: 36

* Commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 versus this commit:

  Section Sizes (Total=-33,826 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: -33,280 bytes (98.4%)
    .text: -546 bytes (1.6%)

  2 symbols added (+), 37 changed (~), 5 removed (-), 313766 unchanged (not
shown)

  Showing 44 symbols with total size: -33840 bytes
  .text=-560 bytes .rodata=-32.5kb    other=0 bytes    total=-33.0kb
  Number of object files: 36

BUG=690428,710035
R=agrieve@chromium.org,bashi@chromium.org,haraken@chromium.org,jbroman@chromi...
==========

to

==========
bindings: Use ExceptionMessages when an interface/callback conversion fails.

https://codereview.chromium.org/2802223002 ("bindings: Throw a type error if
converting an interface or callback function fails") introduced a 32kb size
regression on release Android builds (is_official_build=true), as the new
error strings being thrown as type errors could not be shared and all made
it into the .rodata section.

Start using ExceptionMessages::FailedToConstruct() in the error messages.
Not only do we use the existing error infrastructure, but it allows us to
separate the part that always changes (the callback or interface name) from
the actual details (the "Unable to convert value" bit).

With commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 ("Process TeamDrive
change in change list") as a baseline, the following data can be observed
with //tools/binary_size/map2size.py and console.py:

* Reverting the changes from https://codereview.chromium.org/2802223002
  versus this commit:

  Section Sizes (Total=-546 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: 0 bytes (-0.0%)
    .text: -546 bytes (100.0%)
  2 symbols added (+), 37 changed (~), 5 removed (-), 313766 unchanged (not
shown)

  Showing 44 symbols with total size: -560 bytes
  .text=-560 bytes .rodata=0 bytes    other=0 bytes    total=-560 bytes
  Number of object files: 36

* Commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 versus this commit:

  Section Sizes (Total=-33,826 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: -33,280 bytes (98.4%)
    .text: -546 bytes (1.6%)
  2 symbols added (+), 37 changed (~), 5 removed (-), 313766 unchanged (not
shown)

  Showing 44 symbols with total size: -33840 bytes
  .text=-560 bytes .rodata=-32.5kb    other=0 bytes    total=-33.0kb
  Number of object files: 36

BUG=690428,710035
R=agrieve@chromium.org,bashi@chromium.org,haraken@chromium.org,jbroman@chromi...
==========

[Raphael Kubo da Costa (rakuco), 2017-04-11 13:36:24]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-11 13:36:36]

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

[Raphael Kubo da Costa (rakuco), 2017-04-11 13:46:59]

The CQ bit was unchecked by raphael.kubo.da.costa@intel.com

[Yuki, 2017-04-11 14:01:17]

LGTM.

[haraken, 2017-04-11 14:03:01]

LGTM

[Raphael Kubo da Costa (rakuco), 2017-04-11 14:05:31]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-11 14:05:48]

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

[agrieve, 2017-04-11 14:26:44]

On 2017/04/11 14:03:01, haraken wrote:
> LGTM

lgtm thanks!

[jbroman, 2017-04-11 14:34:31]

Approach looks good to me in general, but I'm not sure FailedToConstruct is a
clear message (from the author's perspective, they weren't necessarily calling a
constructor, right)? An alternative might be to add a more descriptive one to
ExceptionMessages.

String ExceptionMessages::FailedToConvert(const char* type) {
  return String::Format("Failed to convert value to '%s'.", type);
}

[commit-bot: I haz the power, 2017-04-11 15:46:51]

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

[commit-bot: I haz the power, 2017-04-11 15:46:52]

Dry run: This issue passed the CQ dry run.

[Raphael Kubo da Costa (rakuco), 2017-04-11 16:04:14]

On 2017/04/11 14:34:31, jbroman wrote:
> Approach looks good to me in general, but I'm not sure FailedToConstruct is a
> clear message (from the author's perspective, they weren't necessarily calling
a
> constructor, right)? An alternative might be to add a more descriptive one to
> ExceptionMessages.
> 
> String ExceptionMessages::FailedToConvert(const char* type) {
>   return String::Format("Failed to convert value to '%s'.", type);
> }

To be honest I thought the same thing while writing the patch, but decided to
piggyback on FailedToConstruct because the error message looked close enough
(and it saves ~40 bytes). I'll upload a new version with a new exception
message.

[Raphael Kubo da Costa (rakuco), 2017-04-11 16:25:37]

Description was changed from

==========
bindings: Use ExceptionMessages when an interface/callback conversion fails.

https://codereview.chromium.org/2802223002 ("bindings: Throw a type error if
converting an interface or callback function fails") introduced a 32kb size
regression on release Android builds (is_official_build=true), as the new
error strings being thrown as type errors could not be shared and all made
it into the .rodata section.

Start using ExceptionMessages::FailedToConstruct() in the error messages.
Not only do we use the existing error infrastructure, but it allows us to
separate the part that always changes (the callback or interface name) from
the actual details (the "Unable to convert value" bit).

With commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 ("Process TeamDrive
change in change list") as a baseline, the following data can be observed
with //tools/binary_size/map2size.py and console.py:

* Reverting the changes from https://codereview.chromium.org/2802223002
  versus this commit:

  Section Sizes (Total=-546 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: 0 bytes (-0.0%)
    .text: -546 bytes (100.0%)
  2 symbols added (+), 37 changed (~), 5 removed (-), 313766 unchanged (not
shown)

  Showing 44 symbols with total size: -560 bytes
  .text=-560 bytes .rodata=0 bytes    other=0 bytes    total=-560 bytes
  Number of object files: 36

* Commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 versus this commit:

  Section Sizes (Total=-33,826 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: -33,280 bytes (98.4%)
    .text: -546 bytes (1.6%)
  2 symbols added (+), 37 changed (~), 5 removed (-), 313766 unchanged (not
shown)

  Showing 44 symbols with total size: -33840 bytes
  .text=-560 bytes .rodata=-32.5kb    other=0 bytes    total=-33.0kb
  Number of object files: 36

BUG=690428,710035
R=agrieve@chromium.org,bashi@chromium.org,haraken@chromium.org,jbroman@chromi...
==========

to

==========
bindings: Use ExceptionMessages when an interface/callback conversion fails.

https://codereview.chromium.org/2802223002 ("bindings: Throw a type error if
converting an interface or callback function fails") introduced a 32kb size
regression on release Android builds (is_official_build=true), as the new
error strings being thrown as type errors could not be shared and all made
it into the .rodata section.

Start using ExceptionMessages::FailedToConstruct() in the error messages.
Not only do we use the existing error infrastructure, but it allows us to
separate the part that always changes (the callback or interface name) from
the actual details (the "Unable to convert value" bit).

With commit 07c092298a6243fc2c829e517cf2b19f4caea26c ("Fetch: Make Headers'
constructor match the current spec IDL") as a baseline, the following data
can be observed with //tools/binary_size/map2size.py and console.py:

* Reverting the changes from https://codereview.chromium.org/2802223002
  versus this commit:

  Section Sizes (Total=-168 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: 256 bytes (-152.4%)
    .text: -424 bytes (252.4%)
  0 symbols added (+), 27 changed (~), 3 removed (-), 313837 unchanged (not
shown)

  Showing 30 symbols with total size: -160 bytes
  .text=-416 bytes .rodata=256 bytes  other=0 bytes    total=-160 bytes
  Number of object files: 28

* Commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 versus this commit:

  Section Sizes (Total=-33,704 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: -33,280 bytes (98.7%)
    .text: -424 bytes (1.3%)
  0 symbols added (+), 27 changed (~), 3 removed (-), 313837 unchanged (not
shown)

  Showing 30 symbols with total size: -33696 bytes
  .text=-416 bytes .rodata=-32.5kb    other=0 bytes    total=-32.9kb
  Number of object files: 28

BUG=690428,710035
R=agrieve@chromium.org,bashi@chromium.org,haraken@chromium.org,jbroman@chromi...
==========

[Raphael Kubo da Costa (rakuco), 2017-04-11 16:26:13]

Patch v3 is up with a new ExceptionMessage method. I've also updated the
measurements in the CL description with a different commit as the baseline.

Please take another look.

[Raphael Kubo da Costa (rakuco), 2017-04-11 16:26:22]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-11 16:26:59]

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

[jbroman, 2017-04-11 17:07:24]

lgtm

[commit-bot: I haz the power, 2017-04-11 19:11:31]

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

[commit-bot: I haz the power, 2017-04-11 19:11:32]

Dry run: This issue passed the CQ dry run.

[Raphael Kubo da Costa (rakuco), 2017-04-11 20:04:19]

The CQ bit was checked by raphael.kubo.da.costa@intel.com

[Raphael Kubo da Costa (rakuco), 2017-04-11 20:04:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, yukishiino@chromium.org, agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2809003004/#ps40001
(title: "Add ExceptionMessages::FailedToConvertJSValue")

[commit-bot: I haz the power, 2017-04-11 20:05: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, 2017-04-11 20:36:47]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491941059046150,
"parent_rev": "022037f0d183a9d602f7f42accfd4ebf6af80e23", "commit_rev":
"81c4e4ab9d81757ce3dcb49185be46c880f3f22a"}

[commit-bot: I haz the power, 2017-04-11 20:37:36]

Description was changed from

==========
bindings: Use ExceptionMessages when an interface/callback conversion fails.

https://codereview.chromium.org/2802223002 ("bindings: Throw a type error if
converting an interface or callback function fails") introduced a 32kb size
regression on release Android builds (is_official_build=true), as the new
error strings being thrown as type errors could not be shared and all made
it into the .rodata section.

Start using ExceptionMessages::FailedToConstruct() in the error messages.
Not only do we use the existing error infrastructure, but it allows us to
separate the part that always changes (the callback or interface name) from
the actual details (the "Unable to convert value" bit).

With commit 07c092298a6243fc2c829e517cf2b19f4caea26c ("Fetch: Make Headers'
constructor match the current spec IDL") as a baseline, the following data
can be observed with //tools/binary_size/map2size.py and console.py:

* Reverting the changes from https://codereview.chromium.org/2802223002
  versus this commit:

  Section Sizes (Total=-168 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: 256 bytes (-152.4%)
    .text: -424 bytes (252.4%)
  0 symbols added (+), 27 changed (~), 3 removed (-), 313837 unchanged (not
shown)

  Showing 30 symbols with total size: -160 bytes
  .text=-416 bytes .rodata=256 bytes  other=0 bytes    total=-160 bytes
  Number of object files: 28

* Commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 versus this commit:

  Section Sizes (Total=-33,704 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: -33,280 bytes (98.7%)
    .text: -424 bytes (1.3%)
  0 symbols added (+), 27 changed (~), 3 removed (-), 313837 unchanged (not
shown)

  Showing 30 symbols with total size: -33696 bytes
  .text=-416 bytes .rodata=-32.5kb    other=0 bytes    total=-32.9kb
  Number of object files: 28

BUG=690428,710035
R=agrieve@chromium.org,bashi@chromium.org,haraken@chromium.org,jbroman@chromi...
==========

to

==========
bindings: Use ExceptionMessages when an interface/callback conversion fails.

https://codereview.chromium.org/2802223002 ("bindings: Throw a type error if
converting an interface or callback function fails") introduced a 32kb size
regression on release Android builds (is_official_build=true), as the new
error strings being thrown as type errors could not be shared and all made
it into the .rodata section.

Start using ExceptionMessages::FailedToConstruct() in the error messages.
Not only do we use the existing error infrastructure, but it allows us to
separate the part that always changes (the callback or interface name) from
the actual details (the "Unable to convert value" bit).

With commit 07c092298a6243fc2c829e517cf2b19f4caea26c ("Fetch: Make Headers'
constructor match the current spec IDL") as a baseline, the following data
can be observed with //tools/binary_size/map2size.py and console.py:

* Reverting the changes from https://codereview.chromium.org/2802223002
  versus this commit:

  Section Sizes (Total=-168 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: 256 bytes (-152.4%)
    .text: -424 bytes (252.4%)
  0 symbols added (+), 27 changed (~), 3 removed (-), 313837 unchanged (not
shown)

  Showing 30 symbols with total size: -160 bytes
  .text=-416 bytes .rodata=256 bytes  other=0 bytes    total=-160 bytes
  Number of object files: 28

* Commit b1bbca356abfa6f8fdec3cd5815f18b164491a45 versus this commit:

  Section Sizes (Total=-33,704 bytes):
    .bss: 0 bytes (not included in totals)
    .data: 0 bytes (-0.0%)
    .data.rel.ro: 0 bytes (-0.0%)
    .data.rel.ro.local: 0 bytes (-0.0%)
    .rodata: -33,280 bytes (98.7%)
    .text: -424 bytes (1.3%)
  0 symbols added (+), 27 changed (~), 3 removed (-), 313837 unchanged (not
shown)

  Showing 30 symbols with total size: -33696 bytes
  .text=-416 bytes .rodata=-32.5kb    other=0 bytes    total=-32.9kb
  Number of object files: 28

BUG=690428,710035
R=agrieve@chromium.org,bashi@chromium.org,haraken@chromium.org,jbroman@chromi...

Review-Url: https://codereview.chromium.org/2809003004
Cr-Commit-Position: refs/heads/master@{#463753}
Committed:
https://chromium.googlesource.com/chromium/src/+/81c4e4ab9d81757ce3dcb49185be...
==========

[commit-bot: I haz the power, 2017-04-11 20:37:42]

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