New issue
Advanced search Search tips

Issue 829790 link

Starred by 1 user
Status: Fixed
Owner:
ricea@chromium.org
Closed: Jun 2018
Cc:
yhirano@chromium.org
jkummerow@chromium.org
domenic@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 806644
issue 849312



Sign in to add a comment

Eliminate use of CallExtraOrCrash from ReadableStreamOperations

Project Member Reported by ricea@chromium.org, Apr 6 2018 
ReadableStreamOperations.cpp uses CallExtraOrCrash 11 times. This will crash the renderer process if an exception occurs. This appears reasonable because the Javascript in question looks like it can't throw an exception.

Unfortunately, any Javascript function in Blink can throw an exception. I know of two ways to do it:

1. Shutdown the Worker while the function is executing. A special internal exception is thrown to leave V8 code.
2. Call the function with inadequate stack space so it overflows the stack.

There are probably more, but I think they are all "extreme" cases when things are shutting down or failing anyway, so simply not doing anything or passing on the exception is fine.

The difficult part is when ReadableStreamOperations::CreateReadableStream is called by the BodyStreamBuffer constructor during construction of a Response from the network. In this case we probably can't throw. It looks like BodyStreamBuffer::Stream() can deal safely with the stream not being set, so maybe just not setting it if construction failed is the best option.

Comment 1 by ricea@chromium.org, Apr 6 2018

Blocking: 806644

Comment 2 by jkummerow@chromium.org, Apr 16 2018

Cc: jkummerow@chromium.org
Labels: -Pri-2 Pri-1
See also  issue 832202 , where it doesn't look like a pathological corner case. Bumping priority.

Repro in the wild: https://pwp.dassur.ma/ (crashes immediately after load)

go/crash/d22ddc098153fb94

Debug build stacktrace:

(gdb) bt
#0  base::debug::(anonymous namespace)::DebugBreak () at ../../base/debug/debugger_posix.cc:239
#1  0x00007fd4e0c158c8 in base::debug::BreakDebugger () at ../../base/debug/debugger_posix.cc:258
#2  0x00007fd4e0ca4684 in logging::LogMessage::~LogMessage (this=0x7fd489bb4e40) at ../../base/logging.cc:855
#3  0x00007fd4cfd55777 in blink::NonThrowableExceptionState::RethrowV8Exception (this=0x7fd489bb5240) at ../../third_party/blink/renderer/bindings/core/v8/exception_state.cc:202
#4  0x00007fd4d1253002 in blink::ReadableStreamOperations::GetReader (script_state=0x276856bfae30, stream=..., es=...) at ../../third_party/blink/renderer/core/streams/readable_stream_operations.cc:56
#5  0x00007fd4d06def96 in blink::BodyStreamBuffer::CloseAndLockAndDisturb (this=0x108bdab78ab8) at ../../third_party/blink/renderer/core/fetch/body_stream_buffer.cc:319
#6  0x00007fd4d06df92a in blink::BodyStreamBuffer::ReleaseHandle (this=0x108bdab78ab8) at ../../third_party/blink/renderer/core/fetch/body_stream_buffer.cc:435
#7  0x00007fd4d06df588 in blink::BodyStreamBuffer::StartLoading (this=0x108bdab78ab8, loader=0x272193239e0, client=0x27219323a28) at ../../third_party/blink/renderer/core/fetch/body_stream_buffer.cc:202
#8  0x00007fd4ccb2d40e in blink::Cache::PutImpl (this=0x13b9eed61d78, script_state=0x276856bfae30, requests=..., responses=...) at ../../third_party/blink/renderer/modules/cachestorage/cache.cc:777
#9  0x00007fd4ccb31380 in blink::Cache::FetchResolvedForAdd::Call (this=0x13b9eed63350, value=...) at ../../third_party/blink/renderer/modules/cachestorage/cache.cc:255
#10 0x00007fd4cfd7de34 in blink::ScriptFunction::CallCallback (args=...) at ../../third_party/blink/renderer/bindings/core/v8/script_function.cc:32
#11 0x00007fd4d30c8333 in v8::internal::FunctionCallbackArguments::Call (this=0x7fd489bb6600, handler=0xb5b173e4c39) at ../../v8/src/api-arguments-inl.h:93
#12 0x00007fd4d30c6161 in v8::internal::(anonymous namespace)::HandleApiCallHelper<false> (isolate=0x3a621ca84020, function=..., new_target=..., fun_data=..., receiver=..., args=...) at ../../v8/src/builtins/builtins-api.cc:107
#13 0x00007fd4d30c486c in v8::internal::Builtin_Impl_HandleApiCall (args=..., isolate=0x3a621ca84020) at ../../v8/src/builtins/builtins-api.cc:137
#14 0x00007fd4d30c441d in v8::internal::Builtin_HandleApiCall (args_length=6, args_object=0x7fd489bb6850, isolate=0x3a621ca84020) at ../../v8/src/builtins/builtins-api.cc:125

Comment 3 by ricea@chromium.org, Apr 18 2018 

 Issue 832202  is being fixed by disabling PWAFullCodeCache on stable. In that issue a piece of code was actually violating its invariants, so the CHECK() crash was "correct" in some sense.

Comment 4 by ricea@chromium.org, Jun 1 2018 

I've been writing tests in preparation for fixing this. It's an interesting endeavour. While getting it to crash is easy, getting it to crash where you want is hard. For example, I haven't managed to trigger a crash in GetReader() because all code paths call IsDisturbed() or IsLocked() first.

I will probably settle for just testing the things that I *can* test, and then stop using CallExtraOrCrash.

Comment 5 by ricea@chromium.org, Jun 5 2018 

Related issue:

BodyStreamBuffer::ReleaseHandle() calls ReadableStreamOperations::GetReader() inside a NonThrowableExceptionState block, asserting that GetReader() cannot throw an exception. But it *can* throw, in which case we crash a bit later inside the ReadableStreamBytesConsumer constructor with a null deference error in v8::internal::GlobalHandles::MakeWeak.

Comment 6 by ricea@chromium.org, Jun 5 2018

Blocking: 849312
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 18 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1977b7933753c18838cf2a5b05a18738e320df48

commit 1977b7933753c18838cf2a5b05a18738e320df48
Author: Adam Rice <ricea@chromium.org>
Date: Mon Jun 18 15:06:55 2018

Remove blink::CallExtraOrCrash

CallExtraOrCrash is a function to call into V8 Extras and crash if it
fails. Unfortunately, calling into V8 Extras can fail under normal
conditions such as stack overflow and Worker destruction, causing a lot
of unnecessary crashes. Remove CallExtraOrCrash.

This is a stopgap change designed to be mergable to version 68. It
avoids major code restructuring but as a result lacks robust
encapsulation.

The general strategy is to pass a boolean pointer to functions in
ReadableStreamOperations that call into V8 Extras. If the internal call
fails, the boolean is set to true, and the call returns a default value.

BodyStreamBuffer keeps the boolean as member variable. Once a call to a
V8 extra function fails, the boolean is set and no more calls will be
made. This is to avoid us taking action based on information that is
incorrect.

For Tee() and the from-stream version of the BodyStreamBuffer
constructor, we use an ExceptionState object instead. In these cases, it
is straightforward to pass through ExceptionState object from the Blink
bindings. This allows the failure to be propagated safely back to
Javascript.

Conversely, DefaultReaderRead() has been switched from using
ExceptionState to using an error-signalling bool. This is because its
callers do not have access to a real ExceptionState object. Previously
callers used a NonThrowableExceptionState, but this was not suitable as
DefaultReaderRead() can throw.

BodyStreamBuffer mostly wraps access to ReadableStreamOperations, so
some degree of encapsulation has been retained.

7 new layout tests exercise each of the deterministically reachable
crashes, using stack overflows. Because they depend on stack layout and
whether DCHECKs are enabled, they only hit the crashes in particular
environments.

Bug:  829790 , 849312
Change-Id: I47481b33a47b418dc6916e3b4311e60b5fd89e3d
Reviewed-on: https://chromium-review.googlesource.com/1097047
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568009}
[add] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct-from-stream.html
[add] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct.html
[add] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-disturbed.html
[add] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-locked.html
[add] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-release.html
[add] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-tee.html
[add] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/release-handle-crash.html
[add] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/resources/stack-overflow.js
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/bindings/core/v8/v8_script_runner.h
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/body_stream_buffer.cc
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/body_stream_buffer.h
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/body_stream_buffer_test.cc
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/fetch_request_data.cc
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/fetch_request_data.h
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/fetch_response_data.cc
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/fetch_response_data.h
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer_test.cc
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/request.cc
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/request.h
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/fetch/response.cc
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/streams/readable_stream_operations.cc
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/streams/readable_stream_operations.h
[modify] https://crrev.com/1977b7933753c18838cf2a5b05a18738e320df48/third_party/blink/renderer/core/streams/readable_stream_operations_test.cc

