[sms] Implement timeout UI for SMS

This CL implements timeout UI for SMS. I uploaded a screenshot of the
SMS dialog on the issue page.

Bug: 983325
Change-Id: I0299981a1efbc29cb78532cafb090425138c65cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1727730
Commit-Queue: Jun Cai <juncai@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686575}

chrome/android/java/res/drawable/ic_error_blue_18dp.xml

+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 The Chromium Authors. All rights reserved.
+     Use of this source code is governed by a BSD-style license that can be
+     found in the LICENSE file.
+-->
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+        xmlns:tools="http://schemas.android.com/tools"
+        tools:targetApi="21"
+        android:width="18dp"
+        android:height="18dp"
+        android:viewportWidth="48.0"
+        android:viewportHeight="48.0">
+    <path
+        android:pathData="M24 4C12.96 4 4 12.95 4 24s8.96 20 20 20 20-8.95 20-20S35.04 4 24 4zm2 30h-4v-4h4v4zm0-8h-4V14h4v12z"
+        android:fillColor="@color/default_icon_color_blue"/>
+</vector>

chrome/android/java/res/layout/sms_receiver_dialog.xml

@@ -37,6 +37,13 @@
             android:id="@+id/container"
             android:layout_width="wrap_content"
             android:layout_height="wrap_content" >
+            <ImageView
+                android:id="@+id/error_icon"
+                android:layout_height="20dp"
+                android:layout_width="20dp"
+                android:visibility="gone"
+                app:srcCompat="@drawable/ic_error_blue_18dp"
+                tools:ignore="ContentDescription" />
             <ImageView
                 android:id="@+id/done_icon"
                 android:layout_height="20dp"
@@ -70,11 +77,11 @@
         app:stackedMargin="@dimen/button_bar_stacked_margin"
         app:buttonAlignment="end">
         <org.chromium.ui.widget.ButtonCompat
-            android:id="@+id/confirm_button"
+            android:id="@+id/confirm_or_try_again_button"
             android:layout_height="wrap_content"
             android:layout_width="wrap_content"
             android:layout_gravity="end"
-            android:text="@string/sms_dialog_confirm_button_text"
+            android:text="@string/confirm"
             android:enabled="false"
             style="@style/TextButton" />
         <org.chromium.ui.widget.ButtonCompat

chrome/android/java/src/org/chromium/chrome/browser/sms/SmsReceiverDialog.java

