summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrian Smith <brian@dbsoft.org>2023-10-03 01:45:08 -0500
committerBrian Smith <brian@dbsoft.org>2023-10-03 01:45:08 -0500
commit054f135a5455967009587e12c14908904ab4105a (patch)
treecbdaf6f10c86e8368992a750093e2894b5a57074
parent5c8336d123f37b6e22c2b49e0faa8912ce5fd2ce (diff)
downloaduxp-054f135a5455967009587e12c14908904ab4105a.tar.gz
Issue #1442 - Part 26 - FetchStreamReader needs to cancel its reader when it encounters write errors.
https://bugzilla.mozilla.org/show_bug.cgi?id=1416879 Part 5 Also same fix as in Part 24 for FetchStream but in FetchStreamReader.
-rw-r--r--dom/fetch/FetchStreamReader.cpp57
-rw-r--r--dom/fetch/FetchStreamReader.h6
2 files changed, 49 insertions, 14 deletions
diff --git a/dom/fetch/FetchStreamReader.cpp b/dom/fetch/FetchStreamReader.cpp
index 702b44a6b6..c6f5ca4d1d 100644
--- a/dom/fetch/FetchStreamReader.cpp
+++ b/dom/fetch/FetchStreamReader.cpp
@@ -7,6 +7,7 @@
#include "FetchStreamReader.h"
#include "InternalResponse.h"
#include "mozilla/dom/PromiseBinding.h"
+#include "mozilla/dom/DOMException.h"
#include "nsContentUtils.h"
#include "nsIScriptError.h"
#include "nsPIDOMWindow.h"
@@ -31,7 +32,10 @@ public:
{
if (!mWasNotified) {
mWasNotified = true;
- mReader->CloseAndRelease(NS_ERROR_DOM_INVALID_STATE_ERR);
+ // The WorkerPrivate does have a context available, and we could pass it
+ // here to trigger cancellation of the reader, but the author of this
+ // comment chickened out.
+ mReader->CloseAndRelease(nullptr, NS_ERROR_DOM_INVALID_STATE_ERR);
}
return true;
@@ -111,6 +115,9 @@ FetchStreamReader::Create(JSContext* aCx, nsIGlobalObject* aGlobal,
FetchStreamReader::FetchStreamReader(nsIGlobalObject* aGlobal)
: mGlobal(aGlobal)
+ // TODO: Replace with mGlobal->EventTargetFor(TaskCategory::Other)
+ // When we have the Dispatcher API in the tree, see Issue #1442
+ , mOwningEventTarget(NS_GetCurrentThread())
, mBufferRemaining(0)
, mBufferOffset(0)
, mStreamClosed(false)
@@ -120,11 +127,17 @@ FetchStreamReader::FetchStreamReader(nsIGlobalObject* aGlobal)
FetchStreamReader::~FetchStreamReader()
{
- CloseAndRelease(NS_BASE_STREAM_CLOSED);
+ CloseAndRelease(nullptr, NS_BASE_STREAM_CLOSED);
}
+// If a context is provided, an attempt will be made to cancel the reader. The
+// only situation where we don't expect to have a context is when closure is
+// being triggered from the destructor or the WorkerHolder is notifying. If
+// we're at the destructor, it's far too late to cancel anything. And if the
+// WorkerHolder is being notified, the global is going away, so there's also
+// no need to do further JS work.
void
-FetchStreamReader::CloseAndRelease(nsresult aStatus)
+FetchStreamReader::CloseAndRelease(JSContext* aCx, nsresult aStatus)
{
NS_ASSERT_OWNINGTHREAD(FetchStreamReader);
@@ -135,6 +148,21 @@ FetchStreamReader::CloseAndRelease(nsresult aStatus)
RefPtr<FetchStreamReader> kungFuDeathGrip = this;
+ if (aCx) {
+ MOZ_ASSERT(mReader);
+
+ RefPtr<DOMException> error = DOMException::Create(aStatus);
+
+ JS::Rooted<JS::Value> errorValue(aCx);
+ if (ToJSValue(aCx, error, &errorValue)) {
+ JS::Rooted<JSObject*> reader(aCx, mReader);
+ // It's currently safe to cancel an already closed reader because, per the
+ // comments in ReadableStream::cancel() conveying the spec, step 2 of
+ // 3.4.3 that specified ReadableStreamCancel is: If stream.[[state]] is
+ // "closed", return a new promise resolved with undefined.
+ JS::ReadableStreamReaderCancel(aCx, reader, errorValue);
+ }
+ }
mStreamClosed = true;
mGlobal = nullptr;
@@ -162,7 +190,7 @@ FetchStreamReader::StartConsuming(JSContext* aCx,
JS::ReadableStreamReaderMode::Default));
if (!reader) {
aRv.StealExceptionFromJSContext(aCx);
- CloseAndRelease(NS_ERROR_DOM_INVALID_STATE_ERR);
+ CloseAndRelease(aCx, NS_ERROR_DOM_INVALID_STATE_ERR);
return;
}
@@ -202,14 +230,14 @@ FetchStreamReader::OnOutputStreamReady(nsIAsyncOutputStream* aStream)
reader));
if (NS_WARN_IF(!promise)) {
// Let's close the stream.
- CloseAndRelease(NS_ERROR_DOM_INVALID_STATE_ERR);
+ CloseAndRelease(aes.cx(), NS_ERROR_DOM_INVALID_STATE_ERR);
return NS_ERROR_FAILURE;
}
RefPtr<Promise> domPromise = Promise::CreateFromExisting(mGlobal, promise);
if (NS_WARN_IF(!domPromise)) {
// Let's close the stream.
- CloseAndRelease(NS_ERROR_DOM_INVALID_STATE_ERR);
+ CloseAndRelease(aes.cx(), NS_ERROR_DOM_INVALID_STATE_ERR);
return NS_ERROR_FAILURE;
}
@@ -236,13 +264,13 @@ FetchStreamReader::ResolvedCallback(JSContext* aCx,
FetchReadableStreamReadDataDone valueDone;
if (!valueDone.Init(aCx, aValue)) {
JS_ClearPendingException(aCx);
- CloseAndRelease(NS_ERROR_DOM_INVALID_STATE_ERR);
+ CloseAndRelease(aCx, NS_ERROR_DOM_INVALID_STATE_ERR);
return;
}
if (valueDone.mDone) {
// Stream is completed.
- CloseAndRelease(NS_BASE_STREAM_CLOSED);
+ CloseAndRelease(aCx, NS_ERROR_DOM_INVALID_STATE_ERR);
return;
}
@@ -250,7 +278,7 @@ FetchStreamReader::ResolvedCallback(JSContext* aCx,
new FetchReadableStreamReadDataArray);
if (!value->Init(aCx, aValue) || !value->mValue.WasPassed()) {
JS_ClearPendingException(aCx);
- CloseAndRelease(NS_ERROR_DOM_INVALID_STATE_ERR);
+ CloseAndRelease(aCx, NS_ERROR_DOM_INVALID_STATE_ERR);
return;
}
@@ -270,7 +298,12 @@ FetchStreamReader::ResolvedCallback(JSContext* aCx,
mBufferOffset = 0;
mBufferRemaining = len;
- WriteBuffer();
+ nsresult rv = WriteBuffer();
+ if (NS_FAILED(rv)) {
+ // DOMException only understands errors from domerr.msg, so we normalize to
+ // identifying an abort if the write fails.
+ CloseAndRelease(aCx, NS_ERROR_DOM_ABORT_ERR);
+ }
}
nsresult
@@ -292,7 +325,6 @@ FetchStreamReader::WriteBuffer()
}
if (NS_WARN_IF(NS_FAILED(rv))) {
- CloseAndRelease(rv);
return rv;
}
@@ -308,7 +340,6 @@ FetchStreamReader::WriteBuffer()
nsresult rv = mPipeOut->AsyncWait(this, 0, 0, mOwningEventTarget);
if (NS_WARN_IF(NS_FAILED(rv))) {
- CloseAndRelease(rv);
return rv;
}
@@ -320,7 +351,7 @@ FetchStreamReader::RejectedCallback(JSContext* aCx,
JS::Handle<JS::Value> aValue)
{
ReportErrorToConsole(aCx, aValue);
- CloseAndRelease(NS_ERROR_FAILURE);
+ CloseAndRelease(aCx, NS_ERROR_FAILURE);
}
void
diff --git a/dom/fetch/FetchStreamReader.h b/dom/fetch/FetchStreamReader.h
index 6155ee17ff..c25685046c 100644
--- a/dom/fetch/FetchStreamReader.h
+++ b/dom/fetch/FetchStreamReader.h
@@ -40,8 +40,12 @@ public:
void
RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override;
+ // Idempotently close the output stream and null out all state. If aCx is
+ // provided, the reader will also be canceled. aStatus must be a DOM error
+ // as understood by DOMException because it will be provided as the
+ // cancellation reason.
void
- CloseAndRelease(nsresult aStatus);
+ CloseAndRelease(JSContext* aCx, nsresult aStatus);
void
StartConsuming(JSContext* aCx,