Comment 8 by ricea@chromium.org, Jun 18 2018

Status: Fixed (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 19 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683

commit e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Tue Jun 19 01:20:37 2018

Revert "Remove blink::CallExtraOrCrash"

This reverts commit 1977b7933753c18838cf2a5b05a18738e320df48.

Reason for revert: breaks WebKit Linux Trusty MSAN

Tests seem to time out since this CL:

https://test-results.appspot.com/data/layout_results/WebKit_Linux_Trusty_MSAN/8393/layout-test-results/results.html

https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20MSAN/8393

Original change's description:
> Remove blink::CallExtraOrCrash
> 
> CallExtraOrCrash is a function to call into V8 Extras and crash if it
> fails. Unfortunately, calling into V8 Extras can fail under normal
> conditions such as stack overflow and Worker destruction, causing a lot
> of unnecessary crashes. Remove CallExtraOrCrash.
> 
> This is a stopgap change designed to be mergable to version 68. It
> avoids major code restructuring but as a result lacks robust
> encapsulation.
> 
> The general strategy is to pass a boolean pointer to functions in
> ReadableStreamOperations that call into V8 Extras. If the internal call
> fails, the boolean is set to true, and the call returns a default value.
> 
> BodyStreamBuffer keeps the boolean as member variable. Once a call to a
> V8 extra function fails, the boolean is set and no more calls will be
> made. This is to avoid us taking action based on information that is
> incorrect.
> 
> For Tee() and the from-stream version of the BodyStreamBuffer
> constructor, we use an ExceptionState object instead. In these cases, it
> is straightforward to pass through ExceptionState object from the Blink
> bindings. This allows the failure to be propagated safely back to
> Javascript.
> 
> Conversely, DefaultReaderRead() has been switched from using
> ExceptionState to using an error-signalling bool. This is because its
> callers do not have access to a real ExceptionState object. Previously
> callers used a NonThrowableExceptionState, but this was not suitable as
> DefaultReaderRead() can throw.
> 
> BodyStreamBuffer mostly wraps access to ReadableStreamOperations, so
> some degree of encapsulation has been retained.
> 
> 7 new layout tests exercise each of the deterministically reachable
> crashes, using stack overflows. Because they depend on stack layout and
> whether DCHECKs are enabled, they only hit the crashes in particular
> environments.
> 
> Bug:  829790 , 849312
> Change-Id: I47481b33a47b418dc6916e3b4311e60b5fd89e3d
> Reviewed-on: https://chromium-review.googlesource.com/1097047
> Commit-Queue: Adam Rice <ricea@chromium.org>
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#568009}

TBR=ricea@chromium.org,yukishiino@chromium.org,yhirano@chromium.org

Change-Id: I4234c9a5ebb7bed44f3f0a66b266b65a80efb295
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  829790 , 849312
Reviewed-on: https://chromium-review.googlesource.com/1105577
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568279}
[delete] https://crrev.com/c008bf657dbc01fadc4ff1ea3df21f9cf237569a/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct-from-stream.html
[delete] https://crrev.com/c008bf657dbc01fadc4ff1ea3df21f9cf237569a/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct.html
[delete] https://crrev.com/c008bf657dbc01fadc4ff1ea3df21f9cf237569a/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-disturbed.html
[delete] https://crrev.com/c008bf657dbc01fadc4ff1ea3df21f9cf237569a/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-locked.html
[delete] https://crrev.com/c008bf657dbc01fadc4ff1ea3df21f9cf237569a/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-release.html
[delete] https://crrev.com/c008bf657dbc01fadc4ff1ea3df21f9cf237569a/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-tee.html
[delete] https://crrev.com/c008bf657dbc01fadc4ff1ea3df21f9cf237569a/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/release-handle-crash.html
[delete] https://crrev.com/c008bf657dbc01fadc4ff1ea3df21f9cf237569a/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/resources/stack-overflow.js
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/bindings/core/v8/v8_script_runner.h
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/body_stream_buffer.cc
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/body_stream_buffer.h
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/body_stream_buffer_test.cc
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/fetch_request_data.cc
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/fetch_request_data.h
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/fetch_response_data.cc
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/fetch_response_data.h
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer_test.cc
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/request.cc
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/request.h
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/fetch/response.cc
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/streams/readable_stream_operations.cc
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/streams/readable_stream_operations.h
[modify] https://crrev.com/e385ffd32f9a8f98c0b441aa8dbdefafe1e6d683/third_party/blink/renderer/core/streams/readable_stream_operations_test.cc
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 19 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/035ce793d12dbd436c609823f7160364110c9775

