Commit 331d4009 authored by Ayu Ishii's avatar Ayu Ishii Committed by Commit Bot

[sms] Implement AbortController for SMSReceiver API

This change allows developers to use the SMS Receiver API with the Abort
Controller to cancel once the API is called. This will help developers with
flows for retrying and sending a new code and they want to abort the previous
call they made to the API. If the InfoBar is up during abort, it will
leave the InfoBar open while returning an AbortError to the website. If
another request is made while the InfoBar for 1st request is still open (after
abort), upon clicking 'Verify' the SMS retrieved for the 1st request will
return to the 2nd request.

Bug: 976401
Change-Id: I132800f2954fdc370172dcd03789d3f08bcc4040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866914Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#717710}
parent 67324ed2
This diff is collapsed.
......@@ -69,6 +69,13 @@ void SmsService::Receive(ReceiveCallback callback) {
callback_ = std::move(callback);
// |sms_| and prompt are still present from the previous request so a new
// subscription is unnecessary.
if (prompt_open_) {
// TODO(crbug.com/1024598): Add UMA histogram.
return;
}
fetcher_->Subscribe(origin_, this);
}
......@@ -87,10 +94,17 @@ void SmsService::OnReceive(const std::string& one_time_code,
OpenInfoBar(one_time_code);
}
void SmsService::Abort() {
DCHECK(callback_);
Process(SmsStatus::kAborted, base::nullopt);
}
void SmsService::OpenInfoBar(const std::string& one_time_code) {
WebContents* web_contents =
content::WebContents::FromRenderFrameHost(render_frame_host());
prompt_open_ = true;
web_contents->GetDelegate()->CreateSmsPrompt(
render_frame_host(), origin_, one_time_code,
base::BindOnce(&SmsService::OnConfirm, weak_ptr_factory_.GetWeakPtr()),
......@@ -115,7 +129,10 @@ void SmsService::OnConfirm() {
DCHECK(!receive_time_.is_null());
RecordContinueOnSuccessTime(base::TimeTicks::Now() - receive_time_);
Process(SmsStatus::kSuccess, sms_);
prompt_open_ = false;
if (callback_)
Process(SmsStatus::kSuccess, sms_);
}
void SmsService::OnCancel() {
......@@ -125,14 +142,22 @@ void SmsService::OnCancel() {
DCHECK(!receive_time_.is_null());
RecordCancelOnSuccessTime(base::TimeTicks::Now() - receive_time_);
Process(SmsStatus::kCancelled, base::nullopt);
prompt_open_ = false;
if (callback_)
Process(SmsStatus::kCancelled, base::nullopt);
}
void SmsService::CleanUp() {
callback_.Reset();
sms_.reset();
// Skip resetting |sms_| and |receive_time_| while prompt is still open
// in case it needs to be returned to the next incoming request upon prompt
// confirmation.
if (!prompt_open_) {
sms_.reset();
receive_time_ = base::TimeTicks();
}
start_time_ = base::TimeTicks();
receive_time_ = base::TimeTicks();
callback_.Reset();
fetcher_->Unsubscribe(origin_, this);
}
......
......@@ -6,6 +6,7 @@
#define CONTENT_BROWSER_SMS_SMS_SERVICE_H_
#include <memory>
#include <string>
#include "base/callback_forward.h"
#include "base/macros.h"
......@@ -49,6 +50,7 @@ class CONTENT_EXPORT SmsService
// blink::mojom::SmsReceiver:
void Receive(ReceiveCallback) override;
void Abort() override;
// content::SmsQueue::Subscriber
void OnReceive(const std::string& one_time_code,
......@@ -71,6 +73,8 @@ class CONTENT_EXPORT SmsService
const url::Origin origin_;
bool prompt_open_ = false;
ReceiveCallback callback_;
base::Optional<std::string> sms_;
base::TimeTicks start_time_;
......
This diff is collapsed.
......@@ -15,7 +15,8 @@ enum class SMSReceiverOutcome {
kTimeout = 1,
kConnectionError = 2,
kCancelled = 3,
kMaxValue = kCancelled
kAborted = 4,
kMaxValue = kAborted
};
} // namespace blink
......
......@@ -12,7 +12,8 @@ import "mojo/public/mojom/base/time.mojom";
enum SmsStatus {
kSuccess,
kTimeout,
kCancelled
kCancelled,
kAborted
};
// This interface is created per storage partition but its execution is context
......@@ -24,4 +25,7 @@ interface SmsReceiver {
// Returns the raw content of the received SMS.
// |message| is only set if status == kSuccess.
Receive() => (SmsStatus status, string? message);
// Aborts the current retrieval process and resolves it with an
// kAborted SmsStatus.
Abort();
};
......@@ -10,11 +10,13 @@
#include "third_party/blink/public/mojom/sms/sms_receiver.mojom-blink.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
#include "third_party/blink/renderer/core/dom/abort_signal.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/modules/sms/sms.h"
#include "third_party/blink/renderer/modules/sms/sms_metrics.h"
#include "third_party/blink/renderer/modules/sms/sms_receiver_options.h"
#include "third_party/blink/renderer/platform/bindings/name_client.h"
#include "third_party/blink/renderer/platform/bindings/script_state.h"
#include "third_party/blink/renderer/platform/heap/heap.h"
......@@ -25,20 +27,38 @@ namespace blink {
SMSReceiver::SMSReceiver(ExecutionContext* context) : ContextClient(context) {}
SMSReceiver::~SMSReceiver() = default;
ScriptPromise SMSReceiver::receive(ScriptState* script_state,
const SMSReceiverOptions* options) {
const SMSReceiverOptions* options,
ExceptionState& exception_state) {
ExecutionContext* context = ExecutionContext::From(script_state);
DCHECK(context->IsContextThread());
LocalFrame* frame = GetFrame();
if (!frame->IsMainFrame() && frame->IsCrossOriginSubframe()) {
return ScriptPromise::RejectWithDOMException(
script_state, MakeGarbageCollected<DOMException>(
DOMExceptionCode::kNotAllowedError,
"Must have the same origin as the top-level frame."));
exception_state.ThrowDOMException(
DOMExceptionCode::kNotAllowedError,
"Must have the same origin as the top-level frame.");
return ScriptPromise();
}
if (options->hasSignal() && options->signal()->aborted()) {
RecordSMSOutcome(SMSReceiverOutcome::kAborted, GetDocument()->UkmSourceID(),
GetDocument()->UkmRecorder());
exception_state.ThrowDOMException(DOMExceptionCode::kAbortError,
"Request has been aborted.");
return ScriptPromise();
}
auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
if (options->hasSignal()) {
options->signal()->AddAlgorithm(WTF::Bind(&SMSReceiver::Abort,
WrapWeakPersistent(this),
WrapPersistent(resolver)));
}
requests_.insert(resolver);
// See https://bit.ly/2S0zRAS for task types.
......@@ -59,7 +79,16 @@ ScriptPromise SMSReceiver::receive(ScriptState* script_state,
return resolver->Promise();
}
SMSReceiver::~SMSReceiver() = default;
void SMSReceiver::Abort(ScriptPromiseResolver* resolver) {
RecordSMSOutcome(SMSReceiverOutcome::kAborted, GetDocument()->UkmSourceID(),
GetDocument()->UkmRecorder());
service_->Abort();
resolver->Reject(
MakeGarbageCollected<DOMException>(DOMExceptionCode::kAbortError));
requests_.erase(resolver);
}
void SMSReceiver::OnReceive(ScriptPromiseResolver* resolver,
base::TimeTicks start_time,
......
......@@ -28,13 +28,17 @@ class SMSReceiver final : public ScriptWrappable, public ContextClient {
~SMSReceiver() override;
// SMSReceiver IDL interface.
ScriptPromise receive(ScriptState*, const SMSReceiverOptions*);
ScriptPromise receive(ScriptState*,
const SMSReceiverOptions*,
ExceptionState&);
void Trace(blink::Visitor*) override;
private:
HeapHashSet<Member<ScriptPromiseResolver>> requests_;
void Abort(ScriptPromiseResolver* resolver);
void OnReceive(ScriptPromiseResolver* resolver,
base::TimeTicks start_time,
mojom::blink::SmsStatus status,
......
......@@ -9,6 +9,6 @@
Exposed=Window,
RuntimeEnabled=SmsReceiver
] interface SMSReceiver {
[CallWith=ScriptState, MeasureAs=SMSReceiverStart]
[CallWith=ScriptState, RaisesException, MeasureAs=SMSReceiverStart]
Promise<SMS> receive(optional SMSReceiverOptions options);
};
......@@ -5,5 +5,5 @@
// https://github.com/samuelgoto/sms-receiver
dictionary SMSReceiverOptions {
// TODO(b/976401): Implement abort controller.
AbortSignal signal;
};
......@@ -8,7 +8,7 @@ const SmsProvider = (() => {
this.mojoReceiver_ = new blink.mojom.SmsReceiverReceiver(this);
this.interceptor_ = new MojoInterfaceInterceptor(
blink.mojom.SmsReceiver.$interfaceName, "context", true)
blink.mojom.SmsReceiver.$interfaceName, "context", true);
this.interceptor_.oninterfacerequest = (e) => {
this.mojoReceiver_.$.bindHandle(e.handle);
......@@ -27,6 +27,8 @@ const SmsProvider = (() => {
return call();
}
abort() {}
pushReturnValuesForTesting(callName, value) {
this.returnValues_[callName] = this.returnValues_[callName] || [];
this.returnValues_[callName].push(value);
......
......@@ -114,4 +114,21 @@ promise_test(async t => {
}
}, 'Deal with cancelled requests');
promise_test(async t => {
const controller = new AbortController();
const signal = controller.signal;
controller.abort();
await promise_rejects(t, 'AbortError', navigator.sms.receive({signal}));
}, 'Should abort request');
promise_test(async t => {
const controller = new AbortController();
const signal = controller.signal;
let error = navigator.sms.receive({signal});
controller.abort();
await promise_rejects(t, 'AbortError', error);
}, 'Should abort request even while request is in progress.');
</script>
......@@ -13,7 +13,7 @@ interface SMS {
};
dictionary SMSReceiverOptions {
// TODO(b/976401): Implement abort controller.
AbortSignal signal;
};
[
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment