Allow editing passwords in settings/passwords

chrome/browser/ui/passwords/password_manager_presenter_unittest.cc

Index: chrome/browser/ui/passwords/password_manager_presenter_unittest.cc
diff --git a/chrome/browser/ui/passwords/password_manager_presenter_unittest.cc b/chrome/browser/ui/passwords/password_manager_presenter_unittest.cc
index 1cee3092e502b9b5c821f1dce7b020c307dee802..feea0d0c90c77a2fa65ce035d6bd0b1c5b6eef53 100644
--- a/chrome/browser/ui/passwords/password_manager_presenter_unittest.cc
+++ b/chrome/browser/ui/passwords/password_manager_presenter_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/macros.h"

[engedy, 2014/09/15 14:38:59]

Do we need this?

[jaekyeom, 2014/09/16 07:58:10]

Yes, for arraysize().

+#include "base/memory/ref_counted.h"

[engedy, 2014/09/15 14:38:59]

I think we can drop this, as this must already be included from
mock_password_store_service.h, otherwise it would not compile -- and we only
need it for calling this.

[jaekyeom, 2014/09/16 07:58:10]

Done.

 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/password_manager/mock_password_store_service.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
@@ -13,7 +15,9 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::ASCIIToUTF16;
+using testing::AllOf;

[engedy, 2014/09/15 14:38:59]

nit: Conventially, we only introduce a using directive if it can save lines in
total.

[jaekyeom, 2014/09/16 07:58:10]

Done.

 using testing::Eq;