commit 035ce793d12dbd436c609823f7160364110c9775
Author: Adam Rice <ricea@chromium.org>
Date: Tue Jun 19 12:01:29 2018

Reland "Remove blink::CallExtraOrCrash"

The only changes in this reland are to add additional Slow expectations, since
two of the layout tests are very slow on MSAN bots. See  crbug.com/853977 .

Originally reviewed at
https://chromium-review.googlesource.com/c/chromium/src/+/1097047.

Original description:

CallExtraOrCrash is a function to call into V8 Extras and crash if it
fails. Unfortunately, calling into V8 Extras can fail under normal
conditions such as stack overflow and Worker destruction, causing a lot
of unnecessary crashes. Remove CallExtraOrCrash.

This is a stopgap change designed to be mergable to version 68. It
avoids major code restructuring but as a result lacks robust
encapsulation.

The general strategy is to pass a boolean pointer to functions in
ReadableStreamOperations that call into V8 Extras. If the internal call
fails, the boolean is set to true, and the call returns a default value.

BodyStreamBuffer keeps the boolean as member variable. Once a call to a
V8 extra function fails, the boolean is set and no more calls will be
made. This is to avoid us taking action based on information that is
incorrect.

For Tee() and the from-stream version of the BodyStreamBuffer
constructor, we use an ExceptionState object instead. In these cases, it
is straightforward to pass through ExceptionState object from the Blink
bindings. This allows the failure to be propagated safely back to
Javascript.

Conversely, DefaultReaderRead() has been switched from using
ExceptionState to using an error-signalling bool. This is because its
callers do not have access to a real ExceptionState object. Previously
callers used a NonThrowableExceptionState, but this was not suitable as
DefaultReaderRead() can throw.

BodyStreamBuffer mostly wraps access to ReadableStreamOperations, so
some degree of encapsulation has been retained.

7 new layout tests exercise each of the deterministically reachable
crashes, using stack overflows. Because they depend on stack layout and
whether DCHECKs are enabled, they only hit the crashes in particular
environments.

TBR=yhirano@chromium.org,yukishiino@chromium.org

Bug:  829790 , 849312,  853977 
Change-Id: I308ee14ba063f25742eae3cb4c889c3bd50b38d3
Reviewed-on: https://chromium-review.googlesource.com/1105678
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568408}
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/WebKit/LayoutTests/SlowTests
[add] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct-from-stream.html
[add] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct.html
[add] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-disturbed.html
[add] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-locked.html
[add] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-release.html
[add] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-tee.html
[add] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/release-handle-crash.html
[add] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/resources/stack-overflow.js
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/bindings/core/v8/v8_script_runner.h
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/body_stream_buffer.cc
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/body_stream_buffer.h
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/body_stream_buffer_test.cc
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/fetch_request_data.cc
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/fetch_request_data.h
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/fetch_response_data.cc
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/fetch_response_data.h
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer_test.cc
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/request.cc
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/request.h
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/fetch/response.cc
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/streams/readable_stream_operations.cc
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/streams/readable_stream_operations.h
[modify] https://crrev.com/035ce793d12dbd436c609823f7160364110c9775/third_party/blink/renderer/core/streams/readable_stream_operations_test.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 25 2018

Labels: merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/17254140a1f41e6057fcb22885c6178fa82affad

commit 17254140a1f41e6057fcb22885c6178fa82affad
Author: Adam Rice <ricea@chromium.org>
Date: Mon Jun 25 09:14:36 2018

Reland "Remove blink::CallExtraOrCrash" (M68 cherry-pick)

