Skip to content

Commit

Permalink
[ImageDecoder] Replace IndexSizeError with RangeError.
Browse files Browse the repository at this point in the history
IndexSizeError is deprecated, so switch to RangeError. It's a bit
tricky since RangeError isn't a DOMException, so a new error message
field is introduced just for throwing the RangeError.

R=tguilbert

Bug: 1182435
Change-Id: I788fcf76c60cf6d0a1f4eb3e979d68aeaec297ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2872622
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#879176}
  • Loading branch information
dalecurtis authored and Chromium LUCI CQ committed May 5, 2021
1 parent d2bab3e commit 343bef1
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ void ImageDecoderExternal::DecodeRequest::Trace(Visitor* visitor) const {
visitor->Trace(exception);
}

bool ImageDecoderExternal::DecodeRequest::IsFinal() const {
return result || exception || range_error_message;
}

// static
ScriptPromise ImageDecoderExternal::isTypeSupported(ScriptState* script_state,
String type) {
Expand Down Expand Up @@ -429,7 +433,7 @@ void ImageDecoderExternal::MaybeSatisfyPendingDecodes() {
}

// Ignore already submitted requests and those already satisfied.
if (request->pending || request->exception || request->result)
if (request->pending || request->IsFinal())
continue;

if (!data_complete_) {
Expand All @@ -455,11 +459,9 @@ void ImageDecoderExternal::MaybeSatisfyPendingDecodes() {
decode_weak_factory_.GetWeakPtr()));
}

auto* new_end =
std::stable_partition(pending_decodes_.begin(), pending_decodes_.end(),
[](const auto& request) {
return !request->result && !request->exception;
});
auto* new_end = std::stable_partition(
pending_decodes_.begin(), pending_decodes_.end(),
[](const auto& request) { return !request->IsFinal(); });

// Copy completed requests to a new local vector to avoid reentrancy issues
// when resolving and rejecting the promises.
Expand All @@ -470,10 +472,15 @@ void ImageDecoderExternal::MaybeSatisfyPendingDecodes() {

// Note: Promise resolution may invoke calls into this class.
for (auto& request : completed_decodes) {
if (request->exception)
if (request->exception) {
request->resolver->Reject(request->exception);
else
} else if (request->range_error_message) {
ScriptState::Scope scope(script_state_);
request->resolver->Reject(V8ThrowException::CreateRangeError(
script_state_->GetIsolate(), *request->range_error_message));
} else {
request->resolver->Resolve(request->result);
}
}
}

Expand All @@ -495,13 +502,12 @@ void ImageDecoderExternal::OnDecodeReady(

request->pending = false;
if (result->status == ImageDecoderCore::Status::kIndexError) {
request->exception = MakeGarbageCollected<DOMException>(
DOMExceptionCode::kIndexSizeError,
request->range_error_message =
ExceptionMessages::IndexOutsideRange<uint32_t>(
"frame index", request->frame_index, 0,
ExceptionMessages::kInclusiveBound,
tracks_->selectedTrack().value()->frameCount(),
ExceptionMessages::kExclusiveBound));
ExceptionMessages::kExclusiveBound);
MaybeSatisfyPendingDecodes();
return;
}
Expand All @@ -512,11 +518,10 @@ void ImageDecoderExternal::OnDecodeReady(
// Once we're data complete, if no further image can be decoded, we should
// reject the decode() since it can't be satisfied.
if (data_complete_) {
request->exception = MakeGarbageCollected<DOMException>(
DOMExceptionCode::kIndexSizeError,
String::Format("Unexpected end of image. Request for frame index %d "
"can't be satisfied.",
request->frame_index));
request->range_error_message = String::Format(
"Unexpected end of image. Request for frame index %d "
"can't be satisfied.",
request->frame_index);
}

MaybeSatisfyPendingDecodes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,21 @@ class MODULES_EXPORT ImageDecoderExternal final
bool construction_succeeded_ = false;

// Pending decode() requests.
struct DecodeRequest : public GarbageCollected<DecodeRequest> {
struct DecodeRequest final : public GarbageCollected<DecodeRequest> {
DecodeRequest(ScriptPromiseResolver* resolver,
uint32_t frame_index,
bool complete_frames_only);
void Trace(Visitor*) const;
bool IsFinal() const;

Member<ScriptPromiseResolver> resolver;
uint32_t frame_index;
bool complete_frames_only;
bool pending = false;
base::Optional<size_t> bytes_read_index;
Member<ImageDecodeResult> result;

base::Optional<String> range_error_message;
Member<DOMException> exception;
};
HeapVector<Member<DecodeRequest>> pending_decodes_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,11 @@ promise_test(t => {
})
.then(buffer => {
let decoder = new ImageDecoder({data: buffer, type: 'image/png'});
return promise_rejects_dom(
t, 'IndexSizeError', decoder.decode({frameIndex: 1}));
return promise_rejects_js(
t, RangeError, decoder.decode({frameIndex: 1}));
});
});
}, 'Test out of range index returns IndexSizeError');
}, 'Test out of range index returns RangeError');

promise_test(t => {
var decoder;
Expand All @@ -214,11 +214,11 @@ promise_test(t => {
// Queue two decodes to ensure index verification and decoding are
// properly ordered.
p1 = decoder.decode({frameIndex: 0});
return promise_rejects_dom(
t, 'IndexSizeError', decoder.decode({frameIndex: 1}));
return promise_rejects_js(
t, RangeError, decoder.decode({frameIndex: 1}));
})
.then(_ => {
return promise_rejects_dom(t, 'IndexSizeError', p1);
return promise_rejects_js(t, RangeError, p1);
})
});
}, 'Test partial decoding without a frame results in an error');
Expand Down

0 comments on commit 343bef1

Please sign in to comment.