[sms] Refactor SmsDialog to use event handler

This CL refactors SmsDialog to use event handler. An enum SmsDialogEvent
is added to SmsDialog class and can be used by both C++ and Java code.

Bug: 985441
Change-Id: I1ecc4e70609479124c80a7166a9551ffa77566a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1737875
Commit-Queue: Jun Cai <juncai@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686524}

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

@@ -23,6 +23,7 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.chrome.R;
+import org.chromium.content_public.browser.sms.Event;
 import org.chromium.ui.base.WindowAndroid;
 
 /**
@@ -72,13 +73,13 @@ public class SmsReceiverDialog {
         Button cancelButton = (Button) dialogContainer.findViewById(R.id.cancel_button);
         cancelButton.setOnClickListener(v -> {
             assert mNativeSmsDialogAndroid != 0;
-            SmsReceiverDialogJni.get().onCancel(mNativeSmsDialogAndroid);
+            SmsReceiverDialogJni.get().onEvent(mNativeSmsDialogAndroid, Event.CANCEL);
         });
 
         mConfirmButton = (Button) dialogContainer.findViewById(R.id.confirm_button);
         mConfirmButton.setOnClickListener(v -> {
             assert mNativeSmsDialogAndroid != 0;
-            SmsReceiverDialogJni.get().onConfirm(mNativeSmsDialogAndroid);
+            SmsReceiverDialogJni.get().onEvent(mNativeSmsDialogAndroid, Event.CONFIRM);
         });
 
         mDialog = new Dialog(mActivity);
@@ -149,7 +150,6 @@ public class SmsReceiverDialog {
 
     @NativeMethods
     interface Natives {
-        void onCancel(long nativeSmsDialogAndroid);
-        void onConfirm(long nativeSmsDialogAndroid);
+        void onEvent(long nativeSmsDialogAndroid, int eventType);
     }
 }

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

@@ -27,6 +27,7 @@ import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.content_public.browser.sms.Event;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.content_public.browser.test.util.TestTouchUtils;
 import org.chromium.ui.base.ActivityWindowAndroid;
@@ -52,13 +53,17 @@ public class SmsReceiverDialogTest {
 
     private class TestSmsReceiverDialogJni implements SmsReceiverDialog.Natives {
         @Override
-        public void onCancel(long nativeSmsDialogAndroid) {
-            mCancelButtonClickedCallback.notifyCalled();
-        }
-
-        @Override
-        public void onConfirm(long nativeSmsDialogAndroid) {
-            mConfirmButtonClickedCallback.notifyCalled();
+        public void onEvent(long nativeSmsDialogAndroid, int eventType) {
+            switch (eventType) {
+                case Event.CANCEL:
+                    mCancelButtonClickedCallback.notifyCalled();
+                    return;
+                case Event.CONFIRM:
+                    mConfirmButtonClickedCallback.notifyCalled();
+                    return;
+                default:
+                    assert false : "|eventType| is invalid";
+            }
         }
     }
 

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

@@ -32,10 +32,8 @@ SmsDialogAndroid::~SmsDialogAndroid() {
 }
 
 void SmsDialogAndroid::Open(content::RenderFrameHost* host,
-                            base::OnceClosure on_confirm,
-                            base::OnceClosure on_cancel) {
-  on_confirm_ = std::move(on_confirm);
-  on_cancel_ = std::move(on_cancel);
+                            EventHandler handler) {
+  handler_ = std::move(handler);
 
   content::WebContents* web_contents =
       content::WebContents::FromRenderFrameHost(host);
@@ -55,10 +53,6 @@ void SmsDialogAndroid::SmsReceived() {
   Java_SmsReceiverDialog_smsReceived(AttachCurrentThread(), java_dialog_);
 }
 
-void SmsDialogAndroid::OnConfirm(JNIEnv* env) {
-  std::move(on_confirm_).Run();
-}
-
-void SmsDialogAndroid::OnCancel(JNIEnv* env) {
-  std::move(on_cancel_).Run();
+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

@@ -19,22 +19,16 @@ class SmsDialogAndroid : public content::SmsDialog {
   explicit SmsDialogAndroid(const url::Origin& origin);
   ~SmsDialogAndroid() override;
 
-  void Open(content::RenderFrameHost*,
-            base::OnceClosure on_confirm,
-            base::OnceClosure on_cancel) override;
+  void Open(content::RenderFrameHost*, EventHandler handler) override;
   void Close() override;
   void SmsReceived() override;
 
-  // Report the user manually clicks the 'Confirm' button.
-  void OnConfirm(JNIEnv* env);
-
-  // Report the user manually dismisses the SMS dialog.
-  void OnCancel(JNIEnv* env);
+  // Report the user's action through |event_type|.
+  void OnEvent(JNIEnv* env, jint event_type);
 
  private:
   base::android::ScopedJavaGlobalRef<jobject> java_dialog_;
-  base::OnceClosure on_confirm_;
-  base::OnceClosure on_cancel_;
+  EventHandler handler_;
   DISALLOW_COPY_AND_ASSIGN(SmsDialogAndroid);
 };
 

content/browser/sms/sms_browsertest.cc

@@ -72,20 +72,19 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Receive) {
 
   auto* dialog = new NiceMock<MockSmsDialog>();
 
-  base::OnceClosure on_confirm_callback;
+  SmsDialog::EventHandler hdl;
 
   EXPECT_CALL(delegate_, CreateSmsDialog(_))
       .WillOnce(Return(ByMove(base::WrapUnique(dialog))));
 
-  EXPECT_CALL(*dialog, Open(_, _, _))
-      .WillOnce(Invoke([&on_confirm_callback](content::RenderFrameHost*,
-                                              base::OnceClosure on_confirm,
-                                              base::OnceClosure on_cancel) {
-        on_confirm_callback = std::move(on_confirm);
-      }));
+  EXPECT_CALL(*dialog, Open(_, _))
+      .WillOnce(
+          Invoke([&hdl](RenderFrameHost*, SmsDialog::EventHandler handler) {
+            hdl = std::move(handler);
+          }));
 
-  EXPECT_CALL(*dialog, SmsReceived()).WillOnce(Invoke([&on_confirm_callback]() {
-    std::move(on_confirm_callback).Run();
+  EXPECT_CALL(*dialog, SmsReceived()).WillOnce(Invoke([&hdl]() {
+    std::move(hdl).Run(SmsDialog::Event::kConfirm);
   }));
 
   auto* provider = new NiceMock<MockSmsProvider>();
@@ -117,20 +116,19 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, AtMostOnePendingSmsRequest) {
 
   auto* dialog = new NiceMock<MockSmsDialog>();
 
-  base::OnceClosure on_confirm_callback;
+  SmsDialog::EventHandler hdl;
 
   EXPECT_CALL(delegate_, CreateSmsDialog(_))
       .WillOnce(Return(ByMove(base::WrapUnique(dialog))));
 
-  EXPECT_CALL(*dialog, Open(_, _, _))
-      .WillOnce(Invoke([&on_confirm_callback](content::RenderFrameHost*,
-                                              base::OnceClosure on_confirm,
-                                              base::OnceClosure on_cancel) {
-        on_confirm_callback = std::move(on_confirm);
-      }));
+  EXPECT_CALL(*dialog, Open(_, _))
+      .WillOnce(
+          Invoke([&hdl](RenderFrameHost*, SmsDialog::EventHandler handler) {
+            hdl = std::move(handler);
+          }));
 
-  EXPECT_CALL(*dialog, SmsReceived()).WillOnce(Invoke([&on_confirm_callback]() {
-    std::move(on_confirm_callback).Run();
+  EXPECT_CALL(*dialog, SmsReceived()).WillOnce(Invoke([&hdl]() {
+    std::move(hdl).Run(SmsDialog::Event::kConfirm);
   }));
 
   auto* provider = new NiceMock<MockSmsProvider>();
@@ -244,7 +242,8 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsSameOrigin) {
     true
   )";
 
-  base::OnceClosure on_confirm1, on_confirm2;
+  SmsDialog::EventHandler hdl_1;
+  SmsDialog::EventHandler hdl_2;
 
   {
     base::RunLoop loop;
@@ -260,12 +259,11 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsSameOrigin) {
       loop.Quit();
     }));
 
-    EXPECT_CALL(*dialog, Open(_, _, _))
-        .WillOnce(Invoke([&on_confirm1](content::RenderFrameHost*,
-                                        base::OnceClosure on_confirm,
-                                        base::OnceClosure on_cancel) {
-          on_confirm1 = std::move(on_confirm);
-        }));
+    EXPECT_CALL(*dialog, Open(_, _))
+        .WillOnce(
+            Invoke([&hdl_1](RenderFrameHost*, SmsDialog::EventHandler handler) {
+              hdl_1 = std::move(handler);
+            }));
 
     // First tab registers an observer.
     EXPECT_EQ(true, EvalJs(tab1, script));
@@ -287,12 +285,11 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsSameOrigin) {
       loop.Quit();
     }));
 
-    EXPECT_CALL(*dialog, Open(_, _, _))
-        .WillOnce(Invoke([&on_confirm2](content::RenderFrameHost*,
-                                        base::OnceClosure on_confirm,
-                                        base::OnceClosure on_cancel) {
-          on_confirm2 = std::move(on_confirm);
-        }));
+    EXPECT_CALL(*dialog, Open(_, _))
+        .WillOnce(
+            Invoke([&hdl_2](RenderFrameHost*, SmsDialog::EventHandler handler) {
+              hdl_2 = std::move(handler);
+            }));
 
     // Second tab registers an observer.
     EXPECT_EQ(true, EvalJs(tab2, script));
@@ -304,7 +301,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsSameOrigin) {
 
   provider->NotifyReceive(url::Origin::Create(url), "hello1");
 
-  std::move(on_confirm1).Run();
+  std::move(hdl_1).Run(SmsDialog::Event::kConfirm);
 
   EXPECT_EQ("hello1", EvalJs(tab1, "sms"));
 
@@ -312,7 +309,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsSameOrigin) {
 
   provider->NotifyReceive(url::Origin::Create(url), "hello2");
 
-  std::move(on_confirm2).Run();
+  std::move(hdl_2).Run(SmsDialog::Event::kConfirm);
 
   EXPECT_EQ("hello2", EvalJs(tab2, "sms"));
 
@@ -356,26 +353,24 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsDifferentOrigin) {
   auto* dialog1 = new NiceMock<MockSmsDialog>();
   auto* dialog2 = new NiceMock<MockSmsDialog>();
 
-  base::OnceClosure on_confirm_callback1;
-  base::OnceClosure on_confirm_callback2;
+  SmsDialog::EventHandler hdl_1;
+  SmsDialog::EventHandler hdl_2;
 
   EXPECT_CALL(delegate_, CreateSmsDialog(_))
       .WillOnce(Return(ByMove(base::WrapUnique(dialog1))))
       .WillOnce(Return(ByMove(base::WrapUnique(dialog2))));
 
-  EXPECT_CALL(*dialog1, Open(_, _, _))
-      .WillOnce(Invoke([&on_confirm_callback1](content::RenderFrameHost*,
-                                               base::OnceClosure on_confirm,
-                                               base::OnceClosure on_cancel) {
-        on_confirm_callback1 = std::move(on_confirm);
-      }));
+  EXPECT_CALL(*dialog1, Open(_, _))
+      .WillOnce(
+          Invoke([&hdl_1](RenderFrameHost*, SmsDialog::EventHandler handler) {
+            hdl_1 = std::move(handler);
+          }));
 
-  EXPECT_CALL(*dialog2, Open(_, _, _))
-      .WillOnce(Invoke([&on_confirm_callback2](content::RenderFrameHost*,
-                                               base::OnceClosure on_confirm,
-                                               base::OnceClosure on_cancel) {
-        on_confirm_callback2 = std::move(on_confirm);
-      }));
+  EXPECT_CALL(*dialog2, Open(_, _))
+      .WillOnce(
+          Invoke([&hdl_2](RenderFrameHost*, SmsDialog::EventHandler handler) {
+            hdl_2 = std::move(handler);
+          }));
 
   EXPECT_EQ(true, EvalJs(tab1, script));
   EXPECT_EQ(true, EvalJs(tab2, script));
@@ -386,7 +381,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsDifferentOrigin) {
 
   provider->NotifyReceive(url::Origin::Create(url1), "hello1");
 
-  std::move(on_confirm_callback1).Run();
+  std::move(hdl_1).Run(SmsDialog::Event::kConfirm);
 
   EXPECT_EQ("hello1", EvalJs(tab1, "sms"));
 
@@ -394,7 +389,7 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, TwoTabsDifferentOrigin) {
 
   provider->NotifyReceive(url::Origin::Create(url2), "hello2");
 
-  std::move(on_confirm_callback2).Run();
+  std::move(hdl_2).Run(SmsDialog::Event::kConfirm);
 
   EXPECT_EQ("hello2", EvalJs(tab2, "sms"));
 
@@ -446,13 +441,11 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Cancels) {
   EXPECT_CALL(delegate, CreateSmsDialog(_))
       .WillOnce(Return(ByMove(base::WrapUnique(dialog))));
 
-  EXPECT_CALL(*dialog, Open(_, _, _))
-      .WillOnce(
-          Invoke([](content::RenderFrameHost*, base::OnceClosure on_confirm,
-                    base::OnceClosure on_cancel) {
-            // Simulates the user pressing "cancel".
-            std::move(on_cancel).Run();
-          }));
+  EXPECT_CALL(*dialog, Open(_, _))
+      .WillOnce(Invoke([](RenderFrameHost*, SmsDialog::EventHandler handler) {
+        // Simulates the user pressing "cancel".
+        std::move(handler).Run(SmsDialog::Event::kCancel);
+      }));
 
   EXPECT_CALL(*dialog, Close()).WillOnce(Return());
 

content/browser/sms/sms_service.cc

@@ -10,6 +10,7 @@
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
+#include "base/logging.h"
 #include "base/optional.h"
 #include "content/browser/sms/sms_metrics.h"
 #include "content/public/browser/sms_dialog.h"
@@ -143,16 +144,27 @@ void SmsService::OnCancel() {
   Process(SmsStatus::kCancelled, base::nullopt);
 }
 
+void SmsService::OnEvent(SmsDialog::Event event_type) {
+  switch (event_type) {
+    case SmsDialog::Event::kConfirm:
+      OnConfirm();
+      return;
+    case SmsDialog::Event::kCancel:
+      OnCancel();
+      return;
+  }
+  DVLOG(1) << "Unsupported event type: " << event_type;
+  NOTREACHED();
+}
+
 void SmsService::Prompt() {
   WebContents* web_contents =
-      content::WebContents::FromRenderFrameHost(render_frame_host());
+      WebContents::FromRenderFrameHost(render_frame_host());
   const url::Origin origin = render_frame_host()->GetLastCommittedOrigin();
   prompt_ = web_contents->GetDelegate()->CreateSmsDialog(origin);
   if (prompt_) {
-    prompt_->Open(
-        render_frame_host(),
-        base::BindOnce(&SmsService::OnConfirm, base::Unretained(this)),
-        base::BindOnce(&SmsService::OnCancel, base::Unretained(this)));
+    prompt_->Open(render_frame_host(),
+                  base::BindOnce(&SmsService::OnEvent, base::Unretained(this)));
   }
 }
 

content/browser/sms/sms_service.h

@@ -15,6 +15,7 @@
 #include "content/browser/sms/sms_provider.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/frame_service_base.h"
+#include "content/public/browser/sms_dialog.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "third_party/blink/public/mojom/sms/sms_receiver.mojom.h"
 #include "url/origin.h"
@@ -60,11 +61,14 @@ class CONTENT_EXPORT SmsService
 
   // Callback when the |timer_| times out.
   void OnTimeout();
-  // Callback when the user manually clicks 'Confirm' button.
+  // Called when the user manually clicks 'Confirm' button.
   void OnConfirm();
-  // Callback when the user manually dismisses the dialog.
+  // Called when the user manually dismisses the dialog.
   void OnCancel();
 
+  // Handles the user's action.
+  void OnEvent(SmsDialog::Event event_type);
+
   // |sms_provider_| is safe because all instances of SmsProvider are owned
   // by a SmsKeyedService, which is owned by a Profile, which transitively
   // owns SmsServices.

content/browser/sms/sms_service_unittest.cc

@@ -78,23 +78,22 @@ class Service {
   NiceMock<MockSmsWebContentsDelegate>* delegate() { return &delegate_; }
   NiceMock<MockSmsProvider>* provider() { return &provider_; }
 
-  void SetupSmsDialog(content::RenderFrameHost* rfh) {
+  void SetupSmsDialog(RenderFrameHost* rfh) {
     auto* dialog = new NiceMock<MockSmsDialog>();
 
     EXPECT_CALL(*delegate(), CreateSmsDialog(_))
         .WillOnce(Return(ByMove(base::WrapUnique(dialog))));
 
-    EXPECT_CALL(*dialog, Open(rfh, _, _))
+    EXPECT_CALL(*dialog, Open(rfh, _))
         .WillOnce(
-            Invoke([&](content::RenderFrameHost*, base::OnceClosure on_confirm,
-                       base::OnceClosure on_cancel) {
-              on_confirm_callback_ = std::move(on_confirm);
+            Invoke([&](RenderFrameHost*, SmsDialog::EventHandler handler) {
+              handler_ = std::move(handler);
             }));
 
     EXPECT_CALL(*dialog, SmsReceived()).WillOnce(Invoke([&]() {
       // Simulates user clicking the "Confirm" button to verify received
       // sms.
-      std::move(on_confirm_callback_).Run();
+      std::move(handler_).Run(SmsDialog::Event::kConfirm);
     }));
 
     EXPECT_CALL(*dialog, Close()).WillOnce(Return());
@@ -113,7 +112,7 @@ class Service {
   NiceMock<MockSmsProvider> provider_;
   blink::mojom::SmsReceiverPtr service_ptr_;
   std::unique_ptr<SmsService> service_;
-  base::OnceClosure on_confirm_callback_;
+  SmsDialog::EventHandler handler_;
 };
 
 class SmsServiceTest : public RenderViewHostTestHarness {
@@ -441,13 +440,11 @@ TEST_F(SmsServiceTest, Cancel) {
   EXPECT_CALL(*service.delegate(), CreateSmsDialog(_))
       .WillOnce(Return(ByMove(base::WrapUnique(dialog))));
 
-  EXPECT_CALL(*dialog, Open(main_rfh(), _, _))
-      .WillOnce(
-          Invoke([](content::RenderFrameHost*, base::OnceClosure on_confirm,
-                    base::OnceClosure on_cancel) {
-            // Simulates the user pressing "Cancel".
-            std::move(on_cancel).Run();
-          }));
+  EXPECT_CALL(*dialog, Open(main_rfh(), _))
+      .WillOnce(Invoke([](RenderFrameHost*, SmsDialog::EventHandler handler) {
+        // Simulates the user pressing "Cancel".
+        std::move(handler).Run(SmsDialog::Event::kCancel);
+      }));
 
   loop.Run();
 
@@ -479,7 +476,7 @@ 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(Return());
 
   EXPECT_CALL(*dialog, Close()).WillOnce(Return());
 
@@ -496,24 +493,23 @@ TEST_F(SmsServiceTest, SecondRequestTimesOutEarlierThanFirstRequest) {
   auto* dialog1 = new NiceMock<MockSmsDialog>();
   auto* dialog2 = new NiceMock<MockSmsDialog>();
 
-  base::OnceClosure on_confirm_callback;
+  SmsDialog::EventHandler hdl;
 
   EXPECT_CALL(*service.delegate(), CreateSmsDialog(_))
       .WillOnce(Return(ByMove(base::WrapUnique(dialog1))))
       .WillOnce(Return(ByMove(base::WrapUnique(dialog2))));
 
-  EXPECT_CALL(*dialog1, Open(main_rfh(), _, _))
-      .WillOnce(Invoke([&on_confirm_callback](content::RenderFrameHost*,
-                                              base::OnceClosure on_confirm,
-                                              base::OnceClosure on_cancel) {
-        on_confirm_callback = std::move(on_confirm);
-      }));
+  EXPECT_CALL(*dialog1, Open(main_rfh(), _))
+      .WillOnce(
+          Invoke([&hdl](RenderFrameHost*, SmsDialog::EventHandler handler) {
+            hdl = std::move(handler);
+          }));
 
-  EXPECT_CALL(*dialog2, Open(main_rfh(), _, _)).Times(1);
+  EXPECT_CALL(*dialog2, Open(main_rfh(), _)).Times(1);
 
-  EXPECT_CALL(*dialog1, SmsReceived())
-      .WillOnce(Invoke(
-          [&on_confirm_callback]() { std::move(on_confirm_callback).Run(); }));
+  EXPECT_CALL(*dialog1, SmsReceived()).WillOnce(Invoke([&hdl]() {
+    std::move(hdl).Run(SmsDialog::Event::kConfirm);
+  }));
 
   EXPECT_CALL(*service.provider(), Retrieve())
       .WillOnce(Invoke([&service]() {
@@ -601,13 +597,11 @@ TEST_F(SmsServiceTest, RecordMetricsForCancelOnSuccess) {
     EXPECT_CALL(*service.delegate(), CreateSmsDialog(_))
         .WillOnce(Return(ByMove(base::WrapUnique(dialog))));
 
-    EXPECT_CALL(*dialog, Open(main_rfh(), _, _))
-        .WillOnce(
-            Invoke([](content::RenderFrameHost*, base::OnceClosure on_confirm,
-                      base::OnceClosure on_cancel) {
-              // Simulates the user pressing "Cancel".
-              std::move(on_cancel).Run();
-            }));
+    EXPECT_CALL(*dialog, Open(main_rfh(), _))
+        .WillOnce(Invoke([](RenderFrameHost*, SmsDialog::EventHandler handler) {
+          // Simulates the user pressing "Cancel".
+          std::move(handler).Run(SmsDialog::Event::kCancel);
+        }));
 
     loop.Run();
 
@@ -637,21 +631,20 @@ TEST_F(SmsServiceTest, RecordMetricsForCancelOnSuccess) {
             }));
 
     auto* dialog = new NiceMock<MockSmsDialog>();
-    base::OnceClosure on_cancel_callback;
+    SmsDialog::EventHandler hdl;
 
     EXPECT_CALL(*service.delegate(), CreateSmsDialog(_))
         .WillOnce(Return(ByMove(base::WrapUnique(dialog))));
 
-    EXPECT_CALL(*dialog, Open(main_rfh(), _, _))
-        .WillOnce(Invoke([&on_cancel_callback](content::RenderFrameHost*,
-                                               base::OnceClosure on_confirm,
-                                               base::OnceClosure on_cancel) {
-          on_cancel_callback = std::move(on_cancel);
-        }));
+    EXPECT_CALL(*dialog, Open(main_rfh(), _))
+        .WillOnce(
+            Invoke([&hdl](RenderFrameHost*, SmsDialog::EventHandler handler) {
+              hdl = std::move(handler);
+            }));
 
     EXPECT_CALL(*dialog, SmsReceived()).WillOnce(Invoke([&]() {
       // Simulates user clicking the "Cancel" once the SMS has been received.
-      std::move(on_cancel_callback).Run();
+      std::move(hdl).Run(SmsDialog::Event::kCancel);
     }));
 
     loop.Run();

content/browser/sms/test/mock_sms_dialog.h

@@ -16,8 +16,7 @@ class MockSmsDialog : public SmsDialog {
   MockSmsDialog();
   ~MockSmsDialog() override;
 
-  MOCK_METHOD3(Open,
-               void(RenderFrameHost*, base::OnceClosure, base::OnceClosure));
+  MOCK_METHOD2(Open, void(RenderFrameHost*, EventHandler handler));
   MOCK_METHOD0(Close, void());
   MOCK_METHOD0(SmsReceived, void());
 

content/public/android/BUILD.gn

@@ -364,6 +364,7 @@ java_cpp_enum("content_public_android_java_enums_srcjar") {
     "//content/public/browser/browsing_data_remover.h",
     "//content/public/browser/invalidate_type.h",
     "//content/public/browser/navigation_controller.h",
+    "//content/public/browser/sms_dialog.h",
     "//content/public/common/browser_controls_state.h",
     "//content/public/common/console_message_level.h",
     "//content/public/common/screen_orientation_values.h",

content/public/browser/sms_dialog.h

@@ -16,11 +16,20 @@ class RenderFrameHost;
 // messages.
 class CONTENT_EXPORT SmsDialog {
  public:
+  // A Java counterpart will be generated for this enum.
+  // GENERATED_JAVA_ENUM_PACKAGE: org.chromium.content_public.browser.sms
+  enum Event {
+    // User manually clicked the 'Confirm' button.
+    kConfirm = 0,
+    // User manually dismissed the SMS dialog.
+    kCancel = 1,
+  };
+
+  using EventHandler = base::OnceCallback<void(Event)>;
+
   SmsDialog() = default;
   virtual ~SmsDialog() = default;
-  virtual void Open(content::RenderFrameHost* host,
-                    base::OnceClosure on_confirm,
-                    base::OnceClosure on_cancel) = 0;
+  virtual void Open(RenderFrameHost* host, EventHandler handler) = 0;
   virtual void Close() = 0;
   virtual void SmsReceived() = 0;