[WebOTP] Fix user declining prompts in CodeBrowser API

During the initial implementation of recording |UserCancelled| as an
|Outcome|, we didn't use |kUserCancelled| because it could not resolve
the promise upon user dismissing the prompt. We used |kCancelled| as
a temporary workaround to resolve the promise.

This patch removes the workaround because we are now able to resolve
promises after crrev.com/c/2569946.

Bug: 1138454
Change-Id: Id91180b7624e5f0687b37a0864a1d64e406baf8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2579988
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835795}

content/browser/sms/sms_browsertest.cc

@@ -655,48 +655,6 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, SmsReceivedAfterTabIsClosed) {
   ExpectOutcomeUKM(url, blink::WebOTPServiceOutcome::kUnhandledRequest);
 }
 
-IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Cancels) {
-  base::HistogramTester histogram_tester;
-  GURL url = GetTestUrl(nullptr, "simple_page.html");
-  EXPECT_TRUE(NavigateToURL(shell(), url));
-
-  auto provider = std::make_unique<MockSmsProvider>();
-  MockSmsProvider* mock_provider_ptr = provider.get();
-  BrowserMainLoop::GetInstance()->SetSmsProviderForTesting(std::move(provider));
-
-  shell()->web_contents()->SetDelegate(&delegate_);
-
-  base::RunLoop ukm_loop;
-
-  ExpectSmsPrompt();
-
-  EXPECT_CALL(*mock_provider_ptr, Retrieve(_)).WillOnce(Invoke([&]() {
-    mock_provider_ptr->NotifyReceive(OriginList{url::Origin::Create(url)},
-                                     "hello", UserConsent::kNotObtained);
-    DismissPrompt();
-  }));
-
-  // Wait for UKM to be recorded to avoid race condition between outcome
-  // capture and evaluation.
-  ukm_recorder()->SetOnAddEntryCallback(Entry::kEntryName,
-                                        ukm_loop.QuitClosure());
-
-  EXPECT_TRUE(ExecJs(shell(), R"(
-     var error = navigator.credentials.get({otp: {transport: ["sms"]}})
-       .catch(({name}) => {
-         return name;
-       });
-    )"));
-
-  ukm_loop.Run();
-
-  EXPECT_EQ("AbortError", EvalJs(shell(), "error"));
-
-  content::FetchHistogramsFromChildProcesses();
-  ExpectOutcomeUKM(url, blink::WebOTPServiceOutcome::kCancelled);
-  histogram_tester.ExpectTotalCount("Blink.Sms.Receive.TimeCancel", 1);
-}
-
 IN_PROC_BROWSER_TEST_F(SmsBrowserTest, AbortAfterSmsRetrieval) {
   GURL url = GetTestUrl(nullptr, "simple_page.html");
   EXPECT_TRUE(NavigateToURL(shell(), url));

content/browser/sms/user_consent_handler.cc

@@ -17,7 +17,7 @@ NoopUserConsentHandler::~NoopUserConsentHandler() = default;
 void NoopUserConsentHandler::RequestUserConsent(
     const std::string& one_time_code,
     CompletionCallback on_complete) {
-  std::move(on_complete).Run(SmsStatus::kSuccess);
+  std::move(on_complete).Run(UserConsentResult::kApproved);
 }
 
 bool NoopUserConsentHandler::is_active() const {
@@ -39,7 +39,7 @@ void PromptBasedUserConsentHandler::RequestUserConsent(
   WebContents* web_contents =
       content::WebContents::FromRenderFrameHost(frame_host_);
   if (!web_contents->GetDelegate()) {
-    std::move(on_complete).Run(SmsStatus::kCancelled);
+    std::move(on_complete).Run(UserConsentResult::kNoDelegate);
     return;
   }
 
@@ -61,17 +61,12 @@ bool PromptBasedUserConsentHandler::is_async() const {
 
 void PromptBasedUserConsentHandler::OnConfirm() {
   is_prompt_open_ = false;
-  std::move(on_complete_).Run(SmsStatus::kSuccess);
+  std::move(on_complete_).Run(UserConsentResult::kApproved);
 }
 
 void PromptBasedUserConsentHandler::OnCancel() {
   is_prompt_open_ = false;
-  // TODO(crbug.com/1138454): This should be SmsStatus::kUserCancelled. However,
-  // due to the limitation of the user consent API, we currently do not resolve
-  // the promise with kUserCancelled. Reuse the existing enum kCancelled to make
-  // sure that dismissing a prompt works with this backend. We should correct
-  // the enum once the limitation is fixed.
-  std::move(on_complete_).Run(SmsStatus::kCancelled);
+  std::move(on_complete_).Run(UserConsentResult::kDenied);
 }
 
 }  // namespace content
\ No newline at end of file

content/browser/sms/user_consent_handler.h

@@ -14,7 +14,8 @@ namespace content {
 
 class RenderFrameHost;
 
-using CompletionCallback = base::OnceCallback<void(blink::mojom::SmsStatus)>;
+enum class UserConsentResult { kApproved, kDenied, kNoDelegate };
+using CompletionCallback = base::OnceCallback<void(UserConsentResult)>;
 
 class CONTENT_EXPORT UserConsentHandler {
  public:

content/browser/sms/user_consent_handler_unittest.cc

@@ -25,7 +25,6 @@ using url::Origin;
 namespace content {
 
 namespace {
-using blink::mojom::SmsStatus;
 
 const char kTestUrl[] = "https://testing.test";
 
@@ -101,8 +100,9 @@ TEST_F(PromptBasedUserConsentHandlerTest, ConfirmInvokedCallback) {
   PromptBasedUserConsentHandler consent_handler{main_rfh(), origin};
   EXPECT_FALSE(consent_handler.is_active());
   bool succeed;
-  auto callback = base::BindLambdaForTesting(
-      [&](SmsStatus status) { succeed = (status == SmsStatus::kSuccess); });
+  auto callback = base::BindLambdaForTesting([&](UserConsentResult result) {
+    succeed = (result == UserConsentResult::kApproved);
+  });
   consent_handler.RequestUserConsent("12345", std::move(callback));
   EXPECT_TRUE(consent_handler.is_active());
   ConfirmPrompt();
@@ -120,8 +120,9 @@ TEST_F(PromptBasedUserConsentHandlerTest, CancelingInvokedCallback) {
   PromptBasedUserConsentHandler consent_handler{main_rfh(), origin};
   EXPECT_FALSE(consent_handler.is_active());
   bool cancelled;
-  auto callback = base::BindLambdaForTesting(
-      [&](SmsStatus status) { cancelled = (status == SmsStatus::kCancelled); });
+  auto callback = base::BindLambdaForTesting([&](UserConsentResult result) {
+    cancelled = (result == UserConsentResult::kDenied);
+  });
   consent_handler.RequestUserConsent("12345", std::move(callback));
   EXPECT_TRUE(consent_handler.is_active());
   DismissPrompt();
@@ -143,8 +144,9 @@ TEST_F(PromptBasedUserConsentHandlerTest, CancelsWhenNoDelegate) {
 
   PromptBasedUserConsentHandler consent_handler{main_rfh(), origin};
   bool cancelled;
-  auto callback = base::BindLambdaForTesting(
-      [&](SmsStatus status) { cancelled = (status == SmsStatus::kCancelled); });
+  auto callback = base::BindLambdaForTesting([&](UserConsentResult result) {
+    cancelled = (result == UserConsentResult::kNoDelegate);
+  });
   consent_handler.RequestUserConsent("12345", std::move(callback));
   EXPECT_TRUE(cancelled);
 }

content/browser/sms/webotp_service.cc

@@ -190,7 +190,7 @@ void WebOTPService::OnReceive(const std::string& one_time_code,
   UserConsentHandler* consent_handler =
       CreateConsentHandler(consent_requirement);
   consent_handler->RequestUserConsent(
-      one_time_code, base::BindOnce(&WebOTPService::CompleteRequest,
+      one_time_code, base::BindOnce(&WebOTPService::OnUserConsentComplete,
                                     weak_ptr_factory_.GetWeakPtr()));
 }
 
@@ -295,6 +295,7 @@ void WebOTPService::CleanUp() {
   }
   start_time_ = base::TimeTicks();
   callback_.Reset();
+  prompt_failure_.reset();
   fetcher_->Unsubscribe(origin_list_, this);
 }
 
@@ -387,11 +388,26 @@ void WebOTPService::RecordMetrics(blink::mojom::SmsStatus status) {
     if (status == SmsStatus::kSuccess) {
       DCHECK(!receive_time_.is_null());
       RecordContinueOnSuccessTime(base::TimeTicks::Now() - receive_time_);
-    } else if (status == SmsStatus::kCancelled) {
+    } else if (prompt_failure_ &&
+               prompt_failure_.value() == FailureType::kPromptCancelled) {
       DCHECK(!receive_time_.is_null());
       RecordCancelOnSuccessTime(base::TimeTicks::Now() - receive_time_);
     }
   }
 }
 
+void WebOTPService::OnUserConsentComplete(UserConsentResult result) {
+  switch (result) {
+    case UserConsentResult::kApproved:
+      CompleteRequest(SmsStatus::kSuccess);
+      break;
+    case UserConsentResult::kNoDelegate:
+      CompleteRequest(SmsStatus::kCancelled);
+      break;
+    case UserConsentResult::kDenied:
+      OnFailure(FailureType::kPromptCancelled);
+      break;
+  }
+}
+
 }  // namespace content

content/browser/sms/webotp_service.h

@@ -71,6 +71,8 @@ class CONTENT_EXPORT WebOTPService
   // Rejects the pending request if it has not been resolved naturally yet.
   void OnTimeout();
 
+  void OnUserConsentComplete(UserConsentResult);
+
  protected:
   // content::WebContentsObserver:
   void NavigationEntryCommitted(

content/browser/sms/webotp_service_unittest.cc

@@ -566,7 +566,7 @@ class ServiceWithPrompt : public Service {
       FAIL() << "User prompt is not available";
       return;
     }
-    std::move(on_complete_callback_).Run(SmsStatus::kSuccess);
+    std::move(on_complete_callback_).Run(UserConsentResult::kApproved);
     on_complete_callback_.Reset();
   }
 
@@ -575,7 +575,8 @@ class ServiceWithPrompt : public Service {
       FAIL() << "User prompt is not available";
       return;
     }
-    std::move(on_complete_callback_).Run(SmsStatus::kCancelled);
+    std::move(on_complete_callback_).Run(UserConsentResult::kDenied);
+    ActivateTimer();
     on_complete_callback_.Reset();
   }
 
@@ -914,4 +915,30 @@ TEST_F(WebOTPServiceTest,
   ExpectNoOutcomeUKM();
 }
 
+TEST_F(WebOTPServiceTest, RecordUserDismissPrompt) {
+  GURL url = GURL(kTestUrl);
+  NavigateAndCommit(url);
+
+  ServiceWithPrompt service(web_contents());
+
+  base::RunLoop ukm_loop;
+  ukm_recorder()->SetOnAddEntryCallback(Entry::kEntryName,
+                                        ukm_loop.QuitClosure());
+
+  service.ExpectRequestUserConsent();
+  EXPECT_CALL(*service.provider(), Retrieve(_)).WillOnce(Invoke([&service]() {
+    service.NotifyReceive(GURL(kTestUrl), "hi", UserConsent::kNotObtained);
+    service.DismissPrompt();
+  }));
+
+  service.MakeRequest(BindLambdaForTesting(
+      [](SmsStatus status, const Optional<string>& otp) {}));
+
+  ukm_loop.Run();
+
+  ExpectOutcomeUKM(url, blink::WebOTPServiceOutcome::kUserCancelled);
+  ExpectTimingUKM("TimeUserCancelMs");
+  histogram_tester().ExpectTotalCount("Blink.Sms.Receive.TimeUserCancel", 1);
+}
+
 }  // namespace content