[Pwd Mgr] Remove PasswordEntryType Enum

This change cleans up the password list sorting logic by removing the
PasswordEntryType enum. This two state enum added no value, as the same
information was already stored on the PasswordForm itself in the form of
`blacklisted_by_user`. This change cleans up the interface of
CreateSortKey() and SortEntriesAndHideDuplicates().

Bug: 778146
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I883a5945168b569cdf1d6312f5e5d2eab4fb2ba5
Reviewed-on: https://chromium-review.googlesource.com/1143384Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577145}

chrome/browser/ui/passwords/password_manager_presenter.cc

@@ -50,9 +50,8 @@ namespace {
 // from |duplicates|.
 void RemoveDuplicates(const autofill::PasswordForm& form,
                       password_manager::DuplicatesMap* duplicates,
-                      PasswordStore* store,
-                      password_manager::PasswordEntryType entry_type) {
-  std::string key = password_manager::CreateSortKey(form, entry_type);
+                      PasswordStore* store) {
+  std::string key = password_manager::CreateSortKey(form);
   std::pair<password_manager::DuplicatesMap::iterator,
             password_manager::DuplicatesMap::iterator>
       dups = duplicates->equal_range(key);
@@ -193,8 +192,7 @@ void PasswordManagerPresenter::RemoveSavedPassword(size_t index) {
     return;
 
   const autofill::PasswordForm& password_entry = *password_list_[index];
-  RemoveDuplicates(password_entry, &password_duplicates_, store,
-                   password_manager::PasswordEntryType::SAVED);
+  RemoveDuplicates(password_entry, &password_duplicates_, store);
   RemoveLogin(password_entry);
   base::RecordAction(
       base::UserMetricsAction("PasswordManager_RemoveSavedPassword"));
@@ -215,7 +213,7 @@ void PasswordManagerPresenter::RemovePasswordException(size_t index) {
   const autofill::PasswordForm& password_exception_entry =
       *password_exception_list_[index];
   RemoveDuplicates(password_exception_entry, &password_exception_duplicates_,
-                   store, password_manager::PasswordEntryType::BLACKLISTED);
+                   store);
   RemoveLogin(password_exception_entry);
   base::RecordAction(
       base::UserMetricsAction("PasswordManager_RemovePasswordException"));
@@ -346,9 +344,8 @@ void PasswordManagerPresenter::PasswordListPopulater::Populate() {
 void PasswordManagerPresenter::PasswordListPopulater::OnGetPasswordStoreResults(
     std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
   page_->password_list_ = std::move(results);
-  password_manager::SortEntriesAndHideDuplicates(
-      &page_->password_list_, &page_->password_duplicates_,
-      password_manager::PasswordEntryType::SAVED);
+  password_manager::SortEntriesAndHideDuplicates(&page_->password_list_,
+                                                 &page_->password_duplicates_);
   page_->SetPasswordList();
 }
 
@@ -372,7 +369,6 @@ void PasswordManagerPresenter::PasswordExceptionListPopulater::
         std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
   page_->password_exception_list_ = std::move(results);
   password_manager::SortEntriesAndHideDuplicates(
-      &page_->password_exception_list_, &page_->password_exception_duplicates_,
-      password_manager::PasswordEntryType::BLACKLISTED);
+      &page_->password_exception_list_, &page_->password_exception_duplicates_);
   page_->SetPasswordExceptionList();
 }

components/password_manager/core/browser/password_list_sorter.cc

@@ -27,8 +27,7 @@ constexpr char kSortKeyNoFederationSymbol = '-';
 
 }  // namespace
 
-std::string CreateSortKey(const autofill::PasswordForm& form,
-                          PasswordEntryType entry_type) {
+std::string CreateSortKey(const autofill::PasswordForm& form) {
   std::string shown_origin;
   GURL link_url;
   std::tie(shown_origin, link_url) = GetShownOriginAndLinkUrl(form);
@@ -61,7 +60,7 @@ std::string CreateSortKey(const autofill::PasswordForm& form,
   key += is_android_uri ? facet_uri.canonical_spec()
                         : SplitByDotAndReverse(shown_origin);
 
-  if (entry_type == PasswordEntryType::SAVED) {
+  if (!form.blacklisted_by_user) {
     key += kSortKeyPartsSeparator + base::UTF16ToUTF8(form.username_value) +
            kSortKeyPartsSeparator + base::UTF16ToUTF8(form.password_value);
 
@@ -78,13 +77,12 @@ std::string CreateSortKey(const autofill::PasswordForm& form,
 
 void SortEntriesAndHideDuplicates(
     std::vector<std::unique_ptr<autofill::PasswordForm>>* list,
-    DuplicatesMap* duplicates,
-    PasswordEntryType entry_type) {
+    DuplicatesMap* duplicates) {
   std::vector<std::pair<std::string, std::unique_ptr<autofill::PasswordForm>>>
       keys_to_forms;
   keys_to_forms.reserve(list->size());
   for (auto& form : *list) {
-    std::string key = CreateSortKey(*form, entry_type);
+    std::string key = CreateSortKey(*form);
     keys_to_forms.emplace_back(std::move(key), std::move(form));
   }
 

components/password_manager/core/browser/password_list_sorter.h

@@ -16,8 +16,6 @@ struct PasswordForm;
 
 namespace password_manager {
 
-enum class PasswordEntryType { SAVED, BLACKLISTED };
-
 // Multimap from sort key to password forms.
 using DuplicatesMap =
     std::multimap<std::string, std::unique_ptr<autofill::PasswordForm>>;
@@ -25,21 +23,20 @@ using DuplicatesMap =
 // Creates key for sorting password or password exception entries. The key is
 // eTLD+1 followed by the reversed list of domains (e.g.
 // secure.accounts.example.com => example.com.com.example.accounts.secure) and
-// the scheme. If |entry_type == SAVED|, username, password and federation are
-// appended to the key. For Android credentials the canocial spec is included.
-std::string CreateSortKey(const autofill::PasswordForm& form,
-                          PasswordEntryType entry_type);
+// the scheme. If |form| is not blacklisted, username, password and federation
+// are appended to the key. If not, no further information is added. For Android
+// credentials the canocial spec is included.
+std::string CreateSortKey(const autofill::PasswordForm& form);
 
 // Sort entries of |list| based on sort key. The key is the concatenation of
 // origin, entry type (non-Android credential, Android w/ affiliated web realm
-// or Android w/o affiliated web realm). If |entry_type == SAVED|,
-// username, password and federation are also included in sort key. If there
-// are several forms with the same key, all such forms but the first one are
-// stored in |duplicates| instead of |list|.
+// or Android w/o affiliated web realm). If a form in |list| is not blacklisted,
+// username, password and federation are also included in sort key. If there are
+// several forms with the same key, all such forms but the first one are stored
+// in |duplicates| instead of |list|.
 void SortEntriesAndHideDuplicates(
     std::vector<std::unique_ptr<autofill::PasswordForm>>* list,
-    DuplicatesMap* duplicates,
-    PasswordEntryType entry_type);
+    DuplicatesMap* duplicates);
 
 }  // namespace password_manager
 

ios/chrome/browser/ui/settings/save_passwords_collection_view_controller.mm

@@ -525,12 +525,10 @@ initWithActivityItems:(NSArray*)activityItems
       savedForms_.push_back(std::move(form));
   }
 
+  password_manager::SortEntriesAndHideDuplicates(&savedForms_,
+                                                 &savedPasswordDuplicates_);
   password_manager::SortEntriesAndHideDuplicates(
-      &savedForms_, &savedPasswordDuplicates_,
-      password_manager::PasswordEntryType::SAVED);
-  password_manager::SortEntriesAndHideDuplicates(
-      &blacklistedForms_, &blacklistedPasswordDuplicates_,
-      password_manager::PasswordEntryType::BLACKLISTED);
+      &blacklistedForms_, &blacklistedPasswordDuplicates_);
 
   [self updateEditButton];
   [self reloadData];
@@ -709,15 +707,12 @@ initWithActivityItems:(NSArray*)activityItems
     auto& forms = blacklisted ? blacklistedForms_ : savedForms_;
     auto& duplicates =
         blacklisted ? blacklistedPasswordDuplicates_ : savedPasswordDuplicates_;
-    password_manager::PasswordEntryType entryType =
-        blacklisted ? password_manager::PasswordEntryType::BLACKLISTED
-                    : password_manager::PasswordEntryType::SAVED;
 
     DCHECK_LT(formIndex, forms.size());
     auto formIterator = forms.begin() + formIndex;
 
     std::unique_ptr<autofill::PasswordForm> form = std::move(*formIterator);
-    std::string key = password_manager::CreateSortKey(*form, entryType);
+    std::string key = password_manager::CreateSortKey(*form);
     auto duplicatesRange = duplicates.equal_range(key);
     for (auto iterator = duplicatesRange.first;
          iterator != duplicatesRange.second; ++iterator) {
@@ -800,11 +795,7 @@ initWithActivityItems:(NSArray*)activityItems
   password_manager::DuplicatesMap& duplicates =
       form.blacklisted_by_user ? blacklistedPasswordDuplicates_
                                : savedPasswordDuplicates_;
-  password_manager::PasswordEntryType entryType =
-      form.blacklisted_by_user
-          ? password_manager::PasswordEntryType::BLACKLISTED
-          : password_manager::PasswordEntryType::SAVED;
-  std::string key = password_manager::CreateSortKey(form, entryType);
+  std::string key = password_manager::CreateSortKey(form);
   auto duplicatesRange = duplicates.equal_range(key);
   for (auto iterator = duplicatesRange.first;
        iterator != duplicatesRange.second; ++iterator) {