[sms] Remove concept of timeouts in renderer process

This change follows this CL [1] and does further cleanup to remove
the concept of timeouts from the SMS Receiver API mainly from the
API interface and renderer process.

[1] https://crrev.com/c/1783219

Bug: 1000376
Change-Id: If7b8c17e0f1332df17e374c6f84e772a267c4760
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1783227Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693153}

content/browser/sms/sms_browsertest.cc

@@ -114,7 +114,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Receive) {
   // Test that SMS content can be retrieved after navigator.sms.receive().
   std::string script = R"(
     (async () => {
-      let sms = await navigator.sms.receive({timeout: 60});
+      let sms = await navigator.sms.receive();
       return sms.content;
     }) ();
   )";
@@ -203,7 +203,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Reload) {
   std::string script = R"(
     // kicks off the sms receiver, adding the service
     // to the observer's list.
-    navigator.sms.receive({timeout: 60});
+    navigator.sms.receive();
     true
   )";
 
@@ -245,7 +245,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Close) {
       base::WrapUnique(provider));
 
   std::string script = R"(
-    navigator.sms.receive({timeout: 60});
+    navigator.sms.receive();
     true
   )";
 
@@ -282,7 +282,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsSameOrigin) {
   NavigateToURL(tab2, url);
 
   std::string script = R"(
-    navigator.sms.receive({timeout: 60}).then(({content}) => {
+    navigator.sms.receive().then(({content}) => {
       sms = content;
     });
     true
@@ -398,7 +398,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsDifferentOrigin) {
   NavigateToURL(tab2, url2);
 
   std::string script = R"(
-    navigator.sms.receive({timeout: 60}).then(({content}) => {
+    navigator.sms.receive().then(({content}) => {
       sms = content;
     });
     true;
@@ -479,7 +479,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, SmsReceivedAfterTabIsClosed) {
 
   std::string script = R"(
     // kicks off an sms receiver call, but deliberately leaves it hanging.
-    navigator.sms.receive({timeout: 60});
+    navigator.sms.receive();
     true
   )";
 
@@ -531,7 +531,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Cancels) {
                                         ukm_loop.QuitClosure());
 
   std::string script = R"(
-    navigator.sms.receive({timeout: 60}).catch(({name}) => {
+    navigator.sms.receive().catch(({name}) => {
       error = name;
     });
     true;

content/browser/sms/sms_service.cc

@@ -56,7 +56,7 @@ void SmsService::Create(
   new SmsService(provider, host, std::move(receiver));
 }
 
-void SmsService::Receive(base::TimeDelta timeout, ReceiveCallback callback) {
+void SmsService::Receive(ReceiveCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (callback_) {
     std::move(callback).Run(blink::mojom::SmsStatus::kCancelled, base::nullopt);

content/browser/sms/sms_service.h

@@ -52,7 +52,7 @@ class CONTENT_EXPORT SmsService
                  const std::string& sms) override;
 
   // blink::mojom::SmsReceiver:
-  void Receive(base::TimeDelta timeout, ReceiveCallback) override;
+  void Receive(ReceiveCallback) override;
 
  private:
   void OpenInfoBar(const std::string& one_time_code);

content/browser/sms/sms_service_unittest.cc

@@ -13,7 +13,6 @@
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
-#include "base/time/time.h"
 #include "content/browser/sms/test/mock_sms_provider.h"
 #include "content/browser/sms/test/mock_sms_web_contents_delegate.h"
 #include "content/browser/web_contents/web_contents_impl.h"
@@ -32,7 +31,6 @@
 
 using base::BindLambdaForTesting;
 using base::Optional;
-using base::TimeDelta;
 using blink::mojom::SmsReceiver;
 using blink::mojom::SmsStatus;
 using std::string;
@@ -91,8 +89,8 @@ class Service {
         }));
   }
 
-  void MakeRequest(TimeDelta timeout, SmsReceiver::ReceiveCallback callback) {
-    service_remote_->Receive(timeout, std::move(callback));
+  void MakeRequest(SmsReceiver::ReceiveCallback callback) {
+    service_remote_->Receive(std::move(callback));
   }
 
   void NotifyReceive(const GURL& url, const string& message) {
@@ -138,7 +136,6 @@ TEST_F(SmsServiceTest, Basic) {
   }));
 
   service.MakeRequest(
-      TimeDelta::FromSeconds(10),
       BindLambdaForTesting(
           [&loop](SmsStatus status, const Optional<string>& sms) {
             EXPECT_EQ(SmsStatus::kSuccess, status);
@@ -166,7 +163,6 @@ TEST_F(SmsServiceTest, HandlesMultipleCalls) {
     }));
 
     service.MakeRequest(
-        TimeDelta::FromSeconds(10),
         BindLambdaForTesting(
             [&loop](SmsStatus status, const Optional<string>& sms) {
               EXPECT_EQ("first", sms.value());
@@ -187,7 +183,6 @@ TEST_F(SmsServiceTest, HandlesMultipleCalls) {
     }));
 
     service.MakeRequest(
-        TimeDelta::FromSeconds(10),
         BindLambdaForTesting(
             [&loop](SmsStatus status, const Optional<string>& sms) {
               EXPECT_EQ("second", sms.value());
@@ -219,7 +214,6 @@ TEST_F(SmsServiceTest, IgnoreFromOtherOrigins) {
   }));
 
   service.MakeRequest(
-      TimeDelta::FromSeconds(10),
       BindLambdaForTesting([&sms_status, &response, &sms_loop](
                                SmsStatus status, const Optional<string>& sms) {
         sms_status = status;
@@ -255,7 +249,6 @@ TEST_F(SmsServiceTest, ExpectOneReceiveTwo) {
   }));
 
   service.MakeRequest(
-      TimeDelta::FromSeconds(10),
       BindLambdaForTesting([&sms_status, &response, &sms_loop](
                                SmsStatus status, const Optional<string>& sms) {
         sms_status = status;
@@ -291,7 +284,6 @@ TEST_F(SmsServiceTest, AtMostOnePendingSmsRequest) {
 
   // Make the first SMS request.
   service.MakeRequest(
-      TimeDelta::FromSeconds(10),
       BindLambdaForTesting([&sms_status1, &response1, &sms1_loop](
                                SmsStatus status, const Optional<string>& sms) {
         sms_status1 = status;
@@ -302,7 +294,6 @@ TEST_F(SmsServiceTest, AtMostOnePendingSmsRequest) {
   // Make the second SMS request, and it will be canceled because the first SMS
   // request is still pending.
   service.MakeRequest(
-      TimeDelta::FromSeconds(10),
       BindLambdaForTesting([&sms_status2, &response2, &sms2_loop](
                                SmsStatus status, const Optional<string>& sms) {
         sms_status2 = status;
@@ -346,7 +337,6 @@ TEST_F(SmsServiceTest, CleansUp) {
   base::RunLoop reload;
 
   service->Receive(
-      base::TimeDelta::FromSeconds(10),
       base::BindLambdaForTesting(
           [&reload](SmsStatus status, const base::Optional<std::string>& sms) {
             EXPECT_EQ(SmsStatus::kTimeout, status);
@@ -379,7 +369,6 @@ TEST_F(SmsServiceTest, PromptsDialog) {
   }));
 
   service.MakeRequest(
-      TimeDelta::FromSeconds(10),
       BindLambdaForTesting(
           [&loop](SmsStatus status, const Optional<string>& sms) {
             EXPECT_EQ("hi", sms.value());
@@ -400,7 +389,6 @@ TEST_F(SmsServiceTest, Cancel) {
   base::RunLoop loop;
 
   service.MakeRequest(
-      TimeDelta::FromSeconds(10),
       BindLambdaForTesting(
           [&loop](SmsStatus status, const Optional<string>& sms) {
             EXPECT_EQ(SmsStatus::kCancelled, status);
@@ -438,7 +426,6 @@ TEST_F(SmsServiceTest, RecordTimeMetricsForContinueOnSuccess) {
   }));
 
   service.MakeRequest(
-      TimeDelta::FromSeconds(10),
       BindLambdaForTesting(
           [&loop](SmsStatus status, const Optional<string>& sms) {
             loop.Quit();
@@ -469,7 +456,6 @@ TEST_F(SmsServiceTest, RecordMetricsForCancelOnSuccess) {
   }));
 
   service.MakeRequest(
-      TimeDelta::FromSeconds(10),
       BindLambdaForTesting(
           [&loop](SmsStatus status, const Optional<string>& sms) {
             loop.Quit();

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

@@ -23,6 +23,5 @@ interface SmsReceiver {
   // to the caller's origin.
   // Returns the raw content of the received SMS.
   // |message| is only set if status == kSuccess.
-  Receive(mojo_base.mojom.TimeDelta timeout)
-    => (SmsStatus status, string? message);
+  Receive() => (SmsStatus status, string? message);
 };

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

@@ -26,15 +26,6 @@ void RecordSMSSuccessTime(base::TimeDelta duration) {
   UMA_HISTOGRAM_MEDIUM_TIMES("Blink.Sms.Receive.TimeSuccess", duration);
 }
 
-void RecordSMSRequestedTimeout(uint32_t timeout_seconds) {
-  UMA_HISTOGRAM_COUNTS_10000("Blink.Sms.Receive.RequestedTimeout",
-                             timeout_seconds);
-}
-
-void RecordSMSTimeoutExceededTime(base::TimeDelta duration) {
-  UMA_HISTOGRAM_MEDIUM_TIMES("Blink.Sms.Receive.TimeTimeoutExceeded", duration);
-}
-
 void RecordSMSCancelTime(base::TimeDelta duration) {
   UMA_HISTOGRAM_MEDIUM_TIMES("Blink.Sms.Receive.TimeCancel", duration);
 }

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

@@ -29,13 +29,6 @@ void RecordSMSOutcome(SMSReceiverOutcome outcome,
 // receives the SMS and presses continue to move on with the verification flow.
 void RecordSMSSuccessTime(base::TimeDelta duration);
 
-// Records the timeout value specified with the API is called. The value of 0
-// indicates that no value was specified.
-void RecordSMSRequestedTimeout(uint32_t timeout_seconds);
-
-// Records the time from when the API is called to when the user gets timed out.
-void RecordSMSTimeoutExceededTime(base::TimeDelta duration);
-
 // Records the time from when the API is called to when the user presses the
 // cancel button to abort SMS retrieval.
 void RecordSMSCancelTime(base::TimeDelta duration);

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

@@ -15,7 +15,6 @@
 #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"
@@ -23,11 +22,6 @@
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 
 namespace blink {
-namespace {
-
-const uint32_t kDefaultTimeoutSeconds = 60;
-
-}  // namespace
 
 SMSReceiver::SMSReceiver(ExecutionContext* context) : ContextClient(context) {}
 
@@ -44,18 +38,6 @@ ScriptPromise SMSReceiver::receive(ScriptState* script_state,
                           "Must be in top-level browsing context."));
   }
 
-  int32_t timeout_seconds =
-      options->hasTimeout() ? options->timeout() : kDefaultTimeoutSeconds;
-
-  if (timeout_seconds <= 0) {
-    return ScriptPromise::RejectWithDOMException(
-        script_state,
-        MakeGarbageCollected<DOMException>(DOMExceptionCode::kNotSupportedError,
-                                           "Invalid timeout."));
-  }
-
-  RecordSMSRequestedTimeout(options->hasTimeout() ? options->timeout() : 0);
-
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   requests_.insert(resolver);
 
@@ -71,7 +53,6 @@ ScriptPromise SMSReceiver::receive(ScriptState* script_state,
   }
 
   service_->Receive(
-      base::TimeDelta::FromSeconds(timeout_seconds),
       WTF::Bind(&SMSReceiver::OnReceive, WrapPersistent(this),
                 WrapPersistent(resolver), base::TimeTicks::Now()));
 
@@ -92,7 +73,6 @@ void SMSReceiver::OnReceive(ScriptPromiseResolver* resolver,
   if (status == mojom::blink::SmsStatus::kTimeout) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kTimeoutError, "SMSReceiver timed out."));
-    RecordSMSTimeoutExceededTime(base::TimeTicks::Now() - start_time);
     RecordSMSOutcome(SMSReceiverOutcome::kTimeout, source_id, recorder);
     return;
   } else if (status == mojom::blink::SmsStatus::kCancelled) {

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

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

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

@@ -12,7 +12,7 @@
 
  1) Include <script src="./sms_provider.js"></script> in your test.
  2) Set expectations
-  await expect(receive).andReturn((timeout) => {
+  await expect(receive).andReturn(() => {
     // mock behavior
   })
  3) Call navigator.sms.receive()
@@ -34,7 +34,7 @@
 'use strict';
 
 promise_test(async t => {
-  await expect(receive).andReturn((timeout) => {
+  await expect(receive).andReturn(() => {
       return Promise.resolve({
         status: Status.kSuccess,
         message: "hello",
@@ -47,13 +47,13 @@ promise_test(async t => {
 }, 'Basic usage');
 
 promise_test(async t => {
-  await expect(receive).andReturn((timeout) => {
+  await expect(receive).andReturn(() => {
       return Promise.resolve({
         status: Status.kSuccess,
         message: "hello1",
       });
   });
-  await expect(receive).andReturn((timeout) => {
+  await expect(receive).andReturn(() => {
       return Promise.resolve({
         status: Status.kSuccess,
         message: "hello2",
@@ -71,103 +71,35 @@ promise_test(async t => {
 }, 'Handle multiple requests in different order.');
 
 promise_test(async t => {
-  await expect(receive).andReturn((timeout) => {
+  await expect(receive).andReturn(() => {
       return Promise.resolve({
-        status: Status.kTimeout,
+        status: Status.kCancelled,
       });
   });
-  await expect(receive).andReturn((timeout) => {
+  await expect(receive).andReturn(() => {
       return Promise.resolve({
         status: Status.kSuccess,
         message: "success",
       });
   });
 
-  let timeout_sms = navigator.sms.receive();
+  let cancelled_sms = navigator.sms.receive();
   let successful_sms = navigator.sms.receive();
 
   let successful_msg = await successful_sms;
   assert_equals(successful_msg.content, "success");
 
   try {
-    await timeout_sms;
-    assert_unreached('Expected TimeoutError to be thrown.');
+    await cancelled_sms;
+    assert_unreached('Expected AbortError to be thrown.');
   } catch (error) {
-    assert_equals(error.name, "TimeoutError");
-    assert_equals(error.message, "SMSReceiver timed out.");
+    assert_equals(error.name, "AbortError");
+    assert_equals(error.message, "SMSReceiver was aborted.");
   }
 }, 'Handle multiple requests with success and error.');
 
 promise_test(async t => {
-  await expect(receive).andReturn((timeout) => {
-      return Promise.resolve({
-        status: Status.kTimeout,
-      });
-  });
-
-  try {
-    await navigator.sms.receive();
-    assert_unreached('Expected TimeoutError to be thrown.');
-  } catch (error) {
-    assert_equals(error.name, "TimeoutError");
-    assert_equals(error.message, "SMSReceiver timed out.");
-  }
-}, 'Deal with timeouts');
-
-promise_test(async t => {
-  try {
-    await navigator.sms.receive({timeout: 0});
-    assert_unreached('Expected NotSupportedError to be thrown.');
-  } catch (error) {
-    assert_equals(error.name, "NotSupportedError");
-    assert_equals(error.message, "Invalid timeout.");
-  }
-}, 'Should throw error with invalid timeout (0)');
-
-promise_test(async t => {
-  try {
-    await navigator.sms.receive({timeout: null});
-    assert_unreached('Expected NotSupportedError to be thrown.');
-  } catch (error) {
-    assert_equals(error.name, "NotSupportedError");
-    assert_equals(error.message, "Invalid timeout.");
-  }
-}, 'Should throw error with invalid timeout (null)');
-
-promise_test(async t => {
-  try {
-    await navigator.sms.receive({timeout: -1});
-    assert_unreached('Expected NotSupportedError to be thrown.');
-  } catch (error) {
-    assert_equals(error.name, "NotSupportedError");
-    assert_equals(error.message, "Invalid timeout.");
-  }
-}, 'Should throw error with invalid timeout (-1)');
-
-promise_test(async t => {
-  try {
-    await navigator.sms.receive({timeout: NaN});
-    assert_unreached('Expected NotSupportedError to be thrown.');
-  } catch (error) {
-    assert_equals(error.name, "NotSupportedError");
-    assert_equals(error.message, "Invalid timeout.");
-  }
-}, 'Should throw error with invalid timeout (NaN)');
-
-promise_test(async t => {
-  await expect(receive).andReturn((timeout) => {
-      return Promise.resolve({
-        status: Status.kSuccess,
-        message: "hello",
-      });
-  });
-
-  let sms = await navigator.sms.receive({timeout: undefined});
-  assert_equals(sms.content, "hello");
-}, 'Should use default value for timeout (undefined)');
-
-promise_test(async t => {
-  await expect(receive).andReturn((timeout) => {
+  await expect(receive).andReturn(() => {
       return Promise.resolve({
         status: Status.kCancelled,
       });

third_party/blink/web_tests/external/wpt/sms/sms_provider.js

@@ -36,16 +36,16 @@ class FakeSmsReceiverImpl {
     return this;
   }
 
-  receive(timeout) {
+  receive() {
     let call = this.returnValues.receive.shift();
     if (!call) {
       throw new Error("Unexpected call.");
     }
-    return call(timeout);
+    return call();
   }
 }
 
-function receive(timeout, callback) {
+function receive(callback) {
   throw new Error("expected to be overriden by tests");
 }
 

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

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