+using testing::Field;
 using testing::Property;
 
 class MockPasswordUIView : public PasswordUIView {
@@ -57,6 +61,9 @@ class PasswordManagerPresenterTest : public testing::Test {
     PasswordStoreFactory::GetInstance()->SetTestingFactory(
         &profile_, MockPasswordStoreService::Build);
     mock_controller_.reset(new MockPasswordUIView(&profile_));
+    mock_store_ = static_cast<password_manager::MockPasswordStore*>(
+        PasswordStoreFactory::GetForProfile(&profile_,
+                                            Profile::EXPLICIT_ACCESS).get());
   }
   void AddPasswordEntry(const GURL& origin,
                         const std::string& user_name,
@@ -64,10 +71,17 @@ class PasswordManagerPresenterTest : public testing::Test {
   void AddPasswordException(const GURL& origin);
   void UpdateLists();
   MockPasswordUIView* GetUIController() { return mock_controller_.get(); }
+  PasswordManagerPresenter* GetPasswordManagerPresenter() {
+    return mock_controller_->GetPasswordManagerPresenter();
+  }
+  password_manager::MockPasswordStore* GetPasswordStore() {
+    return mock_store_.get();
+  }
 
  private:
   TestingProfile profile_;
   scoped_ptr<MockPasswordUIView> mock_controller_;
+  scoped_refptr<password_manager::MockPasswordStore> mock_store_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordManagerPresenterTest);
 };
@@ -82,20 +96,18 @@ void PasswordManagerPresenterTest::AddPasswordEntry(
   form->username_value = base::ASCIIToUTF16(user_name);
   form->password_element = base::ASCIIToUTF16("Passwd");
   form->password_value = base::ASCIIToUTF16(password);
-  mock_controller_->GetPasswordManagerPresenter()->password_list_
-      .push_back(form);
+  GetPasswordManagerPresenter()->password_list_.push_back(form);
 }
 
 void PasswordManagerPresenterTest::AddPasswordException(const GURL& origin) {
   autofill::PasswordForm* form = new autofill::PasswordForm();
   form->origin = origin;
-  mock_controller_->GetPasswordManagerPresenter()->password_exception_list_
-      .push_back(form);
+  GetPasswordManagerPresenter()->password_exception_list_.push_back(form);
 }
 
 void PasswordManagerPresenterTest::UpdateLists() {
-  mock_controller_->GetPasswordManagerPresenter()->SetPasswordList();
-  mock_controller_->GetPasswordManagerPresenter()->SetPasswordExceptionList();
+  GetPasswordManagerPresenter()->SetPasswordList();
+  GetPasswordManagerPresenter()->SetPasswordExceptionList();
 }
 
 namespace {
@@ -145,4 +157,102 @@ TEST_F(PasswordManagerPresenterTest, UIControllerIsCalled) {
   UpdateLists();
 }
 
+// AddPassword and UpdatePassword are never called on Android.
+#if !defined(OS_ANDROID)
+TEST_F(PasswordManagerPresenterTest, CallAddPassword) {
+  GURL basic_origin("http://host.com");
+  base::string16 username = base::ASCIIToUTF16("username");
+  base::string16 password = base::ASCIIToUTF16("password");
+  EXPECT_CALL(
+      *GetPasswordStore(),
+      AddLogin(AllOf(
+          Field(&autofill::PasswordForm::signon_realm, Eq(basic_origin.spec())),
+          Field(&autofill::PasswordForm::origin, Eq(basic_origin)),
+          Field(&autofill::PasswordForm::username_value, Eq(username)),
+          Field(&autofill::PasswordForm::password_value, Eq(password)),
+          Field(&autofill::PasswordForm::ssl_valid, Eq(false)))));
+  GetPasswordManagerPresenter()->AddPassword(basic_origin, username, password);
+
+  GURL complex_origin("https://foo:bar@host.com/path?query=v#ref");

[engedy, 2014/09/15 14:38:59]

Please add a custom port as well (and update the expectations on |signon_realm|
and |origin|).

[jaekyeom, 2014/09/16 07:58:10]

Done.

+  EXPECT_CALL(
+      *GetPasswordStore(),
+      AddLogin(AllOf(
+          Field(&autofill::PasswordForm::signon_realm, Eq("https://host.com/")),
+          Field(&autofill::PasswordForm::origin,
+                Eq(GURL("https://host.com/path"))),
+          Field(&autofill::PasswordForm::username_value, Eq(username)),
+          Field(&autofill::PasswordForm::password_value, Eq(password)),
+          Field(&autofill::PasswordForm::ssl_valid, Eq(true)))));
+  GetPasswordManagerPresenter()->AddPassword(complex_origin,
+                                             username,
+                                             password);
+}
+
+TEST_F(PasswordManagerPresenterTest, CallUpdatePassword) {
+  GURL origin1("http://host.com");
+  std::string username1 = "username";

[engedy, 2014/09/15 14:38:59]

nit: Conventionally, we use const char kUsername1[] = "..."; in these cases.

[jaekyeom, 2014/09/16 07:58:10]

Done.

+  AddPasswordEntry(origin1, username1, "password");
+  GURL origin2("https://example.com");
+  std::string username2 = "testname";
+  AddPasswordEntry(origin2, username2, "abcd");
+
+  base::string16 new_password = base::ASCIIToUTF16("testpassword");
+  EXPECT_CALL(
+      *GetPasswordStore(),
+      UpdateLogin(AllOf(
+          Field(&autofill::PasswordForm::origin, Eq(origin1)),
+          Field(&autofill::PasswordForm::username_value,
+                Eq(base::ASCIIToUTF16(username1))),
+          Field(&autofill::PasswordForm::password_value,
+                Eq(new_password)))));
+  GetPasswordManagerPresenter()->UpdatePassword(0, new_password);
+
+  base::string16 new_password_again = base::ASCIIToUTF16("testpassword_again");
+  EXPECT_CALL(
+      *GetPasswordStore(),
+      UpdateLogin(AllOf(
+          Field(&autofill::PasswordForm::origin, Eq(origin1)),
+          Field(&autofill::PasswordForm::username_value,
+                Eq(base::ASCIIToUTF16(username1))),
+          Field(&autofill::PasswordForm::password_value,
+                Eq(new_password_again)))));
+  GetPasswordManagerPresenter()->UpdatePassword(0, new_password_again);
+
+  base::string16 another_password = base::ASCIIToUTF16("mypassword");
+  EXPECT_CALL(
+      *GetPasswordStore(),
+      UpdateLogin(AllOf(
+          Field(&autofill::PasswordForm::origin, Eq(origin2)),
+          Field(&autofill::PasswordForm::username_value,
+                Eq(base::ASCIIToUTF16(username2))),
+          Field(&autofill::PasswordForm::password_value,
+                Eq(another_password)))));
+  GetPasswordManagerPresenter()->UpdatePassword(1, another_password);
+}
+#endif  // !defined(OS_ANDROID)
+
+TEST(PasswordManagerPresenterTestSimple, CallCheckOriginValidityForAdding) {
+  static const char* const kValidOrigins[] = {
+    "http://host.com",
+    "http://host.com/path",
+    "https://host.com",
+    "https://foo:bar@host.com/path?query=v#ref"

[engedy, 2014/09/15 14:38:59]

Please also add an URL with a custom a port specified.

[jaekyeom, 2014/09/16 07:58:10]

Done.

+  };
+  for (size_t i = 0; i < arraysize(kValidOrigins); ++i) {
+    EXPECT_TRUE(PasswordManagerPresenter::CheckOriginValidityForAdding(

[engedy, 2014/09/15 14:38:59]

Could you please add:

  SCOPED_TRACE(kValidOrigins[i].spec());

[jaekyeom, 2014/09/16 07:58:10]

Done.

+        GURL(kValidOrigins[i])));
+  }
+
+  static const char* const kInvalidOrigins[] = {
+    "noscheme",
+    "invalidscheme:host.com",
+    "ftp://ftp.host.com",
+    "about:test"
+  };
+  for (size_t i = 0; i < arraysize(kInvalidOrigins); ++i) {
+    EXPECT_FALSE(PasswordManagerPresenter::CheckOriginValidityForAdding(

[engedy, 2014/09/15 14:38:59]

Same here.

[jaekyeom, 2014/09/16 07:58:10]

Done.

+        GURL(kInvalidOrigins[i])));
+  }
+}
+
 }  // namespace