Skip to content

Commit

Permalink
Plumb an ExceptionState& through UnderlyingSourceBase::pull() and sta…
Browse files Browse the repository at this point in the history
…rt()

Change-Id: Ibd5e7da9b6324d68ec17212f6fa873dc96fdc253
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4913922
Reviewed-by: Nidhi Jaju <nidhijaju@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Auto-Submit: Nate Chapin <japhet@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1217063}
  • Loading branch information
natechapin authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent 164b476 commit efd22e4
Show file tree
Hide file tree
Showing 33 changed files with 161 additions and 141 deletions.
33 changes: 16 additions & 17 deletions third_party/blink/renderer/core/fetch/body_stream_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ BodyStreamBuffer::BodyStreamBuffer(
}

scoped_refptr<BlobDataHandle> BodyStreamBuffer::DrainAsBlobDataHandle(
BytesConsumer::BlobSizePolicy policy) {
BytesConsumer::BlobSizePolicy policy,
ExceptionState& exception_state) {
DCHECK(!IsStreamLocked());
DCHECK(!IsStreamDisturbed());
if (IsStreamClosed() || IsStreamErrored() || stream_broken_)
Expand All @@ -207,13 +208,14 @@ scoped_refptr<BlobDataHandle> BodyStreamBuffer::DrainAsBlobDataHandle(
scoped_refptr<BlobDataHandle> blob_data_handle =
consumer_->DrainAsBlobDataHandle(policy);
if (blob_data_handle) {
CloseAndLockAndDisturb();
CloseAndLockAndDisturb(exception_state);
return blob_data_handle;
}
return nullptr;
}

scoped_refptr<EncodedFormData> BodyStreamBuffer::DrainAsFormData() {
scoped_refptr<EncodedFormData> BodyStreamBuffer::DrainAsFormData(
ExceptionState& exception_state) {
DCHECK(!IsStreamLocked());
DCHECK(!IsStreamDisturbed());
if (IsStreamClosed() || IsStreamErrored() || stream_broken_)
Expand All @@ -224,7 +226,7 @@ scoped_refptr<EncodedFormData> BodyStreamBuffer::DrainAsFormData() {

scoped_refptr<EncodedFormData> form_data = consumer_->DrainAsFormData();
if (form_data) {
CloseAndLockAndDisturb();
CloseAndLockAndDisturb(exception_state);
return form_data;
}
return nullptr;
Expand Down Expand Up @@ -359,18 +361,20 @@ void BodyStreamBuffer::OnStateChange() {
GetExecutionContext()->IsContextDestroyed()) {
return;
}
ExceptionState exception_state(script_state_->GetIsolate(),
ExceptionContextType::kUnknown, "", "");

switch (consumer_->GetPublicState()) {
case BytesConsumer::PublicState::kReadableOrWaiting:
break;
case BytesConsumer::PublicState::kClosed:
Close();
Close(exception_state);
return;
case BytesConsumer::PublicState::kErrored:
GetError();
return;
}
ProcessData();
ProcessData(exception_state);
}

void BodyStreamBuffer::ContextDestroyed() {
Expand Down Expand Up @@ -398,15 +402,15 @@ bool BodyStreamBuffer::IsStreamDisturbed() const {
return stream_->IsDisturbed();
}

void BodyStreamBuffer::CloseAndLockAndDisturb() {
void BodyStreamBuffer::CloseAndLockAndDisturb(ExceptionState& exception_state) {
DCHECK(!stream_broken_);

cached_metadata_handler_ = nullptr;

if (IsStreamReadable()) {
// Note that the stream cannot be "draining", because it doesn't have
// the internal buffer.
Close();
Close(exception_state);
}

stream_->LockAndDisturb(script_state_);
Expand Down Expand Up @@ -459,12 +463,10 @@ void BodyStreamBuffer::Abort() {
CancelConsumer();
}

void BodyStreamBuffer::Close() {
void BodyStreamBuffer::Close(ExceptionState& exception_state) {
// Close() can be called during construction, in which case Controller()
// will not be set yet.
if (underlying_byte_source_) {
ExceptionState exception_state(script_state_->GetIsolate(),
ExceptionContextType::kUnknown, "", "");
if (script_state_->ContextIsValid()) {
ScriptState::Scope scope(script_state_);
stream_->CloseStream(script_state_, exception_state);
Expand Down Expand Up @@ -531,7 +533,7 @@ void BodyStreamBuffer::CancelConsumer() {
}
}

void BodyStreamBuffer::ProcessData() {
void BodyStreamBuffer::ProcessData(ExceptionState& exception_state) {
DCHECK(consumer_);
DCHECK(!in_process_data_);

Expand Down Expand Up @@ -580,9 +582,6 @@ void BodyStreamBuffer::ProcessData() {
ScriptState::Scope scope(script_state_);
auto* byte_controller =
To<ReadableByteStreamController>(stream_->GetController());
ExceptionState exception_state(script_state_->GetIsolate(),
ExceptionContextType::kUnknown, "",
"");
if (byob_view) {
ReadableByteStreamController::Respond(
script_state_, byte_controller, available, exception_state);
Expand All @@ -603,7 +602,7 @@ void BodyStreamBuffer::ProcessData() {
}
}
if (result == BytesConsumer::Result::kDone) {
Close();
Close(exception_state);
return;
}
// If |stream_needs_more_| is true, it means that pull is called and
Expand Down Expand Up @@ -689,7 +688,7 @@ BytesConsumer* BodyStreamBuffer::ReleaseHandle(

BytesConsumer* consumer = consumer_.Release();

CloseAndLockAndDisturb();
CloseAndLockAndDisturb(exception_state);

if (is_closed) {
// Note that the stream cannot be "draining", because it doesn't have
Expand Down
11 changes: 6 additions & 5 deletions third_party/blink/renderer/core/fetch/body_stream_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ class CORE_EXPORT BodyStreamBuffer final

// Callable only when neither locked nor disturbed.
scoped_refptr<BlobDataHandle> DrainAsBlobDataHandle(
BytesConsumer::BlobSizePolicy);
scoped_refptr<EncodedFormData> DrainAsFormData();
BytesConsumer::BlobSizePolicy,
ExceptionState&);
scoped_refptr<EncodedFormData> DrainAsFormData(ExceptionState&);
void DrainAsChunkedDataPipeGetter(
ScriptState*,
mojo::PendingReceiver<network::mojom::blink::ChunkedDataPipeGetter>,
Expand Down Expand Up @@ -102,7 +103,7 @@ class CORE_EXPORT BodyStreamBuffer final

// Closes the stream if necessary, and then locks and disturbs it. Should not
// be called if |stream_broken_| is true.
void CloseAndLockAndDisturb();
void CloseAndLockAndDisturb(ExceptionState&);

bool IsAborted();

Expand Down Expand Up @@ -140,11 +141,11 @@ class CORE_EXPORT BodyStreamBuffer final

BytesConsumer* ReleaseHandle(ExceptionState&);
void Abort();
void Close();
void Close(ExceptionState&);
void GetError();
void RaiseOOMError();
void CancelConsumer();
void ProcessData();
void ProcessData(ExceptionState&);
void EndLoading();
void StopLoading();

Expand Down
18 changes: 11 additions & 7 deletions third_party/blink/renderer/core/fetch/body_stream_buffer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ TEST_P(BodyStreamBufferTest, DrainAsBlobDataHandle) {
EXPECT_EQ(side_data_blob, buffer->GetSideDataBlobForTest());
scoped_refptr<BlobDataHandle> output_blob_data_handle =
buffer->DrainAsBlobDataHandle(
BytesConsumer::BlobSizePolicy::kAllowBlobWithInvalidSize);
BytesConsumer::BlobSizePolicy::kAllowBlobWithInvalidSize,
ASSERT_NO_EXCEPTION);

EXPECT_TRUE(buffer->IsStreamLocked());
EXPECT_TRUE(buffer->IsStreamDisturbed());
Expand All @@ -287,7 +288,8 @@ TEST_P(BodyStreamBufferTest, DrainAsBlobDataHandleReturnsNull) {
EXPECT_EQ(side_data_blob, buffer->GetSideDataBlobForTest());

EXPECT_FALSE(buffer->DrainAsBlobDataHandle(
BytesConsumer::BlobSizePolicy::kAllowBlobWithInvalidSize));
BytesConsumer::BlobSizePolicy::kAllowBlobWithInvalidSize,
ASSERT_NO_EXCEPTION));

EXPECT_FALSE(buffer->IsStreamLocked());
EXPECT_FALSE(buffer->IsStreamDisturbed());
Expand All @@ -309,7 +311,8 @@ TEST_P(BodyStreamBufferTest,
EXPECT_TRUE(buffer->IsStreamReadable());

EXPECT_FALSE(buffer->DrainAsBlobDataHandle(
BytesConsumer::BlobSizePolicy::kAllowBlobWithInvalidSize));
BytesConsumer::BlobSizePolicy::kAllowBlobWithInvalidSize,
ASSERT_NO_EXCEPTION));

EXPECT_FALSE(buffer->IsStreamLocked());
EXPECT_FALSE(buffer->IsStreamDisturbed());
Expand All @@ -335,7 +338,8 @@ TEST_P(BodyStreamBufferTest, DrainAsFormData) {
EXPECT_FALSE(buffer->IsStreamLocked());
EXPECT_FALSE(buffer->IsStreamDisturbed());
EXPECT_EQ(side_data_blob, buffer->GetSideDataBlobForTest());
scoped_refptr<EncodedFormData> output_form_data = buffer->DrainAsFormData();
scoped_refptr<EncodedFormData> output_form_data =
buffer->DrainAsFormData(ASSERT_NO_EXCEPTION);

EXPECT_TRUE(buffer->IsStreamLocked());
EXPECT_TRUE(buffer->IsStreamDisturbed());
Expand All @@ -359,7 +363,7 @@ TEST_P(BodyStreamBufferTest, DrainAsFormDataReturnsNull) {
EXPECT_FALSE(buffer->IsStreamDisturbed());
EXPECT_EQ(side_data_blob, buffer->GetSideDataBlobForTest());

EXPECT_FALSE(buffer->DrainAsFormData());
EXPECT_FALSE(buffer->DrainAsFormData(ASSERT_NO_EXCEPTION));

EXPECT_FALSE(buffer->IsStreamLocked());
EXPECT_FALSE(buffer->IsStreamDisturbed());
Expand All @@ -379,7 +383,7 @@ TEST_P(BodyStreamBufferTest,
EXPECT_FALSE(buffer->IsStreamDisturbed());
EXPECT_TRUE(buffer->IsStreamReadable());

EXPECT_FALSE(buffer->DrainAsFormData());
EXPECT_FALSE(buffer->DrainAsFormData(ASSERT_NO_EXCEPTION));

EXPECT_FALSE(buffer->IsStreamLocked());
EXPECT_FALSE(buffer->IsStreamDisturbed());
Expand Down Expand Up @@ -826,7 +830,7 @@ TEST_P(BodyStreamBufferTest, CachedMetadataHandler) {
EXPECT_EQ(handler, buffer->GetCachedMetadataHandler());
EXPECT_NE(weak_handler.Get(), nullptr);

buffer->CloseAndLockAndDisturb();
buffer->CloseAndLockAndDisturb(ASSERT_NO_EXCEPTION);
}

ThreadState::Current()->CollectAllGarbageForTesting();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace blink {

ScriptPromise BodyStreamBufferUnderlyingByteSource::Pull(
ReadableByteStreamController* controller,
ExceptionState&) {
ExceptionState& exception_state) {
if (!body_stream_buffer_->consumer_) {
// This is a speculative workaround for a crash. See
// https://crbug.com/773525.
Expand All @@ -28,7 +28,7 @@ ScriptPromise BodyStreamBufferUnderlyingByteSource::Pull(
}
body_stream_buffer_->stream_needs_more_ = true;
if (!body_stream_buffer_->in_process_data_) {
body_stream_buffer_->ProcessData();
body_stream_buffer_->ProcessData(exception_state);
}
return ScriptPromise::CastUndefined(GetScriptState());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@

namespace blink {

ScriptPromise BodyStreamBufferUnderlyingSource::pull(
ScriptState* script_state) {
ScriptPromise BodyStreamBufferUnderlyingSource::Pull(
ScriptState* script_state,
ExceptionState& exception_state) {
DCHECK_EQ(script_state, script_state_);
if (!body_stream_buffer_->consumer_) {
// This is a speculative workaround for a crash. See
Expand All @@ -26,7 +27,10 @@ ScriptPromise BodyStreamBufferUnderlyingSource::pull(
}
body_stream_buffer_->stream_needs_more_ = true;
if (!body_stream_buffer_->in_process_data_) {
body_stream_buffer_->ProcessData();
body_stream_buffer_->ProcessData(exception_state);
}
if (exception_state.HadException()) {
return ScriptPromise::Reject(script_state, exception_state);
}
return ScriptPromise::CastUndefined(script_state);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class CORE_EXPORT BodyStreamBufferUnderlyingSource final
script_state_(script_state),
body_stream_buffer_(body_buffer) {}

ScriptPromise pull(ScriptState*) override;
ScriptPromise Pull(ScriptState*, ExceptionState&) override;
ScriptPromise Cancel(ScriptState*,
ScriptValue reason,
ExceptionState&) override;
Expand Down
26 changes: 13 additions & 13 deletions third_party/blink/renderer/core/fetch/fetch_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class FetchLoaderBase : public GarbageCollectedMixin {
// https://fetch.spec.whatwg.org/#fetching
// Note that the actual loading is delegated to subclass via `CreateLoader()`,
// which may or may not start loading immediately.
void Start();
void Start(ExceptionState&);

// Disposes this loader.
// The owner of this loader uses this method to notify disposing of this
Expand Down Expand Up @@ -274,13 +274,13 @@ class FetchLoaderBase : public GarbageCollectedMixin {
absl::optional<String> devtools_request_id = absl::nullopt,
absl::optional<base::UnguessableToken> issue_id = absl::nullopt) = 0;

void PerformSchemeFetch();
void PerformSchemeFetch(ExceptionState&);
void PerformNetworkError(
const String& message,
absl::optional<base::UnguessableToken> issue_id = absl::nullopt);
void FileIssueAndPerformNetworkError(RendererCorsIssueCode,
int64_t identifier);
void PerformHTTPFetch();
void PerformHTTPFetch(ExceptionState&);
void PerformDataFetch();
bool AddConsoleMessage(const String& message,
absl::optional<base::UnguessableToken> issue_id);
Expand Down Expand Up @@ -724,7 +724,7 @@ void FetchManager::Loader::DidFailRedirectCheck(uint64_t identifier) {
IdentifiersFactory::SubresourceRequestId(identifier));
}

void FetchLoaderBase::Start() {
void FetchLoaderBase::Start(ExceptionState& exception_state) {
// "1. If |request|'s url contains a Known HSTS Host, modify it per the
// requirements of the 'URI [sic] Loading and Port Mapping' chapter of HTTP
// Strict Transport Security."
Expand Down Expand Up @@ -770,7 +770,7 @@ void FetchLoaderBase::Start() {
fetch_request_data_->IsolatedWorldOrigin()->CanReadContent(url)) ||
fetch_request_data_->Mode() == network::mojom::RequestMode::kNavigate) {
// "The result of performing a scheme fetch using request."
PerformSchemeFetch();
PerformSchemeFetch(exception_state);
return;
}

Expand Down Expand Up @@ -800,7 +800,7 @@ void FetchLoaderBase::Start() {
// service.
//
// "The result of performing a scheme fetch using |request|."
PerformSchemeFetch();
PerformSchemeFetch(exception_state);
return;
}

Expand All @@ -821,7 +821,7 @@ void FetchLoaderBase::Start() {

// "The result of performing an HTTP fetch using |request| with the
// |CORS flag| set."
PerformHTTPFetch();
PerformHTTPFetch(exception_state);
}

void FetchManager::Loader::Dispose() {
Expand Down Expand Up @@ -856,14 +856,14 @@ void FetchManager::Loader::Abort() {
NotifyFinished();
}

void FetchLoaderBase::PerformSchemeFetch() {
void FetchLoaderBase::PerformSchemeFetch(ExceptionState& exception_state) {
// "To perform a scheme fetch using |request|, switch on |request|'s url's
// scheme, and run the associated steps:"
if (SchemeRegistry::ShouldTreatURLSchemeAsSupportingFetchAPI(
fetch_request_data_->Url().Protocol()) ||
fetch_request_data_->Url().ProtocolIs("blob")) {
// "Return the result of performing an HTTP fetch using |request|."
PerformHTTPFetch();
PerformHTTPFetch(exception_state);
} else if (fetch_request_data_->Url().ProtocolIsData()) {
PerformDataFetch();
} else {
Expand Down Expand Up @@ -929,7 +929,7 @@ void FetchLoaderBase::PerformNetworkError(
Failed(message, nullptr, absl::nullopt, issue_id);
}

void FetchLoaderBase::PerformHTTPFetch() {
void FetchLoaderBase::PerformHTTPFetch(ExceptionState& exception_state) {
// CORS preflight fetch procedure is implemented inside ThreadableLoader.

// "1. Let |HTTPRequest| be a copy of |request|, except that |HTTPRequest|'s
Expand Down Expand Up @@ -960,7 +960,7 @@ void FetchLoaderBase::PerformHTTPFetch() {
fetch_request_data_->Method() != http_names::kHEAD) {
if (fetch_request_data_->Buffer()) {
scoped_refptr<EncodedFormData> form_data =
fetch_request_data_->Buffer()->DrainAsFormData();
fetch_request_data_->Buffer()->DrainAsFormData(exception_state);
if (form_data) {
request.SetHttpBody(form_data);
} else if (RuntimeEnabledFeatures::FetchUploadStreamingEnabled(
Expand Down Expand Up @@ -1384,7 +1384,7 @@ ScriptPromise FetchManager::Fetch(ScriptState* script_state,
GetExecutionContext(), this, resolver, request, script_state, signal);
loaders_.insert(loader);
// TODO(ricea): Reject the Response body with AbortError, not TypeError.
loader->Start();
loader->Start(exception_state);
return promise;
}

Expand Down Expand Up @@ -1503,7 +1503,7 @@ FetchLaterResult* FetchLaterManager::FetchLater(
// fetch records.
deferred_loaders_.insert(deferred_loader);

deferred_loader->Start();
deferred_loader->Start(exception_state);
return deferred_loader->fetch_later_result();
}

Expand Down

0 comments on commit efd22e4

Please sign in to comment.