[Password Manager] Implements entries sorting and duplicates omitting on chrome://settings/passwords

chrome/browser/ui/passwords/password_manager_presenter.h

Index: chrome/browser/ui/passwords/password_manager_presenter.h
diff --git a/chrome/browser/ui/passwords/password_manager_presenter.h b/chrome/browser/ui/passwords/password_manager_presenter.h
index 0096cdf25eb16d730d1b4ca4c0c4fa0272d23dc7..cb345884a558662846c8aa848d8e301d55a41f48 100644
--- a/chrome/browser/ui/passwords/password_manager_presenter.h
+++ b/chrome/browser/ui/passwords/password_manager_presenter.h
@@ -19,6 +19,9 @@
 
 namespace autofill {
 struct PasswordForm;
+
+using DuplicatesMap =

[vabr (Chromium), 2016/03/08 13:09:28]

Please pull DuplicatesMap out of the autofill namespace, there is no reason to
put it in there.

[kolos1, 2016/03/08 15:14:24]

Done.

+    std::multimap<std::string, scoped_ptr<autofill::PasswordForm>>;

[vabr (Chromium), 2016/03/08 13:09:28]

#include <ma> for multimap

[kolos1, 2016/03/08 15:14:24]

Done.

 }
 
 class PasswordUIView;
@@ -72,6 +75,19 @@ class PasswordManagerPresenter
   void SetPasswordList();
   void SetPasswordExceptionList();
 
+  // Sort entries of |list| based on sort key. The key is the concatenation of
+  // origin, username, hashed password and entry type (non-Android credential,
+  // Android w/ affiliated web realm or Android w/o affiliated web realm). If
+  // there is several forms with the same key, the duplicates are moved from

[vabr (Chromium), 2016/03/08 13:09:28]

nit: To clarify that "duplicates" exclude the first occurrence, consider
rephrasing:
"...the same key, all such forms but the first one are stored in |duplicates|
instead of |list|.

I would omit "(the multimap from sort key to forms)" and instead note  next to
the definition of DuplicatesMap that the std::string is a sort key.

[kolos1, 2016/03/08 15:14:24]

Done.

+  // |list| to |duplicates| (the multimap from sort key to forms).
+  // |username_and_password_in_key| defines whether username and  hashed

[vabr (Chromium), 2016/03/08 13:09:28]

The last sentence is not completely clear -- from the above comment it looks
like the username and the hashed comment are always included.

[kolos1, 2016/03/08 15:14:24]

Done.

+  // password are included in sort key.
+  void SortEntriesAndHideDuplicates(
+      const std::string& languages,
+      std::vector<scoped_ptr<autofill::PasswordForm>>* list,
+      autofill::DuplicatesMap* duplicates_,

[vabr (Chromium), 2016/03/08 13:09:28]

nit: Remove the trailing underscore in "duplicates_", this is not a member
variable.

[kolos1, 2016/03/08 15:14:24]

Done.

+      bool username_and_password_in_key);
+
   // A short class to mediate requests to the password store.
   class ListPopulater : public password_manager::PasswordStoreConsumer {
    public:
@@ -117,6 +133,8 @@ class PasswordManagerPresenter
 
   std::vector<scoped_ptr<autofill::PasswordForm>> password_list_;
   std::vector<scoped_ptr<autofill::PasswordForm>> password_exception_list_;
+  autofill::DuplicatesMap password_duplicates_;
+  autofill::DuplicatesMap password_exception_duplicates_;
 
   // Whether to show stored passwords or not.
   BooleanPrefMember show_passwords_;