Skip to content

Commit

Permalink
EyeDropper: Use abort reason
Browse files Browse the repository at this point in the history
This change updates the open() method and abort steps attached to the
provided AbortSginal so that the reason provided to the AbortController
is used to reject the Promise returned by open().

The web-visible effect of this change is minimal because by default the
abort reason is set to the same "AbortError" DOMException which was
already used.

The corresponding specification change is:
WICG/eyedropper-api#25

Change-Id: I414bcd2ae7cd8fba3031c66ba98a5d04b0a7c740
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3559669
Reviewed-by: Ionel Popescu <iopopesc@microsoft.com>
Commit-Queue: Nidhi Jaju <nidhijaju@chromium.org>
Cr-Commit-Position: refs/heads/main@{#987267}
  • Loading branch information
nidhijaju authored and Chromium LUCI CQ committed Mar 31, 2022
1 parent c406472 commit fde9464
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 16 deletions.
55 changes: 40 additions & 15 deletions third_party/blink/renderer/modules/eyedropper/eye_dropper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,25 @@

namespace blink {

constexpr char kAbortMessage[] = "Color selection aborted.";
class EyeDropper::OpenAbortAlgorithm final : public AbortSignal::Algorithm {
public:
OpenAbortAlgorithm(EyeDropper* eyedropper, AbortSignal* signal)
: eyedropper_(eyedropper), abortsignal_(signal) {}
~OpenAbortAlgorithm() override = default;

void Run() override { eyedropper_->AbortCallback(abortsignal_); }

void Trace(Visitor* visitor) const override {
visitor->Trace(eyedropper_);
visitor->Trace(abortsignal_);
Algorithm::Trace(visitor);
}

private:
Member<EyeDropper> eyedropper_;
Member<AbortSignal> abortsignal_;
};

constexpr char kNotAvailableMessage[] = "EyeDropper is not available.";

EyeDropper::EyeDropper(ExecutionContext* context)
Expand Down Expand Up @@ -66,13 +84,12 @@ ScriptPromise EyeDropper::open(ScriptState* script_state,
}

if (options->hasSignal()) {
if (options->signal()->aborted()) {
exception_state.ThrowDOMException(DOMExceptionCode::kAbortError,
kAbortMessage);
return ScriptPromise();
signal_ = options->signal();
if (signal_->aborted()) {
return ScriptPromise::Reject(script_state, signal_->reason(script_state));
}
options->signal()->AddAlgorithm(
WTF::Bind(&EyeDropper::AbortCallback, WrapWeakPersistent(this)));
signal_->AddAlgorithm(
MakeGarbageCollected<OpenAbortAlgorithm>(this, signal_));
}

resolver_ = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
Expand All @@ -91,23 +108,30 @@ ScriptPromise EyeDropper::open(ScriptState* script_state,
return promise;
}

void EyeDropper::AbortCallback() {
eye_dropper_chooser_.reset();

void EyeDropper::AbortCallback(AbortSignal* signal) {
// There is no way to remove abort signal callbacks, so we need to
// perform null-check for `resolver_` to see if the promise has already
// been resolved.
// TODO(https://crbug.com/1296280): It should be possible to remove abort
// callbacks. This object can be reused for multiple eyedropper operations,
// and it might be possible for multiple abort signals to be mixed up.
if (!resolver_ ||
!IsInParallelAlgorithmRunnable(resolver_->GetExecutionContext(),
resolver_->GetScriptState()))

// There is no RemoveAlgorithm() method on AbortSignal so compare the signal
// bound to this callback to the one last passed to open().
if (signal_ != signal)
return;

ScriptState::Scope script_state_scope(resolver_->GetScriptState());
if (resolver_) {
ScriptState* script_state = resolver_->GetScriptState();
if (IsInParallelAlgorithmRunnable(resolver_->GetExecutionContext(),
script_state)) {
ScriptState::Scope script_state_scope(script_state);
resolver_->Reject(signal_->reason(script_state));
}
}

RejectPromiseHelper(DOMExceptionCode::kAbortError, kAbortMessage);
eye_dropper_chooser_.reset();
resolver_ = nullptr;
}

void EyeDropper::EyeDropperResponseHandler(ScriptPromiseResolver* resolver,
Expand Down Expand Up @@ -162,6 +186,7 @@ void EyeDropper::RejectPromiseHelper(DOMExceptionCode exception_code,
void EyeDropper::Trace(Visitor* visitor) const {
visitor->Trace(eye_dropper_chooser_);
visitor->Trace(resolver_);
visitor->Trace(signal_);
ScriptWrappable::Trace(visitor);
}

Expand Down
6 changes: 5 additions & 1 deletion third_party/blink/renderer/modules/eyedropper/eye_dropper.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

namespace blink {

class AbortSignal;
class ColorSelectionOptions;
enum class DOMExceptionCode;
class ExceptionState;
Expand Down Expand Up @@ -43,13 +44,16 @@ class EyeDropper final : public ScriptWrappable {
void Trace(Visitor*) const override;

private:
void AbortCallback();
class OpenAbortAlgorithm;

void AbortCallback(AbortSignal* signal);
void EyeDropperResponseHandler(ScriptPromiseResolver*, bool, uint32_t);
void EndChooser();
void RejectPromiseHelper(DOMExceptionCode, const WTF::String&);

HeapMojoRemote<mojom::blink::EyeDropperChooser> eye_dropper_chooser_;
Member<ScriptPromiseResolver> resolver_;
Member<AbortSignal> signal_;
};

} // namespace blink
Expand Down

0 comments on commit fde9464

Please sign in to comment.