Reland [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.

Original CL: https://crrev.com/c/1866914
Revert CL: https://crrev.com/c/1930275
Failure ticket: https://crbug.com/1027386

Failure locally reproducible with following test command:
python testing/scripts/run_isolated_script_test.py third_party/blink/tools/run_web_tests.py  -t LinuxTest third_party/blink/web_tests/external/wpt/sms/interceptor.https.html --isolated-script-test-output=output.json --isolated-script-test-perf-output=perftest-output.json --additional-expectations third_party/blink/web_tests/LeakExpectations --enable-leak-detection

Bug: 976401
Change-Id: I9b1208a6cf9c05185fa734809ab275235238cf4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1929864Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718954}

content/browser/sms/sms_service.cc

@@ -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);
 }
 

content/browser/sms/sms_service.h

@@ -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_;

third_party/blink/public/common/sms/sms_receiver_outcome.h

@@ -15,7 +15,8 @@ enum class SMSReceiverOutcome {
   kTimeout = 1,
   kConnectionError = 2,
   kCancelled = 3,
-  kMaxValue = kCancelled
+  kAborted = 4,
+  kMaxValue = kAborted
 };
 
 }  // namespace blink

third_party/blink/public/mojom/sms/sms_receiver.mojom

@@ -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();
 };

third_party/blink/renderer/modules/sms/sms_receiver.cc

@@ -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,

third_party/blink/renderer/modules/sms/sms_receiver.h

@@ -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,

third_party/blink/renderer/modules/sms/sms_receiver.idl

@@ -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);
 };

third_party/blink/renderer/modules/sms/sms_receiver_options.idl

@@ -5,5 +5,5 @@
 // https://github.com/samuelgoto/sms-receiver
 
 dictionary SMSReceiverOptions {
-  // TODO(b/976401): Implement abort controller.
+  AbortSignal signal;
 };

third_party/blink/web_tests/external/wpt/resources/chromium/sms_mock.js

@@ -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);
@@ -18,15 +18,16 @@ const SmsProvider = (() => {
       this.returnValues_ = {};
     }
 
-    receive() {
+    async receive() {
       let call = this.returnValues_.receive ?
           this.returnValues_.receive.shift() : null;
-      if (!call) {
-        throw new Error("Unexpected call.");
-      }
+      if (!call)
+        return;
       return call();
     }
 
+    async abort() {}
+
     pushReturnValuesForTesting(callName, value) {
       this.returnValues_[callName] = this.returnValues_[callName] || [];
       this.returnValues_[callName].push(value);

third_party/blink/web_tests/external/wpt/sms/interceptor.https.html

@@ -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>

third_party/blink/web_tests/external/wpt/sms/sms_receiver.idl

@@ -13,7 +13,7 @@ interface SMS {
 };
 
 dictionary SMSReceiverOptions {
-  // TODO(b/976401): Implement abort controller.
+  AbortSignal signal;
 };
 
 [