Enable import/export of passwords into/from Password Manager

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 2997830ea64f9dfcbe2ecec860f7288019fd8d2d..7c393d8bd45fe2b0dc7b69b235fd7f85fef937d4 100644
--- a/chrome/browser/ui/passwords/password_manager_presenter.h
+++ b/chrome/browser/ui/passwords/password_manager_presenter.h
@@ -43,6 +43,9 @@ class PasswordManagerPresenter
   // Gets the password entry at |index|.
   const autofill::PasswordForm* GetPassword(size_t index);
 
+  // Gets all password entries.
+  ScopedVector<autofill::PasswordForm> GetAllPasswords();
+
   // Gets the password exception entry at |index|.
   const autofill::PasswordForm* GetPasswordException(size_t index);
 
@@ -50,6 +53,10 @@ class PasswordManagerPresenter
   // |index| the entry index to be removed.
   void RemoveSavedPassword(size_t index);
 
+  // Add password forms in |forms| to PasswordStore, return true if succeed.

[vabr (Chromium), 2015/07/06 08:57:16]

nit: "if succeed" -> "on success"

[xunlu, 2015/07/07 00:46:03]

Done.

+  virtual bool AddPasswordsToStore(

[vabr (Chromium), 2015/07/06 08:57:16]

There is something fishy about this method -- it does not use anything from the
presenter itself, only gets access to the password store through it. I don't
think this should be part of the presenter logic. My suggestion is to transfer
the for-loop with AddLogin() calls to the handler, and either use
PasswordStoreFactory::GetForProfile() to get the store pointer there, or if
that's not possible, just make the presenter's GetPasswordStore() public and use
it in the handler.

[xunlu, 2015/07/07 00:46:03]

Done.

+      const std::vector<autofill::PasswordForm>& forms);
+
   // Removes the saved password exception entry at |index|.
   // |index| the entry index to be removed.
   void RemovePasswordException(size_t index);
@@ -58,6 +65,10 @@ class PasswordManagerPresenter
   // |index| The index of the entry.
   void RequestShowPassword(size_t index);
 
+  // Request passwords in |password_list_| to be exported. Return true if

[vabr (Chromium), 2015/07/06 08:57:16]

nit: Could you please make it clearer who/what decides to grant the request or
not? Is it the user? Or some automated criterion? 

Also, is |password_list_| the same as "all password entries" referred with
GetAllPasswords above? If so, then we should be consistent and only use "all
password entries" in both cases. (Also note that referencing implementation
details like data members in public API description may cause issues for
readability and staying up to date.)

[xunlu, 2015/07/07 00:46:03]

Done.

+  // request granted.
+  virtual bool RequestToExportPassword();
+
  private:
   friend class PasswordManagerPresenterTest;