- Currently WebAssembly.Memory.grow() assumes that it always has an instance associated with it,
fix to grow and reflect new size when no instance is associated with memory object.
- Correctness fixes for the js api, throw range errors instead of generic errors
BUG=chromium:680938
R=bradnelson@chromium.org, titzer@chromium.org
Review-Url: https://codereview.chromium.org/2638243002
Cr-Commit-Position: refs/heads/master@{#42432}
Enable enqueueing of eager inner function compilation onto the compiler
dispatcher. This enables these tasks to be performed in parallel to
compilation of the outer functio (only for Ignition functions).
We currently synchronize to ensure all inner function compilations
are complete before executing the outer function - future work will
allow outer function execution to happenin parallel to inner function
compilation.
BUG=v8:5203,v8:5215
Review-Url: https://codereview.chromium.org/2611313002
Cr-Commit-Position: refs/heads/master@{#42413}
The resume trampolin used to call the generator function with the context of the
last suspension rather than the closure's context. While that was fine for
Ignition, Turbofan got utterly confused. With this CL, the resume trampolin
always passes in the closure's context (like in the very first call of the
generator function). The generator function itself then restores its previously
current context by reading it from the generator object and doing a
PushContext.
BUG=chromium:681171
Review-Url: https://codereview.chromium.org/2639533002
Cr-Commit-Position: refs/heads/master@{#42407}
The bug was caused by AstTraversalVisitor refactoring:
https://codereview.chromium.org/2169833002/
InitializerRewriter::VisitRewritableExpression in parser.cc didn't recurse; so
it fails when a rewritable expression contains another rewritable expression.
See the bug for more details.
BUG=chromium:679727
Review-Url: https://codereview.chromium.org/2629623002
Cr-Commit-Position: refs/heads/master@{#42274}
Lower StoreDataPropertyInLiteral() when storing
computed property names in object literals.
Add a new AccessMode, kStoreInLiteral. It is similar to
AccessMode::kStore but does not look
up properties on the prototype chain.
99% of all literal definitions with computed property names
end up with generic access_info because of how we count
properties. Once we fix
https://bugs.chromium.org/p/v8/issues/detail?id=5625,
they'll get lowered as well.
BUG=v8:5624
Review-Url: https://codereview.chromium.org/2619773002
Cr-Commit-Position: refs/heads/master@{#42210}
Previously, the Intl implementation tracked types two ways:
- In the intl_initialized_marker_symbol
- In various named properties of the intl_impl_object_symbol value
As far as I can tell, these will never disagree with each other,
modulo bugs in Intl itself. This patch removes the second type
checking system.
This reland includes a fixed type check for
Intl.DateTimeFormat.prototype.formatToParts , which is the only Intl
method which is not bound. All future methods will follow this
pattern.
The second reland ensures that a newly inserted test is only run
if Intl is present.
BUG=v8:5751,chromium:677055, v8:4962
CQ_INCLUDE_TRYBOTS=master.tryserver.v8:v8_linux_noi18n_rel_ng
TBR=yangguo@chromium.org
Review-Url: https://codereview.chromium.org/2623683002
Cr-Commit-Position: refs/heads/master@{#42152}
We did not associate any position to the stack check in the wasm
function prologue, hence a check failed later when trying to map the
non-existent position to the asm.js source position.
With this CL, we add a mapping to the source position table, mapping
the stack check call to byte offset 0 (which is distinct from any valid
instruction position). Also, we add another entry to the asm.js source
position sidetable, mapping byte offset 0 to the start source position
of the function body.
R=titzer@chromium.org, ahaas@chromium.org
BUG=chromium:677685
Review-Url: https://codereview.chromium.org/2609363004
Cr-Commit-Position: refs/heads/master@{#42130}
Don't assume that the prototype of an object is always a JSObject when
inlining the known receiver map case for abstract relational comparison.
BUG=chromium:679202
R=ishell@chromium.org
Review-Url: https://codereview.chromium.org/2621583002
Cr-Commit-Position: refs/heads/master@{#42123}
Reason for revert:
Breaks noi18n.
Original issue's description:
> [intl] Remove redundant type checking system
>
> Previously, the Intl implementation tracked types two ways:
> - In the intl_initialized_marker_symbol
> - In various named properties of the intl_impl_object_symbol value
>
> As far as I can tell, these will never disagree with each other,
> modulo bugs in Intl itself. This patch removes the second type
> checking system.
>
> This reland includes a fixed type check for
> Intl.DateTimeFormat.prototype.formatToParts , which is the only Intl
> method which is not bound. All future methods will follow this
> pattern.
>
> BUG=v8:5751,chromium:677055, v8:4962
> CQ_INCLUDE_TRYBOTS=master.tryserver.v8:v8_linux_noi18n_rel_ng
>
> Review-Url: https://codereview.chromium.org/2600913002
> Cr-Commit-Position: refs/heads/master@{#42118}
> Committed: aa8a2d2789TBR=yangguo@chromium.org,adamk@chromium.org,littledan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5751,chromium:677055, v8:4962
Review-Url: https://codereview.chromium.org/2617323002
Cr-Commit-Position: refs/heads/master@{#42119}
Previously, the Intl implementation tracked types two ways:
- In the intl_initialized_marker_symbol
- In various named properties of the intl_impl_object_symbol value
As far as I can tell, these will never disagree with each other,
modulo bugs in Intl itself. This patch removes the second type
checking system.
This reland includes a fixed type check for
Intl.DateTimeFormat.prototype.formatToParts , which is the only Intl
method which is not bound. All future methods will follow this
pattern.
BUG=v8:5751,chromium:677055, v8:4962
CQ_INCLUDE_TRYBOTS=master.tryserver.v8:v8_linux_noi18n_rel_ng
Review-Url: https://codereview.chromium.org/2600913002
Cr-Commit-Position: refs/heads/master@{#42118}
We need to check both sides for abstract equality of receivers in optimized
code, otherwise we don't handle implicit conversions and undetectable
objects correctly.
R=jarin@chromium.org
BUG=v8:5802
Review-Url: https://codereview.chromium.org/2612213002
Cr-Commit-Position: refs/heads/master@{#42085}
When Crankshaft compiles a keyed load to arguments, it disabled
optimization unless the KEYED_LOAD_IC for the access was monomorphic.
But that's too restrictive, since it will also disable optimization
for this function when the access is on a path that was never executed
so far.
This was spotted in the Node.js core function EventEmitter.prototype.emit,
which was no longer optimizable with Crankshaft using latest V8.
R=jarin@chromium.org
BUG=v8:5790
Review-Url: https://codereview.chromium.org/2607303002
Cr-Commit-Position: refs/heads/master@{#42005}
The TF version of this operation was missing a ToObject coercion, so failed to do
@@toStringTag lookups when passed primitive values.
R=franzih@chromium.org
BUG=v8:5780
Review-Url: https://codereview.chromium.org/2597323002
Cr-Commit-Position: refs/heads/master@{#41961}
ECMA 402 v2 made Intl constructors more strict in terms of how they would
initialize objects, refusing to initialize objects which have already
been constructed. However, when Chrome tried to ship these semantics,
we ran into web compatibility issues.
This patch tries to square the circle and implement the simpler v2 object
semantics while including a compatibility workaround to allow objects to
sort of be initialized later, storing the real underlying Intl object
in a symbol-named property.
The new semantics are described in this PR against the ECMA 402 spec:
https://github.com/tc39/ecma402/pull/84
BUG=v8:4360, v8:4870
LOG=Y
Review-Url: https://codereview.chromium.org/2582993002
Cr-Commit-Position: refs/heads/master@{#41943}
In certain corner-cases we would grow a FAST_ELEMENTS packed backing store of a
JS_ARGUMENTS_TYPE object without converting to holey elements kinds. As a side
effect you could then read out the_hole.
BUG=v8:5772
Review-Url: https://codereview.chromium.org/2597013004
Cr-Commit-Position: refs/heads/master@{#41921}
eval() may introduce a scope which needs to be represented as a context at
runtime, e.g.,
eval('var x; let y; ()=>y')
introduces a variable y which needs to have a context allocated for it. However,
when traversing upwards to find the declaration context for a variable which leaks,
as the declaration of x does above, this context has to be understood to not be
a declaration context in sloppy mode.
This patch makes that distinction by introducing a different map for eval-introduced
contexts. A dynamic search for the appropriate context will continue past an eval
context to find the appropriate context. Marking contexts as eval contexts rather
than function contexts required updates in each compiler backend.
BUG=v8:5295, chromium:648719
Review-Url: https://codereview.chromium.org/2435023002
Cr-Commit-Position: refs/heads/master@{#41869}
Previously, the Intl.DateTimeFormat constructor and other related paths had
a bug where the options bag passed in would be modified in place. This patch
makes V8's Intl implementation follow the specification's logic to avoid
such a modification.
BUG=v8:4219
Review-Url: https://codereview.chromium.org/2587703002
Cr-Commit-Position: refs/heads/master@{#41826}
The scenario here: the asm function fails asm validation,
so we emit a message. In doing so, we create a JSValue wrapper for
the script object that we cache on the script object. This wrapper
is context-dependent and causes the code serializer to choke.
R=mtrofin@chromium.org, titzer@chromium.org
BUG=chromium:674446,chromium:673321
Review-Url: https://codereview.chromium.org/2586943003
Cr-Commit-Position: refs/heads/master@{#41794}
The inspector cannot deal with breaking inside of debug-evaluate.
There is therefore no point in supporting that in the debugger.
The optional additional context parameter for debug-evaluate also
can be removed since it's not being used.
R=jgruber@chromium.org
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2580323002
Cr-Commit-Position: refs/heads/master@{#41791}
When mark-shared-funtion-for-tier-up is enabled, a function could be marked for
optimization, then the baseline (FCG) code is flushed by the GC. The next time
the function is executed, we shouldn't optimize the code if there isn't
baseline code.
BUG=chromium:673242
Review-Url: https://codereview.chromium.org/2575333003
Cr-Commit-Position: refs/heads/master@{#41751}
If the eval contains a let, we need to know whether an inner function
refers to the variable to be able to decide its context allocation
status.
The added test needs https://codereview.chromium.org/2435023002/ too
in order to pass.
BUG=v8:5736
Review-Url: https://codereview.chromium.org/2574753002
Cr-Commit-Position: refs/heads/master@{#41723}
This fixes a corner case where the {FastCloneShallowArrayStub} was used
for literals that are backed by a double backing store and would exceed
limits for new-space allocations on 32-bit architectures. The stub in
question does not support such literals, callers must use the runtime.
Note that this fix is for Ignition as well as FullCodeGenerator.
R=rmcilroy@chromium.org
TEST=mjsunit/regress/regress-crbug-672792
BUG=chromium:672792
Review-Url: https://codereview.chromium.org/2570843002
Cr-Commit-Position: refs/heads/master@{#41713}
Determine if the scope of the function to be serialized includes asm-
wasm, and if so, bypass serialization, since we do not support it in
that scenario.
In this change, we do so regardless of whether the asm-wasm path was
successful. This is so we keep the design simple, since the guidance
to developers, moving forward, is to use wasm.
BUG=643595
Review-Url: https://codereview.chromium.org/2573193002
Cr-Commit-Position: refs/heads/master@{#41704}
The least two bits of the owner field of a Page are used to determine
whether the Page is part of a large object. If these bits are not equal
to 0x11, the page is part of a large object and needs special handling
e.g. in MemoryChunk::FromAnyPointerAddress to determine which chunk it
belongs to.
This CL fixes an issue in which the store buffer overflows after
a large object space allocation but before the object has been fully
initialized. Store buffer overflow handling attempts to look up the
chunk of a page, but fails to do so correctly since the page's owner
field has not yet been initialized.
This CL ensures that the owner field of all pages belonging to a large
object allocation are initialized to a value that is interpreted
correctly.
BUG=chromium:672041
Committed: https://crrev.com/9b6808bfb5366beebe3af30a06f9851edb2039d4
Review-Url: https://codereview.chromium.org/2565713002
Cr-Original-Commit-Position: refs/heads/master@{#41641}
Cr-Commit-Position: refs/heads/master@{#41687}
Debug mirrors will no longer be supported in the near future.
It will now only be tested by being used by the v8-inspector.
R=jgruber@chromium.org
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2566103002
Cr-Commit-Position: refs/heads/master@{#41686}
The fast-path for dynamic local variables was previously checking the
lookup variable rather than the shadowed variable when deciding whether
to add a hole check.
BUG=669540
Review-Url: https://codereview.chromium.org/2551023004
Cr-Commit-Position: refs/heads/master@{#41677}
Reason for revert:
Tree closer: https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/builds/12409
Original issue's description:
> [heap] Initialize the owner on each page after lospace allocation
>
> The least two bits of the owner field of a Page are used to determine
> whether the Page is part of a large object. If these bits are not equal
> to 0x11, the page is part of a large object and needs special handling
> e.g. in MemoryChunk::FromAnyPointerAddress to determine which chunk it
> belongs to.
>
> This CL fixes an issue in which the store buffer overflows after
> a large object space allocation but before the object has been fully
> initialized. Store buffer overflow handling attempts to look up the
> chunk of a page, but fails to do so correctly since the page's owner
> field has not yet been initialized.
>
> This CL ensures that the owner field of all pages belonging to a large
> object allocation are initialized to a value that is interpreted
> correctly.
>
> BUG=chromium:672041
>
> Committed: https://crrev.com/9b6808bfb5366beebe3af30a06f9851edb2039d4
> Cr-Commit-Position: refs/heads/master@{#41641}
TBR=mlippautz@chromium.org,jgruber@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:672041
Review-Url: https://codereview.chromium.org/2562273004
Cr-Commit-Position: refs/heads/master@{#41644}
The least two bits of the owner field of a Page are used to determine
whether the Page is part of a large object. If these bits are not equal
to 0x11, the page is part of a large object and needs special handling
e.g. in MemoryChunk::FromAnyPointerAddress to determine which chunk it
belongs to.
This CL fixes an issue in which the store buffer overflows after
a large object space allocation but before the object has been fully
initialized. Store buffer overflow handling attempts to look up the
chunk of a page, but fails to do so correctly since the page's owner
field has not yet been initialized.
This CL ensures that the owner field of all pages belonging to a large
object allocation are initialized to a value that is interpreted
correctly.
BUG=chromium:672041
Review-Url: https://codereview.chromium.org/2565713002
Cr-Commit-Position: refs/heads/master@{#41641}
By now the predicate in question is an exact negation of %IsAsmWasmCode
as the name intuitively implies. The need for two separate test methods
no longer exists and one of the two can be removed.
R=bradnelson@chromium.org
Review-Url: https://codereview.chromium.org/2562003002
Cr-Commit-Position: refs/heads/master@{#41616}
Passing kAllowLargeObjectAllocation now allocates in LOS if necessary.
Allow such allocations when growing fixed arrays in RegExp's @@match
and @@split operations.
BUG=chromium:670671
Review-Url: https://codereview.chromium.org/2555703003
Cr-Commit-Position: refs/heads/master@{#41526}
Incooperate suggestion from adamk@ to only sent lexical variables to
I+TF that require explicit initialization, i.e. don't send named
function expressions to I+TF. This should recover most of the regression
now.
Also introduce a regression test for the original let issue.
BUG=chromium:670691,v8:5666
R=adamk@chromium.org,yangguo@chromium.org
Review-Url: https://codereview.chromium.org/2556663002
Cr-Commit-Position: refs/heads/master@{#41507}
This avoids the crash which ClusterFuzz found, but still does not
report the same position as without validate.asm.
For calls like "foo()|0", we report the position of the call instead of
the position of the "|" if ToNumber throws an error.
After this CL, the correctness-fuzzer for validate-asm will probably
find mismatches there.
R=titzer@chromium.org
BUG=chromium:670808
Review-Url: https://codereview.chromium.org/2548323002
Cr-Commit-Position: refs/heads/master@{#41500}
This enables {FLAG_debug_code} by default in debug builds. The advantage
is that generated code contained within the snapshot will contain such
debug code. Before we would only get coverage for these pieces with the
no-snapshot builds, which have a meager coverage. One can still pass the
inverse --no-debug-code flag to ensure generated code remains readable
within debug builds as well.
R=machenbach@chromium.org
Review-Url: https://codereview.chromium.org/2528913002
Cr-Commit-Position: refs/heads/master@{#41451}
This fixes the existing workaround in {BytecodeGraphBuilder} where the
number of elements in an array literal is unknown just from the bytecode
alone and needs to be deduced from the constant elements.
Note that this is just a quick fix to prevent calling the fast-clone
stub for boilerplates that are too big to fit on a regular page. In the
long run we need something more solid here.
R=mvstanton@chromium.org
TEST=mjsunit/regress/regress-crbug-669850
BUG=chromium:669850
Review-Url: https://codereview.chromium.org/2542633002
Cr-Commit-Position: refs/heads/master@{#41420}
The EscapeStatusAnalysis didn't know anything about the simplified
operator ConvertTaggedHoleToUndefined, thus leading to a crash. We
now just handled it by pretending that any allocation that goes into
such a node escapes.
BUG=chromium:669451
R=tebbi@chromium.org
Review-Url: https://codereview.chromium.org/2533263002
Cr-Commit-Position: refs/heads/master@{#41359}
This ensures the deoptimization triggered due to materialization of
objects by the {TranslatedState} works in conjunction with OSR. The
optimized code used for OSR is not installed on the function, hence
needs to be specified explicitly when requesting deoptimization for
specific stack frames.
R=jarin@chromium.org
TEST=mjsunit/regress/regress-crbug-668795
BUG=chromium:668795
Review-Url: https://codereview.chromium.org/2534143002
Cr-Commit-Position: refs/heads/master@{#41348}
Up until now assertThrows allows to check the type field of an
exception, which is, however, a custom field introduced in a single
regression test. With the change assertThrows allows to check the
message field of an exception, which is set for standard V8 exceptions
by default.
I use the new assertThrows to refactor test/mjsunit/wasm/divrem-trap.js
R=titzer@chromium.org
Review-Url: https://codereview.chromium.org/2525313003
Cr-Commit-Position: refs/heads/master@{#41302}
A missing @@IsConcatSpreadable check caused the fast path inside the slow path
to be incorrect and follow the default concat strategy when the arguments
arrays contain only doubles.
BUG=chromium:668414
Review-Url: https://codereview.chromium.org/2527173002
Cr-Commit-Position: refs/heads/master@{#41301}
For dictionary-mode receivers, the KeyedStoreGeneric stub can store
properties directly in most cases. Doing so avoids the need to have
an entry in the stub cache for every map/property combination.
Original review: https://codereview.chromium.org/2504403005/
Review-Url: https://codereview.chromium.org/2528883003
Cr-Commit-Position: refs/heads/master@{#41272}
For dictionary-mode receivers, the KeyedStoreGeneric stub can store
properties directly in most cases. Doing so avoids the need to have
an entry in the stub cache for every map/property combination.
Original review: https://codereview.chromium.org/2504403005/
Review-Url: https://codereview.chromium.org/2524943002
Cr-Commit-Position: refs/heads/master@{#41218}
Previous fuzzer fix broke the case when the pending assessment came from the same
block. In that case, the assessments table does not have an entry yet for the block,
because we register only when we're done processing a block.
BUG=667745
Review-Url: https://codereview.chromium.org/2519973004
Cr-Commit-Position: refs/heads/master@{#41193}
* Fix setting script-scope variables through inspector by internalizing
their names.
* Reconstruct values of Number, String, and Boolean classes.
* Adapt a couple of tests for API restrictions.
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2512963002
Cr-Commit-Position: refs/heads/master@{#41175}
This fixes stack unwinding to always recompute the stack pointer for
interpreted frames. For frames materialized by the deoptimizer we elide
the handler frame in between, hence arguments being pushed on the stack
will no longer be pushed into the handler frame but into the interpreted
frame directly.
R=jarin@chromium.org
TEST=mjsunit/regress/regress-crbug-662830
BUG=chromium:662830
Review-Url: https://codereview.chromium.org/2517203003
Cr-Commit-Position: refs/heads/master@{#41170}
The verifier needs to use the block and assessments in that block corresponding to
a predecessor of a "pending" assessment. Not doing that causes incorrect
assessments when 2 locations are swapped.
BUG=665402
Review-Url: https://codereview.chromium.org/2515803002
Cr-Commit-Position: refs/heads/master@{#41159}
Adapted various tests to restrictions of inspector protocol:
* osr-typing-debug-change: Don't set function variable value.
* debug-evaluate-locals: Add variable introduced by eval, run typeof
inside evaluate().
* regress-419663: Don't set duplicate breakpoints.
* regress-crbug-465298: Compare against function name instead of value.
* regress-crbug-621361: Make evaluate return string results.
* debug-script: Various counts were off due to new way tests are called.
Added new inspector script type.
Breakpoints now contain the actual break position, and remote object
reconstruction has been extended a bit.
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2505363002
Cr-Commit-Position: refs/heads/master@{#41129}
The code which pushes and pops to the function name inference stack
generally checks if the stack is active with the IsOpen method. One
piece of code pertaining to async functions was missing that check.
This patch adds it.
BUG=chromium:658267
R=gsathya,caitp
Review-Url: https://codereview.chromium.org/2514893002
Cr-Commit-Position: refs/heads/master@{#41120}
It originates from the era where we used to run a separate preparse step
before parsing and store the function data. Now the usage of preparser
is something completely different, so this flag doesn't make sense any
more.
In addition, this way we get more test coverage for preparser (for small
scripts).
BUG=
Review-Url: https://codereview.chromium.org/2513563002
Cr-Commit-Position: refs/heads/master@{#41110}
This adds clearAllBreakPoints functionality (which requires tracking set
breakpoints internally), and several script-related functions that rely
on runtime functions.
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2508853003
Cr-Commit-Position: refs/heads/master@{#41064}
This adds access to the LiveEdit API object, frame.restart(), and
various breakpoint setters. The LiveEdit API still depends on the JS
debugging context and blocks its removal; but it should be removed
once LiveEdit is rewritten in the midterm.
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2503293002
Cr-Commit-Position: refs/heads/master@{#41062}
The operator in question is guaranteed to produce a tagged value that is
not equal to the-hole, it however does not guarantee the value to be a
HeapObject. The correct representation hence is {kTagged}.
R=bmeurer@chromium.org
TEST=mjsunit/regress/regress-crbug-665587
BUG=chromium:665587
Review-Url: https://codereview.chromium.org/2504183002
Cr-Commit-Position: refs/heads/master@{#41032}
The reason is that non-configurability still allows a writable property to become read-only.
BUG=chromium:663750
Review-Url: https://codereview.chromium.org/2508873002
Cr-Commit-Position: refs/heads/master@{#41029}
This CL further extends the debug wrapper, migrates around 60 tests, and
removes a few tests that use functionality we will not support anymore.
In more detail:
* Removed tests that use:
* enable/disable individual breakpoints
* invocationText()
* the ScriptCollected event
* showBreakPoints
* evalFromScript (and similar)
* mirror.constructedBy and mirror.referencedBy
* event_data.promise()
* Some frame.evaluate uses were adapted since due to differences between
remote objects (inspector) and mirrors. For instance, exceptions are
currently not recreated exactly, since the inspector protocol does not
give us the stack and message separately. Other objects (such as
'this' in debug-evaluate-receiver-before-super) need to be explicitly
converted to a string before the test works correctly.
* Ensure that inspector stores the script before sending ScriptParsed and
ScriptFailedToParse events in order to be able to use the script from
within those events.
* Better remote object reconstruction (e.g. for undefined and arrays).
* New functionality in wrapper:
* debuggerFlags().breakPointsActive.setValue()
* scripts()
* execState.setVariableValue()
* execState.scopeObject().value()
* execState.scopeObject().property()
* execState.frame().allScopes()
* eventData.exception()
* eventData.script()
* setBreakPointsActive()
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2497973002
Cr-Commit-Position: refs/heads/master@{#41019}
This fixes the bogus {Word32Equal} comparison in the ToString builtin
implementing Object.prototype.toString to be a pointer-size {WordEqual}
comparison instead. Comparing just the lower half-word is insufficient
on 64-bit architectures.
R=jgruber@chromium.org
TEST=mjsunit/regress/regress-crbug-664506
BUG=chromium:664506
Review-Url: https://codereview.chromium.org/2496043003
Cr-Commit-Position: refs/heads/master@{#40963}
A SmiUntag() was missing when loading the old backing store's length.
BUG=chromium:664469
Review-Url: https://codereview.chromium.org/2492783004
Cr-Commit-Position: refs/heads/master@{#40921}
This CL adds support for:
* conditional breaks in setBreakpoint,
* locals in frame.local{Count,Name,Value},
* evaluation on a frame in frame.evaluate,
* and more detailed scope information in scopeObject.
Uses of several functions that are not covered by the
inspector protocol and are only used in tests have been removed.
Local handling has been modified to also include arguments as locals.
Inspector differs in this regard from our FrameDetails in that
arguments are always shown as locals. Argument-related functions
were removed.
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2491543002
Cr-Commit-Position: refs/heads/master@{#40917}
This changes {FrameState} nodes modeling "after" states to use bytecode
offsets pointing to the deoptimizing bytecode. This is in sync with the
normal execution, as the bytecode offset is advanced after operations
complete in regular bytecode handlers.
The change is necessary to ensure lazy deoptimized frames contain an
accurate bytecode offset while they are on the stack. Such frames can be
inspected by various stack walks. The continuation builtin will advance
the bytecode offset upon return.
R=jarin@chromium.org
TEST=mjsunit/regress/regress-crbug-660379
BUG=chromium:660379
Review-Url: https://codereview.chromium.org/2487173002
Cr-Commit-Position: refs/heads/master@{#40887}
If an exception is thrown when there is a Promise being created, the Promise
catch prediction code would call into a part implemented in JavaScript to see if
the Promise has a catch handler. If it is not possible to call back into JS,
e.g., due to a stack overflow, then this would lead to a crash. This patch
"speculates" that, if it's impossible to call back into JavaScript, then the
error is unhandled, avoding the issue. In a future patch, the catch prediction
logic should be entirely written in C++, but this patch adds a minimal fix to
be more friendly to backports.
BUG=chromium:662935
R=jgruber
Review-Url: https://codereview.chromium.org/2487833002
Cr-Commit-Position: refs/heads/master@{#40851}
Don't rely on carry flags you didn't set yourself.
BUG=chromium:663402
Review-Url: https://codereview.chromium.org/2484283002
Cr-Commit-Position: refs/heads/master@{#40848}
This adds clearStepping plus the family of
{set,clear}BreakOn{,Uncaught}Exception functions.
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2482903002
Cr-Commit-Position: refs/heads/master@{#40834}
We really should deopt before the for-in index increment.
BUG=chromium:662904
Review-Url: https://codereview.chromium.org/2476423003
Cr-Commit-Position: refs/heads/master@{#40828}
This moves all tests currently working with the inspector debugger wrapper to
test/debugger.
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2480223002
Cr-Commit-Position: refs/heads/master@{#40824}
- When module bytes have a memory maximum defined, compiled module object should set maximum memory
- Exported memory objects should set maximum value on the memory objects
- Update tests to use declared maximum values.
R=ahaas@chromium.org
Review-Url: https://codereview.chromium.org/2474333003
Cr-Commit-Position: refs/heads/master@{#40820}
Note: This CL might regress code that relies on such arguments access.
In that case, we could still optimize the access if it accesses at
constant index (and the argument at that index is not context-allocated).
If any code relies on a general access to context-allocated arguments,
we would need to analyze the function for assignment to the arguments - this
might be quite tricky.
BUG=chromium:662845
Review-Url: https://codereview.chromium.org/2484723002
Cr-Commit-Position: refs/heads/master@{#40813}
The revert somehow lost the contents of regress-2825.js.
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=chromium:662928
Review URL: https://codereview.chromium.org/2483863002 .
Cr-Commit-Position: refs/heads/master@{#40806}
This moves all tests currently working with the inspector debugger wrapper to
test/debugger.
BUG=v8:5530
Review-Url: https://codereview.chromium.org/2480223002
Cr-Commit-Position: refs/heads/master@{#40804}
We need to rename the receiver on CheckHeapObject, because we
don't canonicalize numbers in SignedSmall range, and thus we
the representation selection can hand out TaggedSigned values
for receiver uses, even though we checked for TaggedPointerness
first.
Note that this is rather hacky and just intended to fix the bug
ASAP. We need to think about how to deal with representations in
earlier compilation stages.
BUG=chromium:662410
R=jarin@chromium.org
Review-Url: https://codereview.chromium.org/2485563002
Cr-Commit-Position: refs/heads/master@{#40792}
This disables the usage of the {maybe_assigned} flag that the variable
resolution computes for each variable on non-asm.js code. Note that the
analysis is fundamentally broken for destructuring and top-level lexical
variables. Also note that this still uses the analysis for asm.js code
even though it is not validated. One can still trigger the bug by using
invalid constructs within a function marked with "use asm". The fix is
intentionally minimal so that it can be merged to release branches.
R=bmeurer@chromium.org
TEST=mjsunit/regress/regress-crbug-659915
BUG=chromium:659915
Review-Url: https://codereview.chromium.org/2471523005
Cr-Commit-Position: refs/heads/master@{#40716}
This turns the ZoneList with minimum 6 words overhead into a linked list through variables, using 2 words for the empty list. Additionally the average number of pointers per entry goes down to the optimal 1 per variable that's in a list.
This does introduce 1 pointer unnecessary overhead for dynamic variables. If that becomes a problem we could distinguish between variables in lists and variables not in lists. We can distinguish them at construction-time.
BUG=v8:5209
Review-Url: https://codereview.chromium.org/2475433002
Cr-Commit-Position: refs/heads/master@{#40714}
Conflicting type feedback on Load/StoreICs can lead to out-of-bounds
field access, which is essentially dead code, but EscapeAnalysis was
confused about those. For now, mark the objects as escaping in these
cases, middle-term we want to deal better with this kind of compile-
time known dead code.
BUG=chromium:658185,v8:4586
R=jarin@chromium.org
Review-Url: https://codereview.chromium.org/2459273002
Cr-Commit-Position: refs/heads/master@{#40662}
This flag is on by default for now. Whenever heuristics in the compiler
pipeline decide to use Ignition+TurboFan, then {BytecodeGraphBuilder} is
active. Removing the flag reduces maintenance overhead.
R=mvstanton@chromium.org
Review-Url: https://codereview.chromium.org/2437103002
Cr-Commit-Position: refs/heads/master@{#40639}
For global object property cells, we did not check that the map on the
previous object is still the same for which we actually optimized. So
the optimized code was not in sync with the actual state of the property
cell. When loading from such a global object property cell, Crankshaft
optimizes away any map checks (based on the stable map assumption),
leading to arbitrary memory access in the worst case.
TurboFan has the same bug for stores, but is safe on loads because we
do appropriate map checks there. However mixing TurboFan and Crankshaft
still exposes the bug.
R=yangguo@chromium.org
BUG=chromium:659475
Review-Url: https://codereview.chromium.org/2444233004
Cr-Commit-Position: refs/heads/master@{#40592}
The TurboFan backends currently don't support tail-calls to CPP builtins
because the semantics of kJavaScriptCallArgCountRegister has different
semantics for stub call descriptors versus JavaScript call descriptors.
This is actually a short-coming of the backends and follow-up work will
make the backends more robust in that regard to fail hard on unsupported
constructs like that. This just disables the lowering creating such a
tail-call.
R=bmeurer@chromium.org
BUG=chromium:658691
TEST=mjsunit/regress/regress-crbug-658691
Review-Url: https://codereview.chromium.org/2447383002
Cr-Commit-Position: refs/heads/master@{#40590}
Reason for revert:
Breaks tree: http://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/builds/8789
Original issue's description:
> [compiler] Properly validate stable map assumption for globals.
>
> For global object property cells, we did not check that the map on the
> previous object is still the same for which we actually optimized. So
> the optimized code was not in sync with the actual state of the property
> cell. When loading from such a global object property cell, Crankshaft
> optimizes away any map checks (based on the stable map assumption),
> leading to arbitrary memory access in the worst case.
>
> TurboFan has the same bug for stores, but is safe on loads because we
> do appropriate map checks there. However mixing TurboFan and Crankshaft
> still exposes the bug.
>
> R=yangguo@chromium.org
> BUG=chromium:659475
TBR=yangguo@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:659475
Review-Url: https://codereview.chromium.org/2454513003
Cr-Commit-Position: refs/heads/master@{#40582}
For global object property cells, we did not check that the map on the
previous object is still the same for which we actually optimized. So
the optimized code was not in sync with the actual state of the property
cell. When loading from such a global object property cell, Crankshaft
optimizes away any map checks (based on the stable map assumption),
leading to arbitrary memory access in the worst case.
TurboFan has the same bug for stores, but is safe on loads because we
do appropriate map checks there. However mixing TurboFan and Crankshaft
still exposes the bug.
R=yangguo@chromium.org
BUG=chromium:659475
Review-Url: https://codereview.chromium.org/2444233004
Cr-Commit-Position: refs/heads/master@{#40578}
This brings the BytecodeGenerator in line with FullCodeGenerator, now that
more requests for hole checks are flowing through BuildVariableAssignment.
BUG=chromium:658528
Review-Url: https://codereview.chromium.org/2447783002
Cr-Commit-Position: refs/heads/master@{#40557}
Object.create(null) is most likely to be used for dictionary-like objects.
Hence it would be beneficial to directly create a slow-mode object and avoid
additional overhead later-on.
BUG=
Review-Url: https://codereview.chromium.org/2430273007
Cr-Commit-Position: refs/heads/master@{#40551}
This CL removes code that is now unused since the port of regexp.js has been
completed. Removed functions / classes are:
* regexp.js (GetSubstitution moved to string.js)
* RegExpConstructResult stub
* RegExpFlags intrinsic
* RegExpSource intrinsic
* RegExpInitializeAndCompile runtime function
BUG=v8:5339
Review-Url: https://codereview.chromium.org/2448463002
Cr-Commit-Position: refs/heads/master@{#40547}
CL https://codereview.chromium.org/2177273002 changed FastNewFunctionContextStub
to take a number of slots parameter and in-doing so removed the maximum slot
count for FastNewFunctionContextStub. This made it possible to create a
closure which is larger than kMaxRegularHeapObjectSize and so can't be
allocated by FastNewFunctionContextStub.
Reintroduce FastNewFunctionContextStub::kMaxSlots (but make the limit much
larger) to ensure we call the runtime for contexts which need to be
allocated in the LO space.
BUG=chromium:655573
Review-Url: https://codereview.chromium.org/2445703002
Cr-Commit-Position: refs/heads/master@{#40541}
The test ensures that in RegExp.prototype[@@split], exec is neither
accessed too early nor too often.
BUG=v8:5339,v8:5434
Review-Url: https://codereview.chromium.org/2440413002
Cr-Commit-Position: refs/heads/master@{#40526}
When lowering JSToLength, we cannot just smash arbitrary bounds on the
Select nodes, as that will confuse the representation selection later.
Instead properly rename the input using NumberMax and NumberMin.
R=jarin@chromium.org
BUG=chromium:657478
Review-Url: https://codereview.chromium.org/2440333002
Cr-Commit-Position: refs/heads/master@{#40519}
This CL refactors the handling of metadata associated with WebAssembly
modules to reduce the duplicate marshalling of data from the C++ world
to the JavaScript world. It does this by wrapping the C++ WasmModule*
object in a Foreign that is rooted from the on-heap WasmCompiledModule
(which is itself just a FixedArray). Upon serialization, the C++ object
is ignored and the original WASM wire bytes are serialized. Upon
deserialization, the C++ object is reconstituted by reparsing the bytes.
This is motivated by increasing complications in implementing the JS
API, in particular WebAssembly.Table, which must perform signature
canonicalization across instances.
Additionally, this CL implements the proper base + offset initialization
behavior for tables.
R=rossberg@chromium.org,bradnelson@chromium.org,mtrofin@chromium.org,yangguo@chromium.org
BUG=v8:5507, chromium:575167, chromium:657316
Review-Url: https://chromiumcodereview.appspot.com/2424623002
Cr-Commit-Position: refs/heads/master@{#40434}
Using uint32 to store the the number of control outputs allows WebAssembly switches to have more than 2^16 case.
BUG=v8:5531
TEST=mjsunit/regress/wasm/regression-5531
R=titzer@chromium.org
Review-Url: https://chromiumcodereview.appspot.com/2425983002
Cr-Commit-Position: refs/heads/master@{#40420}
When inlining JSCallConstruct in turbofan, receiver is initialized to model
the behaviour of constructor. When an implicit receiver is not required the
receiver value should be set to the hole value instead of undefined value.
When initializing the receiver via super calls, we check that the receiver
is the hole value.
BUG=chromium:653407
Review-Url: https://codereview.chromium.org/2424123002
Cr-Commit-Position: refs/heads/master@{#40396}
The inlined version of Array.prototype.push returned the value that was
pushed instead of the new "length" property value.
R=jarin@chromium.org
BUG=chromium:656037
Review-Url: https://codereview.chromium.org/2425903002
Cr-Commit-Position: refs/heads/master@{#40384}
There are a couple of operators that can indeed produce Float32
representation, which we might end up using in a TaggedSigned
context, so add the missing conversion (indirectly via Float64).
BUG=chromium:656275
R=jarin@chromium.org
Review-Url: https://codereview.chromium.org/2421193002
Cr-Commit-Position: refs/heads/master@{#40334}
A decoder error sets builder_ to null, which causes builder_->StackCheck
to segfault.
R=titzer@chromium.org
TEST=mjsunit/regress/wasm/loop-stack-check
Review-Url: https://codereview.chromium.org/2416873002
Cr-Commit-Position: refs/heads/master@{#40271}
We accidently dropped the effect on the floor that we have for the
polymorphic map check in case of array elements access.
BUG=chromium:655004
R=jarin@chromium.org
Review-Url: https://codereview.chromium.org/2411273002
Cr-Commit-Position: refs/heads/master@{#40201}
For ConsString, the left hand side must be either sequential or external
if the right hand side is empty.
R=jarin@chromium.org
BUG=chromium:654723
NOTRY=true
Review-Url: https://codereview.chromium.org/2410893003
Cr-Commit-Position: refs/heads/master@{#40192}
There were once plans to generate cross-context code with TurboFan,
however that doesn't fit into the model anymore, and so all of this
is essentially dead untested code (and thus most likely already broken
in subtle ways). With this mode still in place it would also be a lot
harder to make inlining based on SharedFunctionInfo work.
BUG=v8:2206,v8:5499
R=jarin@chromium.org
Review-Url: https://codereview.chromium.org/2406803002
Cr-Commit-Position: refs/heads/master@{#40109}
- Added gating code in the module-decoder to allow SIMD code only when
it can be decoded correctly
- SIMD128 values should not be exported to JS
- Try/Catch should not be available in asmjs modules
- Trivial fixes for S128 values
BUG=chromium:648079
R=ahaas@chromium.org, titzer@chromium.org, bradnelson@chromium.org
Review-Url: https://codereview.chromium.org/2400863003
Cr-Commit-Position: refs/heads/master@{#40067}
This patch sets `this` to be undefined when calling resolve and reject
functions attached to the deferred.
BUG=v8:5476
Review-Url: https://codereview.chromium.org/2399053003
Cr-Commit-Position: refs/heads/master@{#40056}
The implementation of MemorySize with RelocatableInt32Constants is
problematic if MemorySize is placed close to a GrowMemory instruction in
the code. The use of a runtime function guarantees that the order in
which MemorySize and GrowMemory is executed is correct.
R=titzer@chromium.org
BUG=chromium:651961
TEST=mjsunit/regress/wasm/regression-651961
Committed: https://crrev.com/2c12a9a42d454a36fcd2931fa458d72832eeb689
Review-Url: https://codereview.chromium.org/2386183004
Cr-Original-Commit-Position: refs/heads/master@{#39972}
Cr-Commit-Position: refs/heads/master@{#39980}
Reason for revert:
Patch problem
Original issue's description:
> [wasm] Call a runtime function for a MemorySize instruction.
>
> The implementation of MemorySize with RelocatableInt32Constants is
> problematic if MemorySize is placed close to a GrowMemory instruction in
> the code. The use of a runtime function guarantees that the order in
> which MemorySize and GrowMemory is executed is correct.
>
> R=titzer@chromium.org
> BUG=chromium:651961
> TEST=mjsunit/regress/wasm/regression-651961
>
> Committed: https://crrev.com/2c12a9a42d454a36fcd2931fa458d72832eeb689
> Cr-Commit-Position: refs/heads/master@{#39972}
TBR=titzer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:651961
Review-Url: https://codereview.chromium.org/2391223002
Cr-Commit-Position: refs/heads/master@{#39973}
The implementation of MemorySize with RelocatableInt32Constants is
problematic if MemorySize is placed close to a GrowMemory instruction in
the code. The use of a runtime function guarantees that the order in
which MemorySize and GrowMemory is executed is correct.
R=titzer@chromium.org
BUG=chromium:651961
TEST=mjsunit/regress/wasm/regression-651961
Review-Url: https://codereview.chromium.org/2386183004
Cr-Commit-Position: refs/heads/master@{#39972}
This fixes a gc stress bug. We cannot rely on an ordering of
clearing of the weak cells, so we explicitly reset the weak
link to the owning instance, when finalizing a compiled
module. In turn, this serves as a reliable signal when GCs
happen while instantiating, allowing us to correctly link the
new instance.
BUG=chromium:652425
Review-Url: https://codereview.chromium.org/2393443003
Cr-Commit-Position: refs/heads/master@{#39964}
Previously, b6e9f625c1 fixed self-assignment
in parameters to throw. But it failed to deal with the case of
destructuring with defaults. This patch extends that previous approach
to always treat the end of a parameter as its initializer position,
whether it has an initializer or not.
This is the minimal change to make it easy to merge; a follow-up
will rename the field of Parameter from "initializer_end_position"
to "end_position".
BUG=v8:5454
Review-Url: https://codereview.chromium.org/2390943002
Cr-Commit-Position: refs/heads/master@{#39962}
In some (rare) cases, the context depth passed to a dynamic variable lookup can
be zero. In these cases, the fast path for the lookup (i.e. load from context or
global) can always be taken, as there is no need to search the current context.
However, with no slow path checks, the bytecode graph builder had a null
environment for the slow path, causing segfaults when this graph was built.
This patch adds a null check for the slow path environment, and skips building
the slow path if the environment is null.
BUG=chromium:652186
Review-Url: https://codereview.chromium.org/2385123002
Cr-Commit-Position: refs/heads/master@{#39949}
Currently the parameter is first parsed as a reference, and then translated into a parameter. The reference stays around though, and gets resolved to the parameter. That automatically creates a use. Now that I drop all unresolved references when we abort preparsing, that also drops the unresolved reference.
Instead, mark the variable as used when its marked as forced context allocation. That's what happens in almost all other cases.
This raises the question: does it really make sense to parse parameters this ways? It seems pretty generic, but neither fast nor memory-efficient ... Did I misunderstand something?
Just land if you think the CL looks good as is.
BUG=chromium:651613
Review-Url: https://codereview.chromium.org/2386623002
Cr-Commit-Position: refs/heads/master@{#39935}
Making new nodes inside of exception-handled blocks fiddles around with the
current environment to merge the exception paths. In particular, the current
environment pointer is mutated. This patch ensures that when we merge the fast
and slow paths of the LdaContextLookup, we actually merge the correct
environment and do not accidentally merge the exceptional environment.
BUG=chromium:651394
Review-Url: https://codereview.chromium.org/2379043002
Cr-Commit-Position: refs/heads/master@{#39878}
The module size is encoded as a HeapNumber, and needs to be
explicitly cloned.
BUG=chromium:647649
Review-Url: https://codereview.chromium.org/2347333002
Cr-Commit-Position: refs/heads/master@{#39845}
TurboFan lowering (see [0]) of ConsString creation cannot ensure that
the first part of the cons string is non-empty without introducing a phi
and negatively impacting performance.
This modifies ConsStringIterator to allow empty first parts of
ConsStrings.
BUG=v8:5440
Review-Url: https://codereview.chromium.org/2377983002
Cr-Commit-Position: refs/heads/master@{#39817}
Even after https://codereview.chromium.org/2371963002 we still did not
always store a Smi into the JSTypedArray::length field, the runtime
function %TypedArrayInitializeFromArrayLike was still storing whatever
it got from the JavaScript code, which is highly dependent on internal
decisions of the ICs and the representation selection in the optimizing
compilers, so that's pretty fragile.
R=verwaest@chromium.org
BUG=chromium:650933
Review-Url: https://codereview.chromium.org/2377943002
Cr-Commit-Position: refs/heads/master@{#39802}
The call to RegExpSubclassExec may refer to a different exec method
since splitter is newly constructed previously to the call.
BUG=v8:5351
Review-Url: https://codereview.chromium.org/2370733003
Cr-Commit-Position: refs/heads/master@{#39774}
This makes sure the {EscapeAnalysisReducer} inserts proper {TypeGuard}
nodes if the replacement node is not a subtype of the original node.
This happens predominantly for code that has been made unreachable by
type checks.
R=jarin@chromium.org
TEST=mjsunit/regress/regress-crbug-640497
BUG=chromium:640497
Review URL: https://codereview.chromium.org/2363573003 .
Cr-Commit-Position: refs/heads/master@{#39656}
This add support for ConsString objects allocated inline to the escape
analysis pass. The raw hash field in such strings needs special handling
similar to existing raw fields. This also contains materialization code
within the deoptimizer as usual.
R=bmeurer@chromium.org
TEST=mjsunit/regress/regress-crbug-648737
BUG=chromium:648737
Review-Url: https://codereview.chromium.org/2357153002
Cr-Commit-Position: refs/heads/master@{#39594}
This removes an optimization from the code generator that tries to
materialize certain constants (i.e. context and closure) from the
stackframe when possible. This does not work with Harmony tail calls
which are split into several instructions. There have already been
numerous bugs in this optimization, it is too fragile in its current
form.
R=bmeurer@chromium.org
TEST=mjsunit/regress/regress-crbug-648539
BUG=chromium:648539
Review-Url: https://codereview.chromium.org/2357583003
Cr-Commit-Position: refs/heads/master@{#39583}
This patch ensures that variables like .new_target aren't overwritable
using with scopes. It does this by ensuring that scope analysis does
not consider with scopes (or eval scopes) for such 'synthetic variables',
similarly to how the 'this' variable was already handled.
The patch also adds a DCHECK for the dynamic parallel to this case,
replacing a previous unreachable path for a particular instance.
BUG=v8:5405
Review-Url: https://codereview.chromium.org/2353623002
Cr-Commit-Position: refs/heads/master@{#39567}
This patch uses temporaries rather than unresolved variables for
.promise and .debug_is_active. For .promise, a new field is added
to the FunctionState, similarly to .generator_object. This change
fixes a bug where .promise was locally shadowable by with, affecting
program semantics.
BUG=v8:5405
Review-Url: https://codereview.chromium.org/2359513002
Cr-Commit-Position: refs/heads/master@{#39566}
The implicit assignment to the induction variable in a ForInStatement
has been ignored by the AST loop assignment analysis. This was hidden
for cases where the parser introduced a ".for" temporary, but triggers
when the variable is declared outside the loop.
R=bmeurer@chromium.org
TEST=mjsunit/regress/regress-crbug-647887
BUG=chromium:647887
Review-Url: https://codereview.chromium.org/2356733002
Cr-Commit-Position: refs/heads/master@{#39551}
Crankshaft just unconditionally deoptimizes the code when the length of
a string addition result would overflow. In order to protect against
deopt loops we insert a global protector cell.
We will use the same mechanism for inlining certain string additions
into TurboFan as well, and protecting against overflow (we will also
extend this to deal with String.prototype.concat and friends once we
get there).
BUG=v8:5404
R=jarin@chromium.org,hpayer@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.v8:v8_linux64_msan_rel
Committed: https://crrev.com/cb19257a926a55209a6d6858ce26d51a0447ba71
Review-Url: https://codereview.chromium.org/2348293002
Cr-Original-Commit-Position: refs/heads/master@{#39511}
Cr-Commit-Position: refs/heads/master@{#39525}
