New issue
Advanced search Search tips

Issue 853977 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

webkit_layout_tests failing on chromium.webkit/WebKit Linux Trusty MSAN

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Jun 18 2018

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of holte@google.com

webkit_layout_tests failing on chromium.webkit/WebKit Linux Trusty MSAN

Builders failed on: 
- WebKit Linux Trusty MSAN: 
  https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20MSAN


 

Comment 1 by holte@chromium.org, Jun 18 2018

Owner: ricea@chromium.org
These appear to be new tests added by this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1097047

Comment 2 by ortuno@chromium.org, Jun 19 2018

Revert at: https://crrev.com/c/1105577

Comment 3 by ortuno@chromium.org, Jun 19 2018

Labels: -Sheriff-Chromium
No more instances since the revert. Taking out of sheriff queue.
Project Member

Comment 4 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

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

Labels: Type-Bug-Regression
Status: Fixed (was: Available)
Project Member

Comment 6 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 7 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 8 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 9 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

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 20

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

commit e5e1145d96b45d730a223207f8c52ecaacb5aef1
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Fri Jul 20 04:28:27 2018

[OOR-CORS] Copy base tests' slowness expectation to outofblink-cors-ns

http/tests/fetch/chromium/call-extra-crash-tee.html and
http/tests/fetch/chromium/release-handle-crash.html are marked as slow.
Let's mark virtual/outofblink-cors-ns/ ones as slow as well instead of
marking them as TIMEOUT.

Bug: 862184,  853977 
Change-Id: Ia26c116e5c43912591ce61137abe50473c9e8881
Reviewed-on: https://chromium-review.googlesource.com/1144592
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576779}
[modify] https://crrev.com/e5e1145d96b45d730a223207f8c52ecaacb5aef1/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/e5e1145d96b45d730a223207f8c52ecaacb5aef1/third_party/WebKit/LayoutTests/TestExpectations

Sign in to add a comment