Move Error methods to C++

Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Committed: https://crrev.com/9211dee01abc9e8dee16cb0915fc9673132823af
Cr-Original-Commit-Position: refs/heads/master@{#37870}
Cr-Commit-Position: refs/heads/master@{#37908}

[jgruber, 2016-07-12 15:20:18]

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

[commit-bot: I haz the power, 2016-07-12 15:20:27]

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-07-12 15:42:12]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 15:42:13]

Dry run: Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng_trigg...)

[jgruber, 2016-07-13 12:10:31]

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

[commit-bot: I haz the power, 2016-07-13 12:10:37]

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

[jgruber, 2016-07-13 12:17:43]

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

[commit-bot: I haz the power, 2016-07-13 12:17:47]

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-07-13 12:35:13]

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

[commit-bot: I haz the power, 2016-07-13 12:35:14]

Dry run: Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng_trigg...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/8967)
  v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)

[jgruber, 2016-07-13 13:17:21]

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

[commit-bot: I haz the power, 2016-07-13 13:17:24]

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-07-13 13:37:35]

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

[commit-bot: I haz the power, 2016-07-13 13:37:35]

Dry run: Try jobs failed on following builders:
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_arm_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng_trigger...)

[jgruber, 2016-07-13 13:57:15]

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

[commit-bot: I haz the power, 2016-07-13 13:57:28]

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-07-13 14:35:05]

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

[commit-bot: I haz the power, 2016-07-13 14:35:06]

Dry run: Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng_trigg...)

[jgruber, 2016-07-14 14:01:45]

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

[commit-bot: I haz the power, 2016-07-14 14:02:21]

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

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

[commit-bot: I haz the power, 2016-07-14 14:03:51]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)

[jgruber, 2016-07-14 14:11:47]

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

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

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-07-14 14:45:18]

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

[commit-bot: I haz the power, 2016-07-14 14:45:20]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-07-14 14:49:46]

Description was changed from

==========
Initial work on port of error constructor to C++

This ports the %SetCode portion of error setup to C++, including the stack
setter and getter, and Error.prototype.toString.

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

to

==========
Move Error methods to C++

This ports a large portion of Error methods to C++, 
including the constructor, stack setter and getter, 
and Error.prototype.toString.

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

[jgruber, 2016-07-14 14:49:46]

jgruber@chromium.org changed reviewers:
+ yangguo@chromium.org

[Yang, 2016-07-15 13:10:11]

https://codereview.chromium.org/2142933003/diff/120001/src/accessors.cc
File src/accessors.cc (right):

https://codereview.chromium.org/2142933003/diff/120001/src/accessors.cc#newco...
src/accessors.cc:1145: error,
isolate->factory()->formatted_stack_trace_symbol(),
How about we simply reconfigure the "stack" property to a normal data property?
For the user there is no difference between an accessor property that's marked
as special data property, and a normal data property. So instead of caching it
in a hidden symbol, we could just overwrite the stack property with a normal
data property containing the lazily formatted stack trace.

https://codereview.chromium.org/2142933003/diff/120001/src/accessors.cc#newco...
src/accessors.cc:1169: bool MaybeGetProperty(Isolate* isolate, Handle<JSObject>
obj,
We can inline this once this only has one call site.

https://codereview.chromium.org/2142933003/diff/120001/src/accessors.cc#newco...
src/accessors.cc:1230: if (IsErrorObject(isolate, obj)) {
I would just do this check inline (instead of defining what an Error object is).

https://codereview.chromium.org/2142933003/diff/120001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2142933003/diff/120001/src/bootstrapper.cc#ne...
src/bootstrapper.cc:1159: Handle<JSFunction> globalError =
isolate->error_function();
let's name this "global_error" to be consistent with the rest.

https://codereview.chromium.org/2142933003/diff/120001/src/builtins/builtins.cc
File src/builtins/builtins.cc (right):

https://codereview.chromium.org/2142933003/diff/120001/src/builtins/builtins....
src/builtins/builtins.cc:4727: namespace {
new line above this namespace

[jgruber, 2016-07-17 16:20:59]

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

[commit-bot: I haz the power, 2016-07-17 16:21:08]

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-07-17 16:46:21]

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

[commit-bot: I haz the power, 2016-07-17 16:46:23]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/9175)
  v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)

[jgruber, 2016-07-18 12:36:43]

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

[commit-bot: I haz the power, 2016-07-18 12:36:46]

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-07-18 12:59:57]

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

[commit-bot: I haz the power, 2016-07-18 12:59:59]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-07-18 13:06:11]

https://codereview.chromium.org/2142933003/diff/120001/src/accessors.cc
File src/accessors.cc (right):

https://codereview.chromium.org/2142933003/diff/120001/src/accessors.cc#newco...
src/accessors.cc:1145: error,
isolate->factory()->formatted_stack_trace_symbol(),
On 2016/07/15 13:10:11, Yang wrote:
> How about we simply reconfigure the "stack" property to a normal data
property?
> For the user there is no difference between an accessor property that's marked
> as special data property, and a normal data property. So instead of caching it
> in a hidden symbol, we could just overwrite the stack property with a normal
> data property containing the lazily formatted stack trace.