@@ -39,8 +39,10 @@ public class SmsReceiverDialog {
     private Dialog mDialog;
     private ProgressBar mProgressBar;
     private ImageView mDoneIcon;
+    private ImageView mErrorIcon;
     private TextView mStatus;
-    private Button mConfirmButton;
+    private Button mCancelButton;
+    private Button mConfirmOrTryAgainButton;
     private String mOrigin;
 
     @VisibleForTesting
@@ -68,16 +70,19 @@ public class SmsReceiverDialog {
 
         mDoneIcon = (ImageView) dialogContainer.findViewById(R.id.done_icon);
 
+        mErrorIcon = (ImageView) dialogContainer.findViewById(R.id.error_icon);
+
         mStatus = (TextView) dialogContainer.findViewById(R.id.status);
 
-        Button cancelButton = (Button) dialogContainer.findViewById(R.id.cancel_button);
-        cancelButton.setOnClickListener(v -> {
+        mCancelButton = (Button) dialogContainer.findViewById(R.id.cancel_button);
+        mCancelButton.setOnClickListener(v -> {
             assert mNativeSmsDialogAndroid != 0;
             SmsReceiverDialogJni.get().onEvent(mNativeSmsDialogAndroid, Event.CANCEL);
         });
 
-        mConfirmButton = (Button) dialogContainer.findViewById(R.id.confirm_button);
-        mConfirmButton.setOnClickListener(v -> {
+        mConfirmOrTryAgainButton =
+                (Button) dialogContainer.findViewById(R.id.confirm_or_try_again_button);
+        mConfirmOrTryAgainButton.setOnClickListener(v -> {
             assert mNativeSmsDialogAndroid != 0;
             SmsReceiverDialogJni.get().onEvent(mNativeSmsDialogAndroid, Event.CONFIRM);
         });
@@ -132,7 +137,23 @@ public class SmsReceiverDialog {
         mProgressBar.setVisibility(View.GONE);
         mDoneIcon.setVisibility(View.VISIBLE);
         mStatus.setText(mActivity.getText(R.string.sms_dialog_status_sms_received));
-        mConfirmButton.setEnabled(true);
+        mConfirmOrTryAgainButton.setEnabled(true);
+    }
+
+    @CalledByNative
+    void smsTimeout() {
+        if (DEBUG) Log.d(TAG, "SmsReceiverDialog.smsTimeout()");
+
+        mProgressBar.setVisibility(View.GONE);
+        mErrorIcon.setVisibility(View.VISIBLE);
+        mStatus.setText(mActivity.getText(R.string.sms_dialog_status_timeout));
+        mCancelButton.setVisibility(View.GONE);
+        mConfirmOrTryAgainButton.setText(mActivity.getText(R.string.try_again));
+        mConfirmOrTryAgainButton.setEnabled(true);
+        mConfirmOrTryAgainButton.setOnClickListener(v -> {
+            assert mNativeSmsDialogAndroid != 0;
+            SmsReceiverDialogJni.get().onEvent(mNativeSmsDialogAndroid, Event.TIMEOUT);
+        });
     }
 
     /**

chrome/android/java/strings/android_chrome_strings.grd

@@ -3966,8 +3966,8 @@ The site does NOT gain access to the camera. The camera images are only visible
       <message name="IDS_SMS_DIALOG_STATUS_SMS_RECEIVED" desc="Message shown when Chrome has received an SMS on the user's behalf">
         Text message received
       </message>
-      <message name="IDS_SMS_DIALOG_CONFIRM_BUTTON_TEXT" desc="Label on the button that users can click to confirm SMS verification by passing the incoming SMS to the site.">
-        Confirm
+      <message name="IDS_SMS_DIALOG_STATUS_TIMEOUT" desc="Message shown when SMS verification timed out.">
+        Something went wrong
       </message>
 
       <message name="IDS_TEST_DUMMY_MODULE_TITLE"

chrome/android/java/strings/android_chrome_strings_grd/IDS_SMS_DIALOG_CONFIRM_BUTTON_TEXT.png.sha1

-3851b10baa9e6cd314477667cdac69e549ba2e52
\ No newline at end of file

chrome/android/java/strings/android_chrome_strings_grd/IDS_SMS_DIALOG_STATUS_TIMEOUT.png.sha1

+06b38a1c7e0379eb1a4407829baf8b8300c74744
\ No newline at end of file

chrome/android/java/strings/android_chrome_strings_grd/IDS_SMS_DIALOG_TITLE.png.sha1

-ef2666b60a9adfb774b327ecec89f1ef090a78a4
+9a38cd3599c74c36b46f3bf4edf565fa8280eb9d
\ No newline at end of file

chrome/android/javatests/src/org/chromium/chrome/browser/sms/SmsReceiverDialogTest.java

@@ -50,6 +50,7 @@ public class SmsReceiverDialogTest {
 
     final private CallbackHelper mCancelButtonClickedCallback = new CallbackHelper();
     final private CallbackHelper mConfirmButtonClickedCallback = new CallbackHelper();
+    final private CallbackHelper mTryAgainButtonClickedCallback = new CallbackHelper();
 
     private class TestSmsReceiverDialogJni implements SmsReceiverDialog.Natives {
         @Override
@@ -61,6 +62,9 @@ public class SmsReceiverDialogTest {
                 case Event.CONFIRM:
                     mConfirmButtonClickedCallback.notifyCalled();
                     return;
+                case Event.TIMEOUT:
+                    mTryAgainButtonClickedCallback.notifyCalled();
+                    return;
                 default:
                     assert false : "|eventType| is invalid";
             }
@@ -86,43 +90,107 @@ public class SmsReceiverDialogTest {
 
     @Test
     @LargeTest
-    public void testCancelButtonAndConfirmButton() {
+    public void testSmsCancel() throws Throwable {
         Dialog dialog = mSmsDialog.getDialogForTesting();
 
         Button cancelButton = (Button) dialog.findViewById(R.id.cancel_button);
         Assert.assertTrue(cancelButton.isEnabled());
 
-        Button confirmButton = (Button) dialog.findViewById(R.id.confirm_button);
-        Assert.assertFalse(confirmButton.isEnabled());
-
-        TestThreadUtils.runOnUiThreadBlocking(mSmsDialog::smsReceived);
-        Assert.assertTrue(confirmButton.isEnabled());
+        // Simulates the user clicking the "Cancel" button.
+        TestTouchUtils.performClickOnMainSync(
+                InstrumentationRegistry.getInstrumentation(), cancelButton);
+        mCancelButtonClickedCallback.waitForCallback(0, 1);
     }
 
     @Test
     @LargeTest
-    public void testClickCancelButton() throws Throwable {
+    public void testSmsReceivedUserClickingCancelButton() throws Throwable {
         Dialog dialog = mSmsDialog.getDialogForTesting();
+
+        ProgressBar progressBar = (ProgressBar) dialog.findViewById(R.id.progress);
+        ImageView doneIcon = (ImageView) dialog.findViewById(R.id.done_icon);
+        ImageView errorIcon = (ImageView) dialog.findViewById(R.id.error_icon);
+        TextView status = (TextView) dialog.findViewById(R.id.status);
         Button cancelButton = (Button) dialog.findViewById(R.id.cancel_button);
+        Button confirmButton = (Button) dialog.findViewById(R.id.confirm_or_try_again_button);
 
+        Assert.assertEquals(View.VISIBLE, progressBar.getVisibility());
+        Assert.assertEquals(View.GONE, doneIcon.getVisibility());
+        Assert.assertEquals(View.GONE, errorIcon.getVisibility());
+        Assert.assertEquals(
+                mActivityTestRule.getActivity().getString(R.string.sms_dialog_status_waiting),
+                status.getText().toString());
+        Assert.assertTrue(cancelButton.isEnabled());
+        Assert.assertEquals(mActivityTestRule.getActivity().getString(R.string.confirm),
+                confirmButton.getText().toString());
+        Assert.assertFalse(confirmButton.isEnabled());
+
+        // Simulates the SMS being received.
+        TestThreadUtils.runOnUiThreadBlocking(mSmsDialog::smsReceived);
+
+        Assert.assertEquals(View.GONE, progressBar.getVisibility());
+        Assert.assertEquals(View.VISIBLE, doneIcon.getVisibility());
+        Assert.assertEquals(View.GONE, errorIcon.getVisibility());
+        Assert.assertEquals(
+                mActivityTestRule.getActivity().getString(R.string.sms_dialog_status_sms_received),
+                status.getText().toString());
+        Assert.assertTrue(cancelButton.isEnabled());
+        Assert.assertTrue(confirmButton.isEnabled());
+
+        // Simulates the user clicking the "Cancel" button.
         TestTouchUtils.performClickOnMainSync(
                 InstrumentationRegistry.getInstrumentation(), cancelButton);
-
         mCancelButtonClickedCallback.waitForCallback(0, 1);
     }
 
     @Test
     @LargeTest
-    public void testClickConfirmButton() throws Throwable {
+    public void testSmsReceivedUserClickingConfirmButton() throws Throwable {
         Dialog dialog = mSmsDialog.getDialogForTesting();
-        Button confirmButton = (Button) dialog.findViewById(R.id.confirm_button);
 
+        Button confirmButton = (Button) dialog.findViewById(R.id.confirm_or_try_again_button);
+
+        // Simulates the SMS being received.
+        TestThreadUtils.runOnUiThreadBlocking(mSmsDialog::smsReceived);
+
+        // Simulates the user clicking the "Confirm" button.
         TestTouchUtils.performClickOnMainSync(
                 InstrumentationRegistry.getInstrumentation(), confirmButton);
-
         mConfirmButtonClickedCallback.waitForCallback(0, 1);
     }
 
+    @Test
+    @LargeTest
+    public void testSmsTimeout() throws Throwable {
+        Dialog dialog = mSmsDialog.getDialogForTesting();
+
+        ProgressBar progressBar = (ProgressBar) dialog.findViewById(R.id.progress);
+        ImageView doneIcon = (ImageView) dialog.findViewById(R.id.done_icon);
+        ImageView errorIcon = (ImageView) dialog.findViewById(R.id.error_icon);
+        TextView status = (TextView) dialog.findViewById(R.id.status);
+        Button cancelButton = (Button) dialog.findViewById(R.id.cancel_button);
+        Button tryAgainButton = (Button) dialog.findViewById(R.id.confirm_or_try_again_button);
+
+        // Simulates receiving the SMS having timed out.
+        TestThreadUtils.runOnUiThreadBlocking(mSmsDialog::smsTimeout);
+
+        Assert.assertEquals(View.GONE, progressBar.getVisibility());
+        Assert.assertEquals(View.GONE, doneIcon.getVisibility());
+        Assert.assertEquals(View.VISIBLE, errorIcon.getVisibility());
+        Assert.assertEquals(
+                mActivityTestRule.getActivity().getString(R.string.sms_dialog_status_timeout),
+                status.getText().toString());
+        Assert.assertEquals(View.GONE, cancelButton.getVisibility());
+        Assert.assertEquals(mActivityTestRule.getActivity().getString(R.string.try_again),
+                tryAgainButton.getText().toString());
+        Assert.assertTrue(tryAgainButton.isEnabled());
+
+        // Simulates the user clicking the "Try again" button.
+        TestTouchUtils.performClickOnMainSync(
+                InstrumentationRegistry.getInstrumentation(), tryAgainButton);
+        mTryAgainButtonClickedCallback.waitForCallback(0, 1);
+    }
+
     @Test
     @LargeTest
     public void testStatusRowChangesWhenMessageReceived() {

chrome/browser/ui/android/sms/sms_dialog_android.cc

@@ -53,6 +53,10 @@ void SmsDialogAndroid::SmsReceived() {
   Java_SmsReceiverDialog_smsReceived(AttachCurrentThread(), java_dialog_);
 }
 
+void SmsDialogAndroid::SmsTimeout() {
+  Java_SmsReceiverDialog_smsTimeout(AttachCurrentThread(), java_dialog_);
+}
+
 void SmsDialogAndroid::OnEvent(JNIEnv* env, jint event_type) {
   std::move(handler_).Run(static_cast<Event>(event_type));
 }

chrome/browser/ui/android/sms/sms_dialog_android.h

@@ -22,6 +22,7 @@ class SmsDialogAndroid : public content::SmsDialog {
   void Open(content::RenderFrameHost*, EventHandler handler) override;
   void Close() override;
   void SmsReceived() override;
+  void SmsTimeout() override;
 
   // Report the user's action through |event_type|.
   void OnEvent(JNIEnv* env, jint event_type);

content/browser/sms/sms_browsertest.cc

@@ -472,6 +472,23 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TimesOut) {
   BrowserMainLoop::GetInstance()->SetSmsProviderForTesting(
       base::WrapUnique(provider));
 
+  StrictMock<MockSmsWebContentsDelegate> delegate;
+  shell()->web_contents()->SetDelegate(&delegate);
+
+  auto* dialog = new StrictMock<MockSmsDialog>();
+
+  EXPECT_CALL(delegate, CreateSmsDialog(_))
+      .WillOnce(Return(ByMove(base::WrapUnique(dialog))));
+
+  EXPECT_CALL(*dialog, Open(_, _))
+      .WillOnce(Invoke([](content::RenderFrameHost*,
+                          content::SmsDialog::EventHandler event_handler) {
+        // Simulates the user pressing "Try again".
+        std::move(event_handler).Run(SmsDialog::Event::kTimeout);
+      }));
+
+  EXPECT_CALL(*dialog, Close()).WillOnce(Return());
+
   std::string script = R"(
     (async () => {
       try {

content/browser/sms/sms_service.cc

@@ -117,9 +117,7 @@ void SmsService::OnTimeout() {
   DCHECK(callback_);
   DCHECK(!timer_.IsRunning());
 
-  std::move(callback_).Run(blink::mojom::SmsStatus::kTimeout, base::nullopt);
-
-  Dismiss();
+  prompt_->SmsTimeout();
 }
 
 void SmsService::OnConfirm() {
@@ -144,6 +142,12 @@ void SmsService::OnCancel() {
   Process(SmsStatus::kCancelled, base::nullopt);
 }
 
+void SmsService::OnTryAgain() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  Process(SmsStatus::kTimeout, base::nullopt);
+}
+
 void SmsService::OnEvent(SmsDialog::Event event_type) {
   switch (event_type) {
     case SmsDialog::Event::kConfirm:
@@ -152,6 +156,9 @@ void SmsService::OnEvent(SmsDialog::Event event_type) {
     case SmsDialog::Event::kCancel:
       OnCancel();
       return;
+    case SmsDialog::Event::kTimeout:
+      OnTryAgain();
+      return;
   }
   DVLOG(1) << "Unsupported event type: " << event_type;
   NOTREACHED();

content/browser/sms/sms_service.h

@@ -65,6 +65,8 @@ class CONTENT_EXPORT SmsService
   void OnConfirm();
   // Called when the user manually dismisses the dialog.
   void OnCancel();
+  // Callback when the user manually clicks 'Try again' button after a timeout.
+  void OnTryAgain();
 
   // Handles the user's action.
   void OnEvent(SmsDialog::Event event_type);

content/browser/sms/sms_service_unittest.cc

@@ -349,6 +349,18 @@ TEST_F(SmsServiceTest, Timeout) {
             loop.Quit();
           }));
 
+  auto* dialog = new NiceMock<MockSmsDialog>();
+
+  EXPECT_CALL(*service.delegate(), CreateSmsDialog(_))
+      .WillOnce(Return(ByMove(base::WrapUnique(dialog))));
+
+  EXPECT_CALL(*dialog, Open(main_rfh(), _))
+      .WillOnce(Invoke([](content::RenderFrameHost*,
+                          content::SmsDialog::EventHandler event_handler) {
+        // Simulates the user pressing "Try again".
+        std::move(event_handler).Run(SmsDialog::Event::kTimeout);
+      }));
+
   loop.Run();
 }
 
@@ -476,7 +488,12 @@ TEST_F(SmsServiceTest, TimeoutClosesDialog) {
 
   // Deliberately avoid calling the on_cancel callback, to simulate the
   // sms being timed out before the user cancels it.
-  EXPECT_CALL(*dialog, Open(main_rfh(), _)).WillOnce(Return());
+  EXPECT_CALL(*dialog, Open(main_rfh(), _))
+      .WillOnce(Invoke([](content::RenderFrameHost*,
+                          content::SmsDialog::EventHandler event_handler) {
+        // Simulates the user pressing "Try again".
+        std::move(event_handler).Run(SmsDialog::Event::kTimeout);
+      }));
 
   EXPECT_CALL(*dialog, Close()).WillOnce(Return());
 
@@ -505,7 +522,12 @@ TEST_F(SmsServiceTest, SecondRequestTimesOutEarlierThanFirstRequest) {
             hdl = std::move(handler);
           }));
 
-  EXPECT_CALL(*dialog2, Open(main_rfh(), _)).Times(1);
+  EXPECT_CALL(*dialog2, Open(main_rfh(), _))
+      .WillOnce(Invoke([](content::RenderFrameHost*,
+                          content::SmsDialog::EventHandler event_handler) {
+        // Simulates the user pressing "Try again".
+        std::move(event_handler).Run(SmsDialog::Event::kTimeout);
+      }));
 
   EXPECT_CALL(*dialog1, SmsReceived()).WillOnce(Invoke([&hdl]() {
     std::move(hdl).Run(SmsDialog::Event::kConfirm);

content/browser/sms/test/mock_sms_dialog.h

@@ -19,6 +19,7 @@ class MockSmsDialog : public SmsDialog {
   MOCK_METHOD2(Open, void(RenderFrameHost*, EventHandler handler));
   MOCK_METHOD0(Close, void());
   MOCK_METHOD0(SmsReceived, void());
+  MOCK_METHOD0(SmsTimeout, void());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockSmsDialog);

content/public/browser/sms_dialog.h

@@ -23,6 +23,8 @@ class CONTENT_EXPORT SmsDialog {
     kConfirm = 0,
     // User manually dismissed the SMS dialog.
     kCancel = 1,
+    // User manually clicked 'Try again' button after a timeout.
+    kTimeout = 2,
   };
 
   using EventHandler = base::OnceCallback<void(Event)>;
@@ -32,6 +34,7 @@ class CONTENT_EXPORT SmsDialog {
   virtual void Open(RenderFrameHost* host, EventHandler handler) = 0;
   virtual void Close() = 0;
   virtual void SmsReceived() = 0;
+  virtual void SmsTimeout() = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SmsDialog);