The only changes in this reland are to add additional Slow expectations, since
two of the layout tests are very slow on MSAN bots. See  crbug.com/853977 .

Originally reviewed at
https://chromium-review.googlesource.com/c/chromium/src/+/1097047.

Original description:

CallExtraOrCrash is a function to call into V8 Extras and crash if it
fails. Unfortunately, calling into V8 Extras can fail under normal
conditions such as stack overflow and Worker destruction, causing a lot
of unnecessary crashes. Remove CallExtraOrCrash.

This is a stopgap change designed to be mergable to version 68. It
avoids major code restructuring but as a result lacks robust
encapsulation.

The general strategy is to pass a boolean pointer to functions in
ReadableStreamOperations that call into V8 Extras. If the internal call
fails, the boolean is set to true, and the call returns a default value.

BodyStreamBuffer keeps the boolean as member variable. Once a call to a
V8 extra function fails, the boolean is set and no more calls will be
made. This is to avoid us taking action based on information that is
incorrect.

For Tee() and the from-stream version of the BodyStreamBuffer
constructor, we use an ExceptionState object instead. In these cases, it
is straightforward to pass through ExceptionState object from the Blink
bindings. This allows the failure to be propagated safely back to
Javascript.

Conversely, DefaultReaderRead() has been switched from using
ExceptionState to using an error-signalling bool. This is because its
callers do not have access to a real ExceptionState object. Previously
callers used a NonThrowableExceptionState, but this was not suitable as
DefaultReaderRead() can throw.

BodyStreamBuffer mostly wraps access to ReadableStreamOperations, so
some degree of encapsulation has been retained.

7 new layout tests exercise each of the deterministically reachable
crashes, using stack overflows. Because they depend on stack layout and
whether DCHECKs are enabled, they only hit the crashes in particular
environments.

TBR=ricea@chromium.org, yhirano@chromium.org, yukishiino@chromium.org

(cherry picked from commit 035ce793d12dbd436c609823f7160364110c9775)

Bug:  829790 , 849312,  853977 
Change-Id: I308ee14ba063f25742eae3cb4c889c3bd50b38d3
Reviewed-on: https://chromium-review.googlesource.com/1105678
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#568408}
Reviewed-on: https://chromium-review.googlesource.com/1113260
Cr-Commit-Position: refs/branch-heads/3440@{#507}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/WebKit/LayoutTests/SlowTests
[add] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct-from-stream.html
[add] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct.html
[add] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-disturbed.html
[add] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-locked.html
[add] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-release.html
[add] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-tee.html
[add] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/release-handle-crash.html
[add] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/resources/stack-overflow.js
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/bindings/core/v8/v8_script_runner.h
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/body_stream_buffer.cc
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/body_stream_buffer.h
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/body_stream_buffer_test.cc
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/fetch_request_data.cc
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/fetch_request_data.h
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/fetch_response_data.cc
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/fetch_response_data.h
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer_test.cc
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/request.cc
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/request.h
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/fetch/response.cc
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/streams/readable_stream_operations.cc
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/streams/readable_stream_operations.h
[modify] https://crrev.com/17254140a1f41e6057fcb22885c6178fa82affad/third_party/blink/renderer/core/streams/readable_stream_operations_test.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 25 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2

commit ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2
Author: John Budorick <jbudorick@chromium.org>
Date: Mon Jun 25 18:45:04 2018

Revert "Reland "Remove blink::CallExtraOrCrash" (M68 cherry-pick)"

This reverts commit 17254140a1f41e6057fcb22885c6178fa82affad.

Reason for revert: This breaks M68 compile; it uses third_party/blink/renderer/platform/bindings/exception_state.h, which was moved to that location in https://chromium.googlesource.com/chromium/src/+/e51ec048e8a5b676f9ba02a8216bb5eedc7ce87c

Original change's description:
> Reland "Remove blink::CallExtraOrCrash" (M68 cherry-pick)
> 
> The only changes in this reland are to add additional Slow expectations, since
> two of the layout tests are very slow on MSAN bots. See  crbug.com/853977 .
> 
> Originally reviewed at
> https://chromium-review.googlesource.com/c/chromium/src/+/1097047.
> 
> Original description:
> 
> CallExtraOrCrash is a function to call into V8 Extras and crash if it
> fails. Unfortunately, calling into V8 Extras can fail under normal
> conditions such as stack overflow and Worker destruction, causing a lot
> of unnecessary crashes. Remove CallExtraOrCrash.
> 
> This is a stopgap change designed to be mergable to version 68. It
> avoids major code restructuring but as a result lacks robust
> encapsulation.
> 
> The general strategy is to pass a boolean pointer to functions in
> ReadableStreamOperations that call into V8 Extras. If the internal call
> fails, the boolean is set to true, and the call returns a default value.
> 
> BodyStreamBuffer keeps the boolean as member variable. Once a call to a
> V8 extra function fails, the boolean is set and no more calls will be
> made. This is to avoid us taking action based on information that is
> incorrect.
> 
> For Tee() and the from-stream version of the BodyStreamBuffer
> constructor, we use an ExceptionState object instead. In these cases, it
> is straightforward to pass through ExceptionState object from the Blink
> bindings. This allows the failure to be propagated safely back to
> Javascript.
> 
> Conversely, DefaultReaderRead() has been switched from using
> ExceptionState to using an error-signalling bool. This is because its
> callers do not have access to a real ExceptionState object. Previously
> callers used a NonThrowableExceptionState, but this was not suitable as
> DefaultReaderRead() can throw.
> 
> BodyStreamBuffer mostly wraps access to ReadableStreamOperations, so
> some degree of encapsulation has been retained.
> 
> 7 new layout tests exercise each of the deterministically reachable
> crashes, using stack overflows. Because they depend on stack layout and
> whether DCHECKs are enabled, they only hit the crashes in particular
> environments.
> 
> TBR=ricea@chromium.org, yhirano@chromium.org, yukishiino@chromium.org
> 
> (cherry picked from commit 035ce793d12dbd436c609823f7160364110c9775)
> 
> Bug:  829790 , 849312,  853977 
> Change-Id: I308ee14ba063f25742eae3cb4c889c3bd50b38d3
> Reviewed-on: https://chromium-review.googlesource.com/1105678
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Commit-Queue: Adam Rice <ricea@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#568408}
> Reviewed-on: https://chromium-review.googlesource.com/1113260
> Cr-Commit-Position: refs/branch-heads/3440@{#507}
> Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}

TBR=ricea@chromium.org,yukishiino@chromium.org,yhirano@chromium.org

Change-Id: I263bd78fd71b8de3e72c598e2b4090e20f4bcd4f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  829790 , 849312,  853977 
Reviewed-on: https://chromium-review.googlesource.com/1113841
Reviewed-by: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#513}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/WebKit/LayoutTests/SlowTests
[delete] https://crrev.com/7bdb72af41fd3ed1771b14e43ff30947d396fdca/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct-from-stream.html
[delete] https://crrev.com/7bdb72af41fd3ed1771b14e43ff30947d396fdca/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct.html
[delete] https://crrev.com/7bdb72af41fd3ed1771b14e43ff30947d396fdca/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-disturbed.html
[delete] https://crrev.com/7bdb72af41fd3ed1771b14e43ff30947d396fdca/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-locked.html
[delete] https://crrev.com/7bdb72af41fd3ed1771b14e43ff30947d396fdca/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-release.html
[delete] https://crrev.com/7bdb72af41fd3ed1771b14e43ff30947d396fdca/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-tee.html
[delete] https://crrev.com/7bdb72af41fd3ed1771b14e43ff30947d396fdca/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/release-handle-crash.html
[delete] https://crrev.com/7bdb72af41fd3ed1771b14e43ff30947d396fdca/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/resources/stack-overflow.js
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/bindings/core/v8/v8_script_runner.h
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/body_stream_buffer.cc
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/body_stream_buffer.h
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/body_stream_buffer_test.cc
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/fetch_request_data.cc
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/fetch_request_data.h
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/fetch_response_data.cc
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/fetch_response_data.h
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer_test.cc
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/request.cc
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/request.h
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/fetch/response.cc
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/streams/readable_stream_operations.cc
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/streams/readable_stream_operations.h
[modify] https://crrev.com/ad4553f6e0b2c8ee9b791d1ec3d885b1a4ffc5c2/third_party/blink/renderer/core/streams/readable_stream_operations_test.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 26 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6880fe75a2995b99bce58a418113685b5f76fe9

commit e6880fe75a2995b99bce58a418113685b5f76fe9
Author: Adam Rice <ricea@chromium.org>
Date: Tue Jun 26 04:15:19 2018

M68 Reland "Remove blink::CallExtraOrCrash" (includes fixed)

This is functionally the same as the original cherry-pick
17254140a1f41e6057fcb22885c6178fa82affad but with compile fixes.

Also removed
virtual/outofblink-cors/http/tests/fetch/chromium/call-extra-crash-tee.html
and
virtual/outofblink-cors/http/tests/fetch/chromium/release-handle-crash.html
from SlowTests as these tests are not run in M68.

Original description:

CallExtraOrCrash is a function to call into V8 Extras and crash if it
fails. Unfortunately, calling into V8 Extras can fail under normal
conditions such as stack overflow and Worker destruction, causing a lot
of unnecessary crashes. Remove CallExtraOrCrash.

This is a stopgap change designed to be mergable to version 68. It
avoids major code restructuring but as a result lacks robust
encapsulation.

The general strategy is to pass a boolean pointer to functions in
ReadableStreamOperations that call into V8 Extras. If the internal call
fails, the boolean is set to true, and the call returns a default value.

BodyStreamBuffer keeps the boolean as member variable. Once a call to a
V8 extra function fails, the boolean is set and no more calls will be
made. This is to avoid us taking action based on information that is
incorrect.

For Tee() and the from-stream version of the BodyStreamBuffer
constructor, we use an ExceptionState object instead. In these cases, it
is straightforward to pass through ExceptionState object from the Blink
bindings. This allows the failure to be propagated safely back to
Javascript.

Conversely, DefaultReaderRead() has been switched from using
ExceptionState to using an error-signalling bool. This is because its
callers do not have access to a real ExceptionState object. Previously
callers used a NonThrowableExceptionState, but this was not suitable as
DefaultReaderRead() can throw.

BodyStreamBuffer mostly wraps access to ReadableStreamOperations, so
some degree of encapsulation has been retained.

7 new layout tests exercise each of the deterministically reachable
crashes, using stack overflows. Because they depend on stack layout and
whether DCHECKs are enabled, they only hit the crashes in particular
environments.

TBR=ricea@chromium.org, yhirano@chromium.org, yukishiino@chromium.org

(cherry picked from commit 035ce793d12dbd436c609823f7160364110c9775)

Bug:  829790 , 849312,  853977 
Change-Id: I730a6c42da773afd9653bb2985916ffe92bb81c6
Reviewed-on: https://chromium-review.googlesource.com/1114404
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#522}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/WebKit/LayoutTests/SlowTests
[add] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct-from-stream.html
[add] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct.html
[add] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-disturbed.html
[add] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-locked.html
[add] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-release.html
[add] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-tee.html
[add] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/release-handle-crash.html
[add] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/resources/stack-overflow.js
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/bindings/core/v8/v8_script_runner.h
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/body_stream_buffer.cc
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/body_stream_buffer.h
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/body_stream_buffer_test.cc
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/fetch_request_data.cc
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/fetch_request_data.h
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/fetch_response_data.cc
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/fetch_response_data.h
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer_test.cc
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/request.cc
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/request.h
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/fetch/response.cc
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/streams/readable_stream_operations.cc
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/streams/readable_stream_operations.h
[modify] https://crrev.com/e6880fe75a2995b99bce58a418113685b5f76fe9/third_party/blink/renderer/core/streams/readable_stream_operations_test.cc
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 27 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ba3a6ade208a487e3b02544548d9eabf30cc4a6

commit 0ba3a6ade208a487e3b02544548d9eabf30cc4a6
Author: Juan Antonio Navarro Pérez <perezju@chromium.org>
Date: Wed Jun 27 11:07:23 2018

Revert "M68 Reland "Remove blink::CallExtraOrCrash" (includes fixed)"

This reverts commit e6880fe75a2995b99bce58a418113685b5f76fe9.

Reason for revert: Breaks continuous release bots, blocking beta release.

Original change's description:
> M68 Reland "Remove blink::CallExtraOrCrash" (includes fixed)
> 
> This is functionally the same as the original cherry-pick
> 17254140a1f41e6057fcb22885c6178fa82affad but with compile fixes.
> 
> Also removed
> virtual/outofblink-cors/http/tests/fetch/chromium/call-extra-crash-tee.html
> and
> virtual/outofblink-cors/http/tests/fetch/chromium/release-handle-crash.html
> from SlowTests as these tests are not run in M68.
> 
> Original description:
> 
> CallExtraOrCrash is a function to call into V8 Extras and crash if it
> fails. Unfortunately, calling into V8 Extras can fail under normal
> conditions such as stack overflow and Worker destruction, causing a lot
> of unnecessary crashes. Remove CallExtraOrCrash.
> 
> This is a stopgap change designed to be mergable to version 68. It
> avoids major code restructuring but as a result lacks robust
> encapsulation.
> 
> The general strategy is to pass a boolean pointer to functions in
> ReadableStreamOperations that call into V8 Extras. If the internal call
> fails, the boolean is set to true, and the call returns a default value.
> 
> BodyStreamBuffer keeps the boolean as member variable. Once a call to a
> V8 extra function fails, the boolean is set and no more calls will be
> made. This is to avoid us taking action based on information that is
> incorrect.
> 
> For Tee() and the from-stream version of the BodyStreamBuffer
> constructor, we use an ExceptionState object instead. In these cases, it
> is straightforward to pass through ExceptionState object from the Blink
> bindings. This allows the failure to be propagated safely back to
> Javascript.
> 
> Conversely, DefaultReaderRead() has been switched from using
> ExceptionState to using an error-signalling bool. This is because its
> callers do not have access to a real ExceptionState object. Previously
> callers used a NonThrowableExceptionState, but this was not suitable as
> DefaultReaderRead() can throw.
> 
> BodyStreamBuffer mostly wraps access to ReadableStreamOperations, so
> some degree of encapsulation has been retained.
> 
> 7 new layout tests exercise each of the deterministically reachable
> crashes, using stack overflows. Because they depend on stack layout and
> whether DCHECKs are enabled, they only hit the crashes in particular
> environments.
> 
> TBR=ricea@chromium.org, yhirano@chromium.org, yukishiino@chromium.org
> 
> (cherry picked from commit 035ce793d12dbd436c609823f7160364110c9775)
> 
> Bug:  829790 , 849312,  853977 
> Change-Id: I730a6c42da773afd9653bb2985916ffe92bb81c6
> Reviewed-on: https://chromium-review.googlesource.com/1114404
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3440@{#522}
> Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}

TBR=ricea@chromium.org,yukishiino@chromium.org,yhirano@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  829790 , 849312,  853977 , 856902
Change-Id: Ib1207919c376d7e1fe2c392d506dfc56085dc461
Reviewed-on: https://chromium-review.googlesource.com/1116838
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#549}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/WebKit/LayoutTests/SlowTests
[delete] https://crrev.com/1221d1bb03af81ad76f9b6295d44525857b13c2d/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct-from-stream.html
[delete] https://crrev.com/1221d1bb03af81ad76f9b6295d44525857b13c2d/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-construct.html
[delete] https://crrev.com/1221d1bb03af81ad76f9b6295d44525857b13c2d/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-disturbed.html
[delete] https://crrev.com/1221d1bb03af81ad76f9b6295d44525857b13c2d/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-is-locked.html
[delete] https://crrev.com/1221d1bb03af81ad76f9b6295d44525857b13c2d/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-release.html
[delete] https://crrev.com/1221d1bb03af81ad76f9b6295d44525857b13c2d/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/call-extra-crash-tee.html
[delete] https://crrev.com/1221d1bb03af81ad76f9b6295d44525857b13c2d/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/release-handle-crash.html
[delete] https://crrev.com/1221d1bb03af81ad76f9b6295d44525857b13c2d/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/resources/stack-overflow.js
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/bindings/core/v8/v8_script_runner.h
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/body_stream_buffer.cc
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/body_stream_buffer.h
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/body_stream_buffer_test.cc
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/fetch_request_data.cc
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/fetch_request_data.h
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/fetch_response_data.cc
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/fetch_response_data.h
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer_test.cc
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/request.cc
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/request.h
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/fetch/response.cc
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/streams/readable_stream_operations.cc
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/streams/readable_stream_operations.h
[modify] https://crrev.com/0ba3a6ade208a487e3b02544548d9eabf30cc4a6/third_party/blink/renderer/core/streams/readable_stream_operations_test.cc
►

Sign in to add a comment