The parser eagerly rewrites destructuring assignments occuring
in formal parameter initializers, because not doing so would
cause the BindingPattern rewriting to be confused and do the
wrong thing.
This change prevents this rewriting from descending into the
bodies of lazily parsed functions.
In general, it's a mistake to descend into the bodies of function
literals anyways, since they are rewritten separately on their
own time, so there is no distinction made between lazily
"throw away" eagerly parsed functions in the temporary parser
arena, or "real" eagerly parsed functions that will be compiled.
BUG=chromium:594084, v8:811
LOG=N
R=adamk@chromium.org, littledan@chromium.org
Review URL: https://codereview.chromium.org/1864553002
Cr-Commit-Position: refs/heads/master@{#35277}
Previously, CreateDataProperty would fail a DCHECK when used to create
an integer indexed property on a TypedArray. This patch makes it throw
a TypeError instead. The issue came up when Array.prototype.concat
was repaired to use CreateDataProperty rather than SetElement; concat
can be tricked into making a new TypedArray if it is given an Array
whose prototype is a TypedArray. This patch prevents the issue.
R=adamk
LOG=Y
BUG=chromium:596394
Review URL: https://codereview.chromium.org/1821723004
Cr-Commit-Position: refs/heads/master@{#35271}
In simplified numbering, we make sanity checks based on types (e.g.,
NumberSubtract should take numbers as inputs), but this can be
violated if optimization passes make types less precise.
In this CL, we fix load elimination to make sure that types are
smaller in the store -> load elimination by taking an intersection
of the load's type with the store value's type and inserting a guard
with that type. Note that the load type comes from type feedback, so
it can be disjoint from the stored value type (in that case, this
must be dead code because the map chack for the load should prevent
us from using the stored value).
BUG=chromium:599412
LOG=n
Review URL: https://codereview.chromium.org/1857133003
Cr-Commit-Position: refs/heads/master@{#35259}
If we use ScopeIterator inside a debug-evaluate call, we may iterate
over a debug-evaluate context that we created for the debug-evaluate
call. This may trigger assertions.
The solution is to have the ScopeIterator hide debug-evaluate contexts
by unwrapping it if it comes across any.
R=cbruni@chromium.org
BUG=chromium:599662
LOG=N
Review URL: https://codereview.chromium.org/1859033002
Cr-Commit-Position: refs/heads/master@{#35258}
There were a couple of issues with it:
- interpreter is not supported
- the source position was just accidentally correct for full-codegen
- the eval origin could have been cached
Also fixes a few other places to use AbstractCode.
R=mstarzinger@chromium.org
Review URL: https://codereview.chromium.org/1854713002
Cr-Commit-Position: refs/heads/master@{#35257}
When seeing a rest pattern, we used to get the remaining elements from the
iterator by calling %concat_iterable_to_array on it. This was wrong because it
caused an observable [[Get]] for @@iterator (which the iterator may not even
provide).
This CL gets rid of the call to %concat_iterable_to_array and does the iteration
manually in a simple while-loop. It also gets rid of %concat_iterable_to_array
itself because there aren't any other uses of it.
BUG=v8:4759
LOG=n
R=adamk@chromium.org
Review URL: https://codereview.chromium.org/1852703002
Cr-Commit-Position: refs/heads/master@{#35251}
This fixes a corner case where the generator function of a suspended
generator has been marked for optimization. We assume the optimization
approach will cause a bailout because generators are not optimized. But
resuming is more resilient by always activating the unoptimized code.
R=neis@chromium.org,bmeurer@chromium.org
TEST=mjsunit/regress/regress-crbug-513471
BUG=chromium:513471
LOG=n
Review URL: https://codereview.chromium.org/1856683002
Cr-Commit-Position: refs/heads/master@{#35234}
- RegExp.prototype.toString() doesn't have any special handling of
RegExp instances and simply calls the source and flags getters
- Use the original values of global and sticky, rather than based
on the current flag getters, as specified in
https://github.com/tc39/ecma262/pull/494R=yangguo@chromium.org,adamk
LOG=Y
BUG=v8:4602
Review URL: https://codereview.chromium.org/1846303002
Cr-Commit-Position: refs/heads/master@{#35225}
This allows us to remove the troublesome %_MathClz32 intrinsic and also
allows us to utilize the functionality that is already available in
TurboFan. Also introduce a proper NumberClz32 operator so we don't need
to introduce a machine operator at the JS level.
R=epertoso@chromium.org
Review URL: https://codereview.chromium.org/1852553003
Cr-Commit-Position: refs/heads/master@{#35208}
Handles bytecodeArray Objects when verifying the heap and also when
collecting code statistics. The changes include:
1. BytecodeArrays could be a part of the large object space. When
verifying the large object space we should also allow BytecodeArray
objects.
2. Adds support for BytecodeArrays when collecting code statistics.
BUG=v8:4280,chromium:599001
LOG=N
Review URL: https://codereview.chromium.org/1850443006
Cr-Commit-Position: refs/heads/master@{#35202}
Reason for revert:
breaks some chromium browser_tests: https://codereview.chromium.org/1848233002/
Original issue's description:
> Ship --harmony-regexp-exec
>
> There are still spec compliance fixes to be made, but this patch
> turns the flag to shipping to make sure we get more canary coverage
> and performance data from the bots.
>
> BUG=v8:4602
> LOG=y
>
> Committed: https://crrev.com/84492bb66b340f4e0df36758e98fddbb10b5d1dc
> Cr-Commit-Position: refs/heads/master@{#35181}
TBR=littledan@chromium.org,adamk@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4602
Review URL: https://codereview.chromium.org/1852673003
Cr-Commit-Position: refs/heads/master@{#35198}
This CL ensures that we build environments/frame states so that tail caller frame will never become topmost.
BUG=chromium:598998, v8:4698
LOG=N
Review URL: https://codereview.chromium.org/1849503002
Cr-Commit-Position: refs/heads/master@{#35188}
The HandlerCompiler did not properly handle the weird edge case when a
sloppy mode function was installed as an accessor on one of the value
wrapper prototypes and then accessed via a load from a primitive value.
In this case we just passed the primitive value untouched instead of
properly wrapping it first. The CallFunction builtin properly deals with
all the funny edge cases, so we use it instead of duplicating almost all
of the logic here (the performance difference is neglible).
R=verwaest@chromium.org
BUG=chromium:599073, v8:4413
LOG=n
Review URL: https://codereview.chromium.org/1845243005
Cr-Commit-Position: refs/heads/master@{#35187}
There are still spec compliance fixes to be made, but this patch
turns the flag to shipping to make sure we get more canary coverage
and performance data from the bots.
BUG=v8:4602
LOG=y
Review URL: https://codereview.chromium.org/1847103002
Cr-Commit-Position: refs/heads/master@{#35181}
Reason for revert:
TC39 decided that this compatibility fix should be standardized.
Original issue's description:
> Remove RegExp.prototype.source getter compat workaround
>
> The getter RegExp.prototype.source is specified in ES2015 to throw when
> called on a non-RegExp instance, such as RegExp.prototype. We had previously
> put in a compatibility workaround for all RegExp getters to make them
> throw on access specifically with RegExp.prototype as the receiver; however,
> we only have evidence that this is needed for properties other than source.
> This patch removes the compatibility workaround for get RegExp.prototype.source
> and gives it semantics precisely as per the ES2015 specification.
>
> R=adamk
> BUG=chromium:581577,v8:4827
> LOG=Y
>
> Committed: https://crrev.com/80803aa89e31839b8f73959776fa7e1923c6b461
> Cr-Commit-Position: refs/heads/master@{#35086}
R=adamk@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=chromium:581577,v8:4827
LOG=Y
Review URL: https://codereview.chromium.org/1847783003
Cr-Commit-Position: refs/heads/master@{#35180}
In the earlier implementation of GenerateDoubleToObject the context
is loaded from the parent's frame. rsi is clobbered because it is used
to store kHoleNan constnat. It is not always safe to peek at
the parents frame. Bytecode handlers have TypedFrame and the type of
frame is stored at FP + 1. GenerateDoubleToObject expects context
to be store at that place. In the current implementation rsi is pushed
onto the stack and is popped when exiting this function.
BUG=v8:4280,chromium:597565
LOG=N
Review URL: https://codereview.chromium.org/1848473002
Cr-Commit-Position: refs/heads/master@{#35163}
Migrate Math.ceil, Math.round and Math.trunc to TurboFan code stubs,
similar to what we did with Math.floor, and make these builtins properly
optimizable in TurboFan via appropriate simplified operators NumberCeil,
NumberRound and NumberTrunc, which are intended to be reusable for
ToInteger and ToLength optimizations that will be done in a followup CL.
Also allows us to kill the funky %RoundNumber runtime function, which
was quite heavy.
Improve test coverage for Math.ceil and Math.trunc a lot, especially
making sure that we also properly trigger the TurboFan builtin reducer
case.
R=jarin@chromium.org
BUG=v8:4059
LOG=n
Review URL: https://codereview.chromium.org/1841993002
Cr-Commit-Position: refs/heads/master@{#35135}
Debugger fetches the return value of a function when we break at return.
Interpreter holds the return value in accumulator. This is not stored in a
specified location on stack and hence it is not possible to look it up from
stack similar to full-codegen or optimized frames. This cl adds support to
store the value of accumulator on debug breaks. The value of accumulator is
passed to the runtime function and is then stored in thread local data.
Also changes full-codegen implementation to match that of ignition.
The return value from full-codegen is also stored in thread local data.
The return value is fetched directly thread local data instead of
finding it by iterating over frames.
BUG=v8:4280, v8:4690
LOG=N
Review URL: https://codereview.chromium.org/1842683002
Cr-Commit-Position: refs/heads/master@{#35127}
This fixes another bug in Array.prototype.sort (when the array is not a
JSArray and there is a proxy on the prototype chain).
R=cbruni@chromium.org
BUG=chromium:596866
LOG=n
Review URL: https://codereview.chromium.org/1842563004
Cr-Commit-Position: refs/heads/master@{#35101}
In JavaScript code and stubs, JSSP mirrors the CSP but may be unaligned.
But in WASM code only CSP is used, like native code, and it must be
aligned.
Calls into WASM from JS need to carefully align the C stack
pointer (csp) and restore the previous JSSP, while calls from WASM
to JS need to compute a new JSSP and restore their CSP after the
call.
R=ahaas@chromium.org
BUG=
Review URL: https://codereview.chromium.org/1811283003
Cr-Commit-Position: refs/heads/master@{#35096}
Fix and re-enable the flexible representation for Math.floor (which is used to
implement Math.ceil) and Math.round, which allows Math.floor and Math.round to
return double results instead of int32, and therefore allows values outside
the int32 range, especially -0 is now a valid result, which doesn't deopt.
Also port this feature to x64 and ia32 when the CPU supports the SSE4.1
extension.
This addresses all the known deoptimization loops related to Math.round
in the Kraken benchmark suite, and seems to also address most of the
deoptimization loops related to Math.floor in the Oort Online benchmark.
Drive-by-fix: Import the regression tests for the broken HMathFloorOfDiv
optimization that caused the initial revert of the feature (for arm64 only
back then).
BUG=chromium:476477,v8:2890,v8:4059
R=jarin@chromium.org
LOG=n
Review URL: https://codereview.chromium.org/1841513003
Cr-Commit-Position: refs/heads/master@{#35094}
The getter RegExp.prototype.source is specified in ES2015 to throw when
called on a non-RegExp instance, such as RegExp.prototype. We had previously
put in a compatibility workaround for all RegExp getters to make them
throw on access specifically with RegExp.prototype as the receiver; however,
we only have evidence that this is needed for properties other than source.
This patch removes the compatibility workaround for get RegExp.prototype.source
and gives it semantics precisely as per the ES2015 specification.
R=adamk
BUG=chromium:581577,v8:4827
LOG=Y
Review URL: https://codereview.chromium.org/1837843002
Cr-Commit-Position: refs/heads/master@{#35086}
Previously, they would check for Symbol.match/Symbol.search, and then
do another check for Symbol.match in the RegExp constructor. This patch
avoids the second one by skipping the RegExp constructor, as the spec does.
Review URL: https://codereview.chromium.org/1840723002
Cr-Commit-Position: refs/heads/master@{#35085}
Add support for optimizing objects with elements, which do not invoke JS and
cannot change the shape of the Object.
BUG=v8:4663
LOG=N
Review URL: https://codereview.chromium.org/1767113004
Cr-Commit-Position: refs/heads/master@{#35081}
It's been on since M49. Also moved tests from harmony -> es6,
one of which was merged with another test of the same name.
While moving stuff over to regexp.js, I also noticed that there
were unused calls to %FunctionSetName and %SetNativeFlag (those
calls are already handled by InstallGetter()).
Review URL: https://codereview.chromium.org/1838563003
Cr-Commit-Position: refs/heads/master@{#35076}
A bug in error printing meant that we failed to do proper type checks
before calling into C++ code, which could lead to RUNTIME_ASSERT
failures if methods are called on alternative receivers. This patch
adds the right type checks.
BUG=chromium:596718
LOG=Y
R=adamk
Review URL: https://codereview.chromium.org/1831053003
Cr-Commit-Position: refs/heads/master@{#35069}
This patch implements ES2015 RegExp subclassing semantics, namely the
hardest part where RegExp.prototype.exec and certain flag getters can
be overridden in order to provide different behavior. This change is
hidden behind a new flag, --harmony-regexp-exec. The flag guards the
behavior by installing entirely different implementations of the
methods which follow the new semantics.
Preliminary performance tests show a 3-4x regression in the Octane
RegExp benchmark. The new code doesn't call out into several fast
paths that the old code supported, so this is expected.
The patch is tested mostly by test262, where most RegExp tests are fixed,
with the exception of deliberate spec violations for web compatibility,
and for the 'sticky' flag, which is not dynamically read by this patch
in all cases but rather statically compiled into the RegExp. The latter
will require a follow-on patch to implement. A small additional set of
tests verifies one particular case, mostly to check whether the flag
mechanism works.
R=adamk,yangguo@chromium.org
LOG=Y
BUG=v8:4602
Review URL: https://codereview.chromium.org/1596483005
Cr-Commit-Position: refs/heads/master@{#35068}
Makes --ignition cause eager compilation if we aren't building the startup
snapshot.
BUG=v8:4280
LOG=N
Review URL: https://codereview.chromium.org/1811553003
Cr-Commit-Position: refs/heads/master@{#35066}
Reason for revert:
[Sheriff] Seems to break nosnap debug:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20nosnap%20-%20debug/builds/6019
Original issue's description:
> [Interpreter] Adds support to fetch return value on break at return.
>
> Debugger fetches the return value of a function when we break at return.
> Interpreter holds the return value in accumulator. This is not stored in a
> specified location on stack and hence it is not possible to look it up from
> stack similar to full-codegen or optimized frames. This cl adds support to
> store the value of accumulator on debug breaks. The value of accumulator is
> passed to the runtime function and is then stored in thread local data.
>
> Also changes full-codegen implementation to match that of ignition.
> The return value from full-codegen is also stored in thread local data.
> The return value is fetched directly thread local data instead of
> finding it by iterating over frames.
>
> BUG=v8:4280, v8:4690
> LOG=N
>
> Committed: https://crrev.com/fb65527b75754bcf3b173f16f5d0b04a1c6d9b99
> Cr-Commit-Position: refs/heads/master@{#35060}
TBR=rmcilroy@chromium.org,yangguo@chromium.org,weiliang.lin@intel.com,balazs.kilvady@imgtec.com,jyan@ca.ibm.com,mythria@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4280, v8:4690
Review URL: https://codereview.chromium.org/1834733002
Cr-Commit-Position: refs/heads/master@{#35065}
Debugger fetches the return value of a function when we break at return.
Interpreter holds the return value in accumulator. This is not stored in a
specified location on stack and hence it is not possible to look it up from
stack similar to full-codegen or optimized frames. This cl adds support to
store the value of accumulator on debug breaks. The value of accumulator is
passed to the runtime function and is then stored in thread local data.
Also changes full-codegen implementation to match that of ignition.
The return value from full-codegen is also stored in thread local data.
The return value is fetched directly thread local data instead of
finding it by iterating over frames.
BUG=v8:4280, v8:4690
LOG=N
Review URL: https://codereview.chromium.org/1818873003
Cr-Commit-Position: refs/heads/master@{#35060}
With the recent spec change removing the [[Enumerate]] internal method, we now
have to walk the complete prototype chain. This implies that we call the
[[GetPrototypeOf]] trap on proxies.
As a secondary change we now trigger the [[GetOwnProperty]] trap for the for-in
filter step to see whether the properties are still enumerable. Before we did this
in the key-accumulation phase. This way we slightly reduce the number of traps
invoked. Whilst this is not ideal, it comes closer to the Spec's example
implementation.
BUG=v8:1543, v8:4768
LOG=n
Review URL: https://codereview.chromium.org/1748923003
Cr-Commit-Position: refs/heads/master@{#35017}
Now that ES2015 const has shipped, in Chrome 49, legacy const declarations
are no more. This lets us remove a bunch of code from many parts of the
codebase.
In this patch, I remove parser support for generating legacy const variables
from const declarations. This also removes the special "illegal declaration"
bit from Scope, which has ripples into all compiler backends.
Also gone are any tests which relied on legacy const declarations.
Note that we do still generate a Variable in mode CONST_LEGACY in one case:
function name bindings in sloppy mode. The likely fix there is to add a new
Variable::Kind for this case and handle it appropriately for stores in each
backend, but I leave that for a later patch to make this one completely
subtractive.
Review URL: https://codereview.chromium.org/1819123002
Cr-Commit-Position: refs/heads/master@{#35002}
This CL adds support for builtins with JavaScript linkage written using
the TurboFan CodeStubAssembler, but with a JSCall descriptor (which was
already supported thanks to a previous patch by Ben Smith). As a first
example, we convert the Math.sqrt builtin and thereby get rid of the
%_MathSqrt intrinsic, which causes trouble for the representation
selection pass in the JavaScript pipeline.
R=mstarzinger@chromium.org
Review URL: https://codereview.chromium.org/1824993002
Cr-Commit-Position: refs/heads/master@{#34989}
Fixes CopyBytecodeArray to set the interrupt_budget field.
BUG=v8:4280,v8:4690
LOG=N
Review URL: https://codereview.chromium.org/1809123003
Cr-Commit-Position: refs/heads/master@{#34982}
Add missing conversions from other types to f32 in fround.
Restrict fround() to only float, double, signed, unsigned (no unions / intish).
Restrict Bitwise operations to intish, particularly |0, when not applied to a foreign function.
Adding more exhaustive tests of stdlib Math, move to a separate file.
Adding tests of interesting values for the stdlib asm.js functions.
BUG= https://bugs.chromium.org/p/v8/issues/detail?id=4203
TEST=test-asm-validator,asm-wasm
R=titzer@chromium.org,rossberg@chromium.org
LOG=N
Review URL: https://codereview.chromium.org/1804243003
Cr-Commit-Position: refs/heads/master@{#34967}
Both of them shipped in Chrome 49 without incident.
Also move relevant tests from harmony/ to es6/.
Review URL: https://codereview.chromium.org/1815773002
Cr-Commit-Position: refs/heads/master@{#34964}
This is necessary to ensure that "Called non callable" exception will get a proper message and stack trace even for calls at tail position.
BUG=chromium:595615, v8:4698
LOG=N
Review URL: https://codereview.chromium.org/1818003002
Cr-Commit-Position: refs/heads/master@{#34962}
We don't want them to disappear from the stack traces.
BUG=v8:4698
LOG=N
Review URL: https://codereview.chromium.org/1818063002
Cr-Commit-Position: refs/heads/master@{#34957}
This change introduces wide prefix bytecodes to support wide (16-bit)
and extra-wide (32-bit) operands. It retires the previous
wide-bytecodes and reduces the number of operand types.
Operands are now either scalable or fixed size. Scalable operands
increase in width when a bytecode is prefixed with wide or extra-wide.
The bytecode handler table is extended to 256*3 entries. The
first 256 entries are used for bytecodes with 8-bit operands,
the second 256 entries are used for bytecodes with operands that
scale to 16-bits, and the third group of 256 entries are used for
bytecodes with operands that scale to 32-bits.
LOG=N
BUG=v8:4747,v8:4280
Review URL: https://codereview.chromium.org/1783483002
Cr-Commit-Position: refs/heads/master@{#34955}
This rebaselines all our internal tests for error messages thrown by the
implementation of 'instanceof' to the new ES6 semantics. It also applies
a minor rephrasing to the messages in question.
R=rossberg@chromium.org
BUG=v8:4447
LOG=n
Review URL: https://codereview.chromium.org/1822663002
Cr-Commit-Position: refs/heads/master@{#34940}
In case when F inlined normal call to G which tail calls H we should not write translation for G for the tail call site.
Otherwise we will see G in a stack trace inside H.
This CL also enables all existing tests related to ES6 tail call elimination and adds more combinations.
TBR=bmeurer@chromium.org
BUG=v8:4698
LOG=N
Committed: https://crrev.com/689980f7d4dfd4c29492f616d7b616b86ec9af91
Cr-Commit-Position: refs/heads/master@{#34830}
Review URL: https://codereview.chromium.org/1780043004
Cr-Commit-Position: refs/heads/master@{#34920}
Previously only JSFunctions seemed to be valid for toJSON, which doesn't
match the ES6 specification that allows any object with [[Call]]
internal method (i.e. any Callable in V8 terminology), including bound
functions and proxies.
BUG=chromium:595738, chromium:535408
R=yangguo@chromium.org
LOG=n
Review URL: https://codereview.chromium.org/1824533002
Cr-Commit-Position: refs/heads/master@{#34913}
Reason for revert:
This patch actually seemed to cause a further GameBoy regression! Reverting it seems to address the regression.
Original issue's description:
> Restore per-TypedArray-class length accessors as a perf workaround
>
> This patch is a workaround to the performance regression caused by
> implementing the ES2015 TypedArray prototype chain: Include a
> per-TypedArray-subclass length getter so that the superclass getter does
> not become polymorphic. The patch appears to fix a regression in the
> Gameboy Octane benchmark.
>
> BUG=chromium:579905
> R=adamk
> LOG=Y
>
> Committed: https://crrev.com/03ce7711e474a0ef74f723b30ae1527c89dec010
> Cr-Commit-Position: refs/heads/master@{#33501}
R=adamk@chromium.org
BUG=chromium:579905,chromium:593634
LOG=Y
Review URL: https://codereview.chromium.org/1812143004
Cr-Commit-Position: refs/heads/master@{#34906}
This patch fixes two bugs in Array.prototype.concat in conjunction with
subclassing Arrays:
- Create a new property rather than calling Set when adding elements to
the output array. This means setters are not called.
- If there is an exception thrown from DefineProperty, propagate it
outwards properly, rather than swallowing it. This can occur, e.g., with
a Proxy as the new output array.
R=adamk
LOG=Y
BUG=chromium:595319
Review URL: https://codereview.chromium.org/1814933002
Cr-Commit-Position: refs/heads/master@{#34876}
Repair this to match what the runtime correctly does, by first checking if the
function is a constructor before we access the prototype.
R=verwaest@chromium.org
BUG=
Review URL: https://codereview.chromium.org/1810953002
Cr-Commit-Position: refs/heads/master@{#34863}
The way desugared instanceof called OrdinaryHasInstance if the lookup of
@@hasInstance failed was incorrect.
BUG=v8:4774
LOG=N
Review URL: https://codereview.chromium.org/1812793002
Cr-Commit-Position: refs/heads/master@{#34855}
In case when F inlined normal call to G which tail calls H we should not write translation for G for the tail call site.
Otherwise we will see G in a stack trace inside H.
This CL also enables all existing tests related to ES6 tail call elimination.
TBR=bmeurer@chromium.org
BUG=v8:4698
LOG=N
Review URL: https://codereview.chromium.org/1780043004
Cr-Commit-Position: refs/heads/master@{#34830}
Function declarations were previously permitted by V8 in many locations
which no ECMAScript specification allowed; the ECMAScript 2015 spec
enumerates a few locations (in blocks, as well as after labels and in
conditionals when in sloppy mode). This patch ships the flag to restrict
the usage of function declarations to those contexts.
R=adamk
LOG=Y
BUG=v8:4824
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel
Review URL: https://codereview.chromium.org/1799233003
Cr-Commit-Position: refs/heads/master@{#34828}
