[Passwords] Use Strong Aliases for CredentialLeakType Booleans

This change implements using strong booleans for the creation of
CredentialLeakTypes. These are implemented using util::StrongAlias,
which increases type-safety and prevents accidental assignment between
separate boolean parameters. Furthermore, this change performs some
other small clean-ups.

Bug: 986317
Change-Id: I5d08b634921d36abe5a8d1354398fb7a6761b825
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1768353
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691604}

chrome/browser/ui/passwords/credential_leak_dialog_controller_impl_unittest.cc

@@ -15,7 +15,10 @@
 
 namespace {
 
-using password_manager::CreateLeakTypeFromBools;
+using password_manager::CreateLeakType;
+using password_manager::IsReused;
+using password_manager::IsSaved;
+using password_manager::IsSyncing;
 using password_manager::metrics_util::LeakDialogDismissalReason;
 using testing::StrictMock;
 
@@ -55,8 +58,8 @@ class CredentialLeakDialogControllerTest : public testing::Test {
 };
 
 TEST_F(CredentialLeakDialogControllerTest, CredentialLeakDialogClose) {
-  SetUpController(CreateLeakTypeFromBools(
-      /*is_saved=*/false, /*is_reused=*/false, /*is_syncing=*/false));
+  SetUpController(
+      CreateLeakType(IsSaved(false), IsReused(false), IsSyncing(false)));
 
   EXPECT_CALL(leak_prompt(), ShowCredentialLeakPrompt());
   controller().ShowCredentialLeakPrompt(&leak_prompt());
@@ -76,8 +79,8 @@ TEST_F(CredentialLeakDialogControllerTest, CredentialLeakDialogClose) {
 }
 
 TEST_F(CredentialLeakDialogControllerTest, CredentialLeakDialogOk) {
-  SetUpController(CreateLeakTypeFromBools(
-      /*is_saved=*/true, /*is_reused=*/true, /*is_syncing=*/false));
+  SetUpController(
+      CreateLeakType(IsSaved(true), IsReused(true), IsSyncing(false)));
 
   EXPECT_CALL(leak_prompt(), ShowCredentialLeakPrompt());
   controller().ShowCredentialLeakPrompt(&leak_prompt());
@@ -97,8 +100,8 @@ TEST_F(CredentialLeakDialogControllerTest, CredentialLeakDialogOk) {
 }
 
 TEST_F(CredentialLeakDialogControllerTest, CredentialLeakDialogCancel) {
-  SetUpController(CreateLeakTypeFromBools(
-      /*is_saved=*/false, /*is_reused=*/true, /*is_syncing=*/true));
+  SetUpController(
+      CreateLeakType(IsSaved(false), IsReused(true), IsSyncing(true)));
 
   EXPECT_CALL(leak_prompt(), ShowCredentialLeakPrompt());
   controller().ShowCredentialLeakPrompt(&leak_prompt());
@@ -118,8 +121,8 @@ TEST_F(CredentialLeakDialogControllerTest, CredentialLeakDialogCancel) {
 }
 
 TEST_F(CredentialLeakDialogControllerTest, CredentialLeakDialogCheckPasswords) {
-  SetUpController(CreateLeakTypeFromBools(
-      /*is_saved=*/true, /*is_reused=*/true, /*is_syncing=*/true));
+  SetUpController(
+      CreateLeakType(IsSaved(true), IsReused(true), IsSyncing(true)));
 
   EXPECT_CALL(leak_prompt(), ShowCredentialLeakPrompt());
   controller().ShowCredentialLeakPrompt(&leak_prompt());

chrome/browser/ui/passwords/credential_leak_dialog_utils_unittest.cc

@@ -13,8 +13,12 @@
 #include "url/gurl.h"
 #include "url/origin.h"
 
+using password_manager::CreateLeakType;
 using password_manager::CredentialLeakFlags;
 using password_manager::CredentialLeakType;
+using password_manager::IsReused;
+using password_manager::IsSaved;
+using password_manager::IsSyncing;
 
 namespace leak_dialog_utils {
 
@@ -33,35 +37,20 @@ const struct {
   bool should_show_cancel_button;
   bool should_check_passwords;
 } kLeakTypesTestCases[] = {
-    {password_manager::CreateLeakTypeFromBools(
-         /*is_saved=*/false,
-         /*is_reused=*/false,
-         /*is_syncing=*/false),
-     IDS_OK, IDS_CLOSE, IDS_CREDENTIAL_LEAK_CHANGE_PASSWORD_MESSAGE,
+    {CreateLeakType(IsSaved(false), IsReused(false), IsSyncing(false)), IDS_OK,
+     IDS_CLOSE, IDS_CREDENTIAL_LEAK_CHANGE_PASSWORD_MESSAGE,
      IDS_CREDENTIAL_LEAK_TITLE, false, false},
-    {password_manager::CreateLeakTypeFromBools(
-         /*is_saved=*/false,
-         /*is_reused=*/false,
-         /*is_syncing=*/true),
-     IDS_OK, IDS_CLOSE, IDS_CREDENTIAL_LEAK_CHANGE_PASSWORD_MESSAGE,
+    {CreateLeakType(IsSaved(false), IsReused(false), IsSyncing(true)), IDS_OK,
+     IDS_CLOSE, IDS_CREDENTIAL_LEAK_CHANGE_PASSWORD_MESSAGE,
      IDS_CREDENTIAL_LEAK_TITLE, false, false},
-    {password_manager::CreateLeakTypeFromBools(
-         /*is_saved=*/false,
-         /*is_reused=*/true,
-         /*is_syncing=*/true),
+    {CreateLeakType(IsSaved(false), IsReused(true), IsSyncing(true)),
      IDS_LEAK_CHECK_CREDENTIALS, IDS_CLOSE,
      IDS_CREDENTIAL_LEAK_CHANGE_AND_CHECK_PASSWORDS_MESSAGE,
      IDS_CREDENTIAL_LEAK_TITLE, true, true},
-    {password_manager::CreateLeakTypeFromBools(
-         /*is_saved=*/true,
-         /*is_reused=*/false,
-         /*is_syncing=*/true),
-     IDS_OK, IDS_CLOSE, IDS_CREDENTIAL_LEAK_CHANGE_PASSWORD_MESSAGE,
+    {CreateLeakType(IsSaved(true), IsReused(false), IsSyncing(true)), IDS_OK,
+     IDS_CLOSE, IDS_CREDENTIAL_LEAK_CHANGE_PASSWORD_MESSAGE,
      IDS_CREDENTIAL_LEAK_TITLE, false, false},
-    {password_manager::CreateLeakTypeFromBools(
-         /*is_saved=*/true,
-         /*is_reused=*/true,
-         /*is_syncing=*/true),
+    {CreateLeakType(IsSaved(true), IsReused(true), IsSyncing(true)),
      IDS_LEAK_CHECK_CREDENTIALS, IDS_CLOSE,
      IDS_CREDENTIAL_LEAK_CHECK_PASSWORDS_MESSAGE, IDS_CREDENTIAL_LEAK_TITLE,
      true, true}};

components/password_manager/core/browser/leak_detection_delegate.cc

@@ -62,15 +62,14 @@ void LeakDetectionDelegate::OnLeakDetectionDone(bool is_leaked,
       // If the credentials are not synced, the |CredentialLeakType| needed to
       // show the correct notification is already determined.
       OnShowLeakDetectionNotifiction(
-          CreateLeakTypeFromBools(/*is_saved=*/false, /*is_reused=*/false,
-                                  /*is_synced=*/false),
+          CreateLeakType(IsSaved(false), IsReused(false), IsSyncing(false)),
           std::move(url), std::move(username));
     } else {
       // Otherwise query the helper to asynchronously determine the
       // |CredentialLeakType|.
-      helper_ = std::make_unique<LeakDetectionDelegateHelper>(std::move(
+      helper_ = std::make_unique<LeakDetectionDelegateHelper>(
           base::BindOnce(&LeakDetectionDelegate::OnShowLeakDetectionNotifiction,
-                         base::Unretained(this))));
+                         base::Unretained(this)));
       helper_->GetCredentialLeakType(client_->GetPasswordStore(),
                                      std::move(url), std::move(username),
                                      std::move(password));

components/password_manager/core/browser/leak_detection_delegate_helper.cc

@@ -27,16 +27,14 @@ void LeakDetectionDelegateHelper::GetCredentialLeakType(
 
 void LeakDetectionDelegateHelper::OnGetPasswordStoreResults(
     std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
-  bool is_saved = false;
-  for (const auto& form : results) {
-    if (form->origin == url_ && form->username_value == username_) {
-      is_saved = true;
-      break;
-    }
-  }
-  bool is_reused = results.size() > (is_saved ? 1 : 0);
+  IsSaved is_saved(
+      std::any_of(results.begin(), results.end(), [this](const auto& form) {
+        return form->origin == url_ && form->username_value == username_;
+      }));
+
+  IsReused is_reused(results.size() > (is_saved ? 1 : 0));
   CredentialLeakType leak_type =
-      CreateLeakTypeFromBools(is_saved, is_reused, /*is_synced=*/true);
+      CreateLeakType(is_saved, is_reused, IsSyncing(true));
   std::move(callback_).Run(std::move(leak_type), std::move(url_),
                            std::move(username_));
 }

components/password_manager/core/browser/leak_detection_delegate_helper.h

@@ -23,7 +23,8 @@ class PasswordStore;
 class LeakDetectionDelegateHelper : public PasswordStoreConsumer {
  public:
   // Type alias for |callback_|.
-  using LeakTypeReply = base::OnceCallback<void(int, GURL, base::string16)>;
+  using LeakTypeReply =
+      base::OnceCallback<void(CredentialLeakType, GURL, base::string16)>;
 
   explicit LeakDetectionDelegateHelper(LeakTypeReply callback);
   ~LeakDetectionDelegateHelper() override;

components/password_manager/core/browser/leak_detection_delegate_helper_unittest.cc

@@ -88,11 +88,10 @@ class LeakDetectionDelegateHelperTest : public testing::Test {
   }
 
   // Set the expectation for the |CredentialLeakType| in the callback_.
-  void SetOnShowLeakDetectionNotificationExpectation(bool is_saved,
-                                                     bool is_reused) {
-    EXPECT_CALL(
-        callback_,
-        Run(CreateLeakTypeFromBools(is_saved, is_reused, /*is_synced=*/true),
+  void SetOnShowLeakDetectionNotificationExpectation(IsSaved is_saved,
+                                                     IsReused is_reused) {
+    EXPECT_CALL(callback_,
+                Run(CreateLeakType(is_saved, is_reused, IsSyncing(true)),
                     GURL(kLeakedOrigin), ASCIIToUTF16(kLeakedUsername)))
         .Times(1);
   }
@@ -107,8 +106,8 @@ TEST_F(LeakDetectionDelegateHelperTest, NeitherSaveNotReused) {
   std::vector<PasswordForm> password_forms;
 
   SetGetLoginByPasswordConsumerInvocation(std::move(password_forms));
-  SetOnShowLeakDetectionNotificationExpectation(/*is_saved=*/false,
-                                                /*is_reused=*/false);
+  SetOnShowLeakDetectionNotificationExpectation(IsSaved(false),
+                                                IsReused(false));
   InitiateGetCredentialLeakType();
 }
 
@@ -118,8 +117,7 @@ TEST_F(LeakDetectionDelegateHelperTest, SavedLeakedCredentials) {
       CreateForm(kLeakedOrigin, kLeakedUsername)};
 
   SetGetLoginByPasswordConsumerInvocation(std::move(password_forms));
-  SetOnShowLeakDetectionNotificationExpectation(/*is_saved=*/true,
-                                                /*is_reused=*/false);
+  SetOnShowLeakDetectionNotificationExpectation(IsSaved(true), IsReused(false));
   InitiateGetCredentialLeakType();
 }
 
@@ -131,8 +129,7 @@ TEST_F(LeakDetectionDelegateHelperTest,
       CreateForm(kOtherOrigin, kLeakedUsername)};
 
   SetGetLoginByPasswordConsumerInvocation(std::move(password_forms));
-  SetOnShowLeakDetectionNotificationExpectation(/*is_saved=*/true,
-                                                /*is_reused=*/true);
+  SetOnShowLeakDetectionNotificationExpectation(IsSaved(true), IsReused(true));
   InitiateGetCredentialLeakType();
 }
 
@@ -145,8 +142,7 @@ TEST_F(LeakDetectionDelegateHelperTest,
       CreateForm(kLeakedOrigin, kOtherUsername)};
 
   SetGetLoginByPasswordConsumerInvocation(std::move(password_forms));
-  SetOnShowLeakDetectionNotificationExpectation(/*is_saved=*/true,
-                                                /*is_reused=*/true);
+  SetOnShowLeakDetectionNotificationExpectation(IsSaved(true), IsReused(true));
   InitiateGetCredentialLeakType();
 }
 
@@ -156,8 +152,7 @@ TEST_F(LeakDetectionDelegateHelperTest, ReusedPasswordWithOtherUsername) {
       CreateForm(kLeakedOrigin, kOtherUsername)};
 
   SetGetLoginByPasswordConsumerInvocation(std::move(password_forms));
-  SetOnShowLeakDetectionNotificationExpectation(/*is_saved=*/false,
-                                                /*is_reused=*/true);
+  SetOnShowLeakDetectionNotificationExpectation(IsSaved(false), IsReused(true));
   InitiateGetCredentialLeakType();
 }
 
@@ -167,8 +162,7 @@ TEST_F(LeakDetectionDelegateHelperTest, ReusedPasswordOnOtherOrigin) {
       CreateForm(kOtherOrigin, kLeakedUsername)};
 
   SetGetLoginByPasswordConsumerInvocation(std::move(password_forms));
-  SetOnShowLeakDetectionNotificationExpectation(/*is_saved=*/false,
-                                                /*is_reused=*/true);
+  SetOnShowLeakDetectionNotificationExpectation(IsSaved(false), IsReused(true));
   InitiateGetCredentialLeakType();
 }
 
@@ -179,8 +173,7 @@ TEST_F(LeakDetectionDelegateHelperTest, ReusedPassword) {
       CreateForm(kOtherOrigin, kOtherUsername)};
 
   SetGetLoginByPasswordConsumerInvocation(std::move(password_forms));
-  SetOnShowLeakDetectionNotificationExpectation(/*is_saved=*/false,
-                                                /*is_reused=*/true);
+  SetOnShowLeakDetectionNotificationExpectation(IsSaved(false), IsReused(true));
   InitiateGetCredentialLeakType();
 }
 

components/password_manager/core/browser/leak_detection_delegate_unittest.cc

@@ -148,10 +148,10 @@ TEST_F(LeakDetectionDelegateTest, LeakDetectionDone) {
   histogram_tester.ExpectTotalCount(
       "PasswordManager.LeakDetection.NotifyIsLeakedTime", 0);
 
-  EXPECT_CALL(client(), NotifyUserCredentialsWereLeaked(
-                            password_manager::CreateLeakTypeFromBools(
-                                /*is_saved=*/false, /*is_reused=*/false,
-                                /*is_syncing=*/false),
+  EXPECT_CALL(client(),
+              NotifyUserCredentialsWereLeaked(
+                  password_manager::CreateLeakType(
+                      IsSaved(false), IsReused(false), IsSyncing(false)),
                   form.origin));
   delegate_interface->OnLeakDetectionDone(
       true, form.origin, form.username_value, form.password_value);

components/password_manager/core/browser/leak_detection_dialog_utils.cc

@@ -6,9 +6,9 @@
 
 namespace password_manager {
 
-CredentialLeakType CreateLeakTypeFromBools(bool is_saved,
-                                           bool is_reused,
-                                           bool is_syncing) {
+CredentialLeakType CreateLeakType(IsSaved is_saved,
+                                  IsReused is_reused,
+                                  IsSyncing is_syncing) {
   CredentialLeakType leak_type = 0;
   if (is_saved)
     leak_type |= kPasswordSaved;

components/password_manager/core/browser/leak_detection_dialog_utils.h

@@ -5,6 +5,10 @@
 #ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LEAK_DETECTION_DIALOG_UTILS_H_
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LEAK_DETECTION_DIALOG_UTILS_H_
 
+#include <type_traits>
+
+#include "base/util/type_safety/strong_alias.h"
+
 namespace password_manager {
 
 // Defines possible scenarios for leaked credentials.
@@ -18,12 +22,16 @@ enum CredentialLeakFlags {
 };
 
 // Contains combination of CredentialLeakFlags values.
-using CredentialLeakType = int;
+using CredentialLeakType = std::underlying_type_t<CredentialLeakFlags>;
+
+using IsSaved = util::StrongAlias<class IsSavedTag, bool>;
+using IsReused = util::StrongAlias<class IsReusedTag, bool>;
+using IsSyncing = util::StrongAlias<class IsSyncingTag, bool>;
 
-// Creates CredentialLeakType from booleans.
-CredentialLeakType CreateLeakTypeFromBools(bool is_saved,
-                                           bool is_reused,
-                                           bool is_syncing);
+// Creates CredentialLeakType from strong booleans.
+CredentialLeakType CreateLeakType(IsSaved is_saved,
+                                  IsReused is_reused,
+                                  IsSyncing is_syncing);
 
 // Checks whether password is saved in chrome.
 bool IsPasswordSaved(CredentialLeakType leak_type);