Good idea, done.

https://codereview.chromium.org/2142933003/diff/120001/src/accessors.cc#newco...
src/accessors.cc:1169: bool MaybeGetProperty(Isolate* isolate, Handle<JSObject>
obj,
On 2016/07/15 13:10:11, Yang wrote:
> We can inline this once this only has one call site.

Done.

https://codereview.chromium.org/2142933003/diff/120001/src/accessors.cc#newco...
src/accessors.cc:1230: if (IsErrorObject(isolate, obj)) {
On 2016/07/15 13:10:11, Yang wrote:
> I would just do this check inline (instead of defining what an Error object
is).

Done.

https://codereview.chromium.org/2142933003/diff/120001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2142933003/diff/120001/src/bootstrapper.cc#ne...
src/bootstrapper.cc:1159: Handle<JSFunction> globalError =
isolate->error_function();
On 2016/07/15 13:10:11, Yang wrote:
> let's name this "global_error" to be consistent with the rest.

Done.

https://codereview.chromium.org/2142933003/diff/120001/src/builtins/builtins.cc
File src/builtins/builtins.cc (right):

https://codereview.chromium.org/2142933003/diff/120001/src/builtins/builtins....
src/builtins/builtins.cc:4727: namespace {
On 2016/07/15 13:10:11, Yang wrote:
> new line above this namespace

Done.

[jgruber, 2016-07-19 11:02:42]

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

[commit-bot: I haz the power, 2016-07-19 11:02:47]

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

[jgruber, 2016-07-19 11:03:01]

And moved to builtins-error.cc, PTAL.

[Yang, 2016-07-19 11:34:59]

lgtm once comment is addressed.

https://codereview.chromium.org/2142933003/diff/180001/src/accessors.cc
File src/accessors.cc (right):

https://codereview.chromium.org/2142933003/diff/180001/src/accessors.cc#newco...
src/accessors.cc:1172: // The structured stack trace is not available. Look
further up the
Can this actually happen? If this is some obscure edge case, we could just
return undefined.

The Javascript code had to do this because "this" is the receiver, not
necessarily the holder. So we had to manually walk the prototype chain to find
the actual holder. Here we do have the holder.

[commit-bot: I haz the power, 2016-07-19 11:37:59]

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

[commit-bot: I haz the power, 2016-07-19 11:38:00]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-07-19 13:09:02]

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

[commit-bot: I haz the power, 2016-07-19 13:09:06]

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

[jgruber, 2016-07-19 13:21:50]

https://codereview.chromium.org/2142933003/diff/180001/src/accessors.cc
File src/accessors.cc (right):

https://codereview.chromium.org/2142933003/diff/180001/src/accessors.cc#newco...
src/accessors.cc:1172: // The structured stack trace is not available. Look
further up the
On 2016/07/19 11:34:59, Yang wrote:
> Can this actually happen? If this is some obscure edge case, we could just
> return undefined.
> 
> The Javascript code had to do this because "this" is the receiver, not
> necessarily the holder. So we had to manually walk the prototype chain to find
> the actual holder. Here we do have the holder.

Done.

[commit-bot: I haz the power, 2016-07-19 13:40:21]

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

[commit-bot: I haz the power, 2016-07-19 13:40:22]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-07-19 13:43:06]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-07-19 13:43:06]

The patchset sent to the CQ was uploaded after l-g-t-m from yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2142933003/#ps200001
(title: "Address comments")

[commit-bot: I haz the power, 2016-07-19 13:43:13]

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-07-19 13:49:55]

Description was changed from

==========
Move Error methods to C++

This ports a large portion of Error methods to C++, 
including the constructor, stack setter and getter, 
and Error.prototype.toString.

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

to

==========
Move Error methods to C++

This ports a large portion of Error methods to C++, 
including the constructor, stack setter and getter, 
and Error.prototype.toString.

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

[commit-bot: I haz the power, 2016-07-19 13:49:56]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-07-19 13:51:16]

Description was changed from

==========
Move Error methods to C++

This ports a large portion of Error methods to C++, 
including the constructor, stack setter and getter, 
and Error.prototype.toString.

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

to

==========
Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Cr-Commit-Position: refs/heads/master@{#37870}
==========

[commit-bot: I haz the power, 2016-07-19 13:51:18]

Patchset 11 (id:??) landed as
https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Cr-Commit-Position: refs/heads/master@{#37870}

[Benedikt Meurer, 2016-07-19 17:23:35]

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

[Benedikt Meurer, 2016-07-19 17:23:35]

Awesome, Jakob. Thanks for doing.

[jgruber, 2016-07-20 06:41:28]

A revert of this CL (patchset #11 id:200001) has been created in
https://codereview.chromium.org/2159223004/ by jgruber@chromium.org.

The reason for reverting is: Clusterfuzz failures:
https://bugs.chromium.org/p/chromium/issues/detail?id=629749.

[jgruber, 2016-07-20 11:15:19]

Description was changed from

==========
Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Cr-Commit-Position: refs/heads/master@{#37870}
==========

to

==========
Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Cr-Commit-Position: refs/heads/master@{#37870}
==========

[jgruber, 2016-07-20 11:16:01]

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

[commit-bot: I haz the power, 2016-07-20 11:16:11]

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

[jgruber, 2016-07-20 11:46:07]

Fixed two issues exposed by clusterfuzz:

* The first was triggered by calling Error.captureStackTrace() on an existing
error, and led to failures when we tried to add "stack" property accessors to
the map.
* The second was triggered by accessing the "stack" property recursively. In
this case, we'd first reconfigure to a data property, and then attempt to do the
same thing again and fail.

Used a quick and dirty fix for the first since this is temporary code. It will
be replaced once we have eager stack trace formatting for
Error.captureStackTrace.

The second was fixed by checking whether "stack" is still an accessor before
trying to reconfigure to a data property.

PTAL.

[Yang, 2016-07-20 12:07:51]

https://codereview.chromium.org/2142933003/diff/240001/src/accessors.cc
File src/accessors.cc (right):

https://codereview.chromium.org/2142933003/diff/240001/src/accessors.cc#newco...
src/accessors.cc:1218: if (!JSObject::GetProperty(holder,
name).ToHandle(&formatted_stack_trace)) {
In this case we would have an exception. We should call
OptionalRescheduleException and return.

[jgruber, 2016-07-20 12:13:59]

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

[jgruber, 2016-07-20 12:14:11]

https://codereview.chromium.org/2142933003/diff/240001/src/accessors.cc
File src/accessors.cc (right):

https://codereview.chromium.org/2142933003/diff/240001/src/accessors.cc#newco...
src/accessors.cc:1218: if (!JSObject::GetProperty(holder,
name).ToHandle(&formatted_stack_trace)) {
On 2016/07/20 12:07:51, Yang wrote:
> In this case we would have an exception. We should call
> OptionalRescheduleException and return.

Done.

[commit-bot: I haz the power, 2016-07-20 12:14:12]

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

[Yang, 2016-07-20 12:29:56]

On 2016/07/20 12:14:11, jgruber wrote:
> https://codereview.chromium.org/2142933003/diff/240001/src/accessors.cc
> File src/accessors.cc (right):
> 
>
https://codereview.chromium.org/2142933003/diff/240001/src/accessors.cc#newco...
> src/accessors.cc:1218: if (!JSObject::GetProperty(holder,
> name).ToHandle(&formatted_stack_trace)) {
> On 2016/07/20 12:07:51, Yang wrote:
> > In this case we would have an exception. We should call
> > OptionalRescheduleException and return.
> 
> Done.

lgtm.

[commit-bot: I haz the power, 2016-07-20 12:51:07]

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

[commit-bot: I haz the power, 2016-07-20 12:51:09]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-07-20 12:52:25]

The CQ bit was checked by jgruber@chromium.org

[commit-bot: I haz the power, 2016-07-20 12:52:40]

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-07-20 13:02:44]

Description was changed from

==========
Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Cr-Commit-Position: refs/heads/master@{#37870}
==========

to

==========
Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Cr-Commit-Position: refs/heads/master@{#37870}
==========

[commit-bot: I haz the power, 2016-07-20 13:02:46]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2016-07-20 13:05:56]

Description was changed from

==========
Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Cr-Commit-Position: refs/heads/master@{#37870}
==========

to

==========
Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Committed: https://crrev.com/9211dee01abc9e8dee16cb0915fc9673132823af
Cr-Original-Commit-Position: refs/heads/master@{#37870}
Cr-Commit-Position: refs/heads/master@{#37908}
==========

[commit-bot: I haz the power, 2016-07-20 13:05:57]

Patchset 14 (id:??) landed as
https://crrev.com/9211dee01abc9e8dee16cb0915fc9673132823af
Cr-Commit-Position: refs/heads/master@{#37908}

[jgruber, 2016-07-21 08:56:32]

A revert of this CL (patchset #14 id:260001) has been created in
https://codereview.chromium.org/2165633005/ by jgruber@chromium.org.

The reason for reverting is: Clusterfuzz failures:

https://bugs.chromium.org/p/chromium/issues/detail?id=630145#c1
https://bugs.chromium.org/p/chromium/issues/detail?id=630144#c1.

[jgruber, 2016-07-21 08:56:58]

Description was changed from

==========
Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Committed: https://crrev.com/9211dee01abc9e8dee16cb0915fc9673132823af
Cr-Original-Commit-Position: refs/heads/master@{#37870}
Cr-Commit-Position: refs/heads/master@{#37908}
==========

to

==========
Move Error methods to C++

This ports a large portion of Error methods to C++,
including the constructor, stack setter and getter,
and Error.prototype.toString.

BUG=

Committed: https://crrev.com/5742da056a290caa13a0b8717ddb1e43424e0d31
Committed: https://crrev.com/9211dee01abc9e8dee16cb0915fc9673132823af
Cr-Original-Commit-Position: refs/heads/master@{#37870}
Cr-Commit-Position: refs/heads/master@{#37908}
==========