Fix password automation hooks to get pyauto test testSavePassword working.

Previously, all passwords tests were diabled in pyauto.  This CL
re-enables passwords.PasswordTest.testSavePassword (by explicitly
disabling all other passwords tests).

BUG=73078
TEST=None

Review URL: http://codereview.chromium.org/6994005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86154 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/automation/automation_provider_observers.cc

@@ -1660,7 +1660,7 @@ void AutomationProviderGetPasswordsObserver::OnPasswordStoreRequestDone(
 
   ListValue* passwords = new ListValue;
   for (std::vector<webkit_glue::PasswordForm*>::const_iterator it =
-          result.begin(); it != result.end(); ++it) {
+           result.begin(); it != result.end(); ++it) {
     DictionaryValue* password_val = new DictionaryValue;
     webkit_glue::PasswordForm* password_form = *it;
     password_val->SetString("username_value", password_form->username_value);
@@ -1673,8 +1673,7 @@ void AutomationProviderGetPasswordsObserver::OnPasswordStoreRequestDone(
                             password_form->username_element);
     password_val->SetString("password_element",
                             password_form->password_element);
-    password_val->SetString("submit_element",
-                                     password_form->submit_element);
+    password_val->SetString("submit_element", password_form->submit_element);
     password_val->SetString("action_target", password_form->action.spec());
     password_val->SetBoolean("blacklist", password_form->blacklisted_by_user);
     passwords->Append(password_val);
@@ -1686,6 +1685,66 @@ void AutomationProviderGetPasswordsObserver::OnPasswordStoreRequestDone(
   delete this;
 }
 
+PasswordStoreLoginsChangedObserver::PasswordStoreLoginsChangedObserver(
+    AutomationProvider* automation,
+    IPC::Message* reply_message,
+    const std::string& result_key)
+    : automation_(automation->AsWeakPtr()),
+      reply_message_(reply_message),
+      result_key_(result_key),
+      done_event_(false, false) {
+  AddRef();
+}
+
+PasswordStoreLoginsChangedObserver::~PasswordStoreLoginsChangedObserver() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+}
+
+void PasswordStoreLoginsChangedObserver::Init() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  BrowserThread::PostTask(
+      BrowserThread::DB,
+      FROM_HERE,
+      NewRunnableMethod(
+          this, &PasswordStoreLoginsChangedObserver::RegisterObserversTask));
+  done_event_.Wait();
+}
+
+void PasswordStoreLoginsChangedObserver::RegisterObserversTask() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+  registrar_.Add(this, NotificationType::LOGINS_CHANGED,
+                 NotificationService::AllSources());
+  done_event_.Signal();
+}
+
+void PasswordStoreLoginsChangedObserver::Observe(
+    NotificationType type,
+    const NotificationSource& source,
+    const NotificationDetails& details) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+  DCHECK(type.value == NotificationType::LOGINS_CHANGED);
+
+  registrar_.RemoveAll();  // Must be done from thread BrowserThread::DB.
+
+  // Notify thread BrowserThread::UI that we're done listening.
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      NewRunnableMethod(
+          this, &PasswordStoreLoginsChangedObserver::IndicateDone));
+}
+
+void PasswordStoreLoginsChangedObserver::IndicateDone() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (automation_) {
+    scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
+    return_value->SetBoolean(result_key_, true);
+    AutomationJSONReply(
+        automation_, reply_message_.release()).SendSuccess(return_value.get());
+  }
+  Release();
+}
+
 AutomationProviderBrowsingDataObserver::AutomationProviderBrowsingDataObserver(
     AutomationProvider* provider,
     IPC::Message* reply_message)

chrome/browser/automation/automation_provider_observers.h

@@ -1034,6 +1034,52 @@ class AutomationProviderGetPasswordsObserver : public PasswordStoreConsumer {
   scoped_ptr<IPC::Message> reply_message_;
 };
 
+// Observes when login entries stored in the password store are changed.  The
+// notifications are sent on BrowserThread::DB, the thread that interacts with
+// the web database.
+class PasswordStoreLoginsChangedObserver
+    : public base::RefCountedThreadSafe<
+          PasswordStoreLoginsChangedObserver,
+          BrowserThread::DeleteOnUIThread>,
+      public NotificationObserver {
+ public:
+  PasswordStoreLoginsChangedObserver(AutomationProvider* automation,
+                                     IPC::Message* reply_message,
+                                     const std::string& result_key);
+  virtual ~PasswordStoreLoginsChangedObserver();
+
+  // Schedules a task on the BrowserThread::DB thread to register the
+  // appropriate observers.
+  virtual void Init();
+
+  // NotificationObserver interface.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
+ private:
+  friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
+  friend class DeleteTask<PasswordStoreLoginsChangedObserver>;
+
+  // Registers the appropriate observers.  Called on thread BrowserThread::DB.
+  void RegisterObserversTask();
+
+  // Sends the |reply_message_| to |automation_| indicating we're done.  Called
+  // on thread BrowserThread::UI (the main browser UI thread).
+  void IndicateDone();
+
+  base::WeakPtr<AutomationProvider> automation_;
+  scoped_ptr<IPC::Message> reply_message_;
+  NotificationRegistrar registrar_;
+  std::string result_key_;
+
+  // Used to ensure that thread BrowserThread::UI waits for thread
+  // BrowserThread::DB to finish registering observers before proceeding.
+  base::WaitableEvent done_event_;
+
+  DISALLOW_COPY_AND_ASSIGN(PasswordStoreLoginsChangedObserver);
+};
+
 // Allows the automation provider to wait for clearing browser data to finish.
 class AutomationProviderBrowsingDataObserver
     : public BrowsingDataRemover::Observer {

chrome/browser/automation/testing_automation_provider.cc

@@ -3543,40 +3543,42 @@ void TestingAutomationProvider::AddSavedPassword(
     Browser* browser,
     DictionaryValue* args,
     IPC::Message* reply_message) {
-  AutomationJSONReply reply(this, reply_message);
   DictionaryValue* password_dict = NULL;
-
   if (!args->GetDictionary("password", &password_dict)) {
-    reply.SendError("Password must be a dictionary.");
+    AutomationJSONReply(this, reply_message).SendError(
+        "Must specify a password dictionary.");
     return;
   }
 
-  // The signon realm is effectively the primary key and must be included.
+  // The "signon realm" is effectively the primary key and must be included.
   // Check here before calling GetPasswordFormFromDict.
   if (!password_dict->HasKey("signon_realm")) {
-    reply.SendError("Password must include signon_realm.");
+    AutomationJSONReply(this, reply_message).SendError(
+        "Password must include a value for 'signon_realm.'");
     return;
   }
+
   webkit_glue::PasswordForm new_password =
       GetPasswordFormFromDict(*password_dict);
 
-  Profile* profile = browser->profile();
   // Use IMPLICIT_ACCESS since new passwords aren't added in incognito mode.
   PasswordStore* password_store =
-      profile->GetPasswordStore(Profile::IMPLICIT_ACCESS);
+      browser->profile()->GetPasswordStore(Profile::IMPLICIT_ACCESS);
 
-  // Set the return based on whether setting the password succeeded.
-  scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
-
-  // It will be null if it's accessed in an incognito window.
-  if (password_store != NULL) {
-    password_store->AddLogin(new_password);
-    return_value->SetBoolean("password_added", true);
-  } else {
+  // The password store does not exist for an incognito window.
+  if (password_store == NULL) {
+    scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
     return_value->SetBoolean("password_added", false);
+    AutomationJSONReply(this, reply_message).SendSuccess(return_value.get());
+    return;
   }
 
-  reply.SendSuccess(return_value.get());
+  // This observer will delete itself.
+  PasswordStoreLoginsChangedObserver *observer =
+      new PasswordStoreLoginsChangedObserver(this, reply_message,
+                                             "password_added");
+  observer->Init();
+  password_store->AddLogin(new_password);
 }
 
 // See RemoveSavedPassword() in chrome/test/functional/pyauto.py for sample
@@ -3603,10 +3605,9 @@ void TestingAutomationProvider::RemoveSavedPassword(
   webkit_glue::PasswordForm to_remove =
       GetPasswordFormFromDict(*password_dict);
 
-  Profile* profile = browser->profile();
   // Use EXPLICIT_ACCESS since passwords can be removed in incognito mode.
   PasswordStore* password_store =
-      profile->GetPasswordStore(Profile::EXPLICIT_ACCESS);
+      browser->profile()->GetPasswordStore(Profile::EXPLICIT_ACCESS);
 
   password_store->RemoveLogin(to_remove);
   reply.SendSuccess(NULL);
@@ -3619,14 +3620,13 @@ void TestingAutomationProvider::GetSavedPasswords(
     Browser* browser,
     DictionaryValue* args,
     IPC::Message* reply_message) {
-  Profile* profile = browser->profile();
   // Use EXPLICIT_ACCESS since saved passwords can be retrieved in
   // incognito mode.
   PasswordStore* password_store =
-      profile->GetPasswordStore(Profile::EXPLICIT_ACCESS);
+      browser->profile()->GetPasswordStore(Profile::EXPLICIT_ACCESS);
   password_store->GetAutofillableLogins(
       new AutomationProviderGetPasswordsObserver(this, reply_message));
-  // Observer deletes itself after returning.
+  // Observer deletes itself after sending the result.
 }
 
 // Refer to ClearBrowsingData() in chrome/test/pyautolib/pyauto.py for sample

chrome/browser/password_manager/password_store_consumer.h

@@ -12,7 +12,7 @@ namespace webkit_glue {
 struct PasswordForm;
 };
 
-// Reads from the PasswordStore are done asynchrously on a separate
+// Reads from the PasswordStore are done asynchronously on a separate
 // thread. PasswordStoreConsumer provides the virtual callback method, which is
 // guaranteed to be executed on this (the UI) thread. It also provides the
 // CancelableRequestConsumer member, which cancels any outstanding requests upon

chrome/test/functional/PYAUTO_TESTS

@@ -58,8 +58,7 @@
       'ntp',
       'omnibox',
       '-omnibox.OmniboxTest.testHistoryResult',  # crbug.com/71715
-      # crbug.com/73078
-      '-passwords',
+      'passwords',
       'pdf',
       'plugins',
       'plugins_check',
@@ -83,14 +82,14 @@
       # crbug.com/50481, crbug.com/54942
       '-downloads.DownloadsTest.testZip',
       '-downloads.DownloadsTest.testRemoveDownload',
-      '-prefs.PrefsTest.testDownloadDirPref',
       # crbug.com/66072
       '-notifications.NotificationsTest.testNotificationOrderAfterClosingOne',
-      # crbug.com/70694
+      # crbug.com/70694, crbug.com/73078
       '-passwords.PasswordTest.testInfoBarDisappearByReload',
       '-passwords.PasswordTest.testNeverSavePasswords',
       # crbug.com/67588
       '-passwords.PasswordTest.testSavedPasswordInTabsAndWindows',
+      '-prefs.PrefsTest.testDownloadDirPref',
     ],
 
     'mac': [
@@ -122,9 +121,8 @@
 
     'linux': [  # linux != chromeos
       '-browser.BrowserTest.testWindowResize',  # crbug.com/44963
-      # crbug.com/70694
-      '-passwords.PasswordTest.testInfoBarDisappearByReload',
-      '-passwords.PasswordTest.testNeverSavePasswords',
+      # System password manager obstructs password automation.
+      '-passwords',
       # Gnome / kwallet popups on linux break sync tests.  crbug.com/80329
       '-sync',
     ],
@@ -158,9 +156,6 @@
       '-infobars.InfobarTest.testPluginCrashInfobar',
       # Flaky: crosbug.com/14439
       '-instant.InstantSettingsTest.testEnableDisableInstant',
-      # crbug.com/70694
-      '-passwords.PasswordTest.testInfoBarDisappearByReload',
-      '-passwords.PasswordTest.testNeverSavePasswords',
 
       # Deal with i18n chars.  crosbug.com/12639
       '-autofill.AutofillTest.testFillProfileCrazyCharacters',

chrome/test/functional/passwords.py

 #!/usr/bin/python
-# Copyright (c) 2010 The Chromium Authors. All rights reserved.
+# Copyright (c) 2011 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -25,6 +25,10 @@ class PasswordTest(pyauto.PyUITest):
       pp = pprint.PrettyPrinter(indent=2)
       pp.pprint(self.GetSavedPasswords())
 
+  def setUp(self):
+    pyauto.PyUITest.setUp(self)
+    self.assertFalse(self.GetSavedPasswords())
+
   def _AssertWithinOneSecond(self, time1, time2):
     self.assertTrue(abs(time1 - time2) < 1.0,
                     'Times not within an acceptable range. '
@@ -54,7 +58,7 @@ class PasswordTest(pyauto.PyUITest):
         'https://www.example.com/', 'https://www.example.com/login',
         'username', 'password', 'https://www.example.com/login/')
     self.assertTrue(self.AddSavedPassword(password1))
-    self.assertEquals(self.GetSavedPasswords(), [password1])
+    self.assertEqual(self.GetSavedPasswords(), [password1])
 
   def testRemovePasswords(self):
     """Verify that saved passwords can be removed."""

chrome/test/pyautolib/pyauto.py

@@ -1797,7 +1797,7 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
     }
     return self._GetResultFromJSONRequest(cmd_dict)
 
-  def AddSavedPassword(self, password_dict, window_index=0):
+  def AddSavedPassword(self, password_dict, windex=0):
     """Adds the given username-password combination to the saved passwords.
 
     Args:
@@ -1812,7 +1812,7 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
         'submit_element': 'submit',                  # The HTML element
         'action_target': 'https://www.example.com/login/',
         'blacklist': False }
-      window_index: window index, defaults to 0
+      windex: window index; defaults to 0 (first window).
 
     *Blacklist notes* To blacklist a site, add a blacklist password with the
     following dictionary items: origin_url, signon_realm, username_element,
@@ -1824,14 +1824,14 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
       mode, adding the password should fail.
 
     Raises:
-      JSONInterfaceError on error.
+      pyauto_errors.JSONInterfaceError if the automation call returns an error.
     """
     cmd_dict = {  # Prepare command for the json interface
       'command': 'AddSavedPassword',
       'password': password_dict
     }
     return self._GetResultFromJSONRequest(
-        cmd_dict, windex=window_index)['password_added']
+        cmd_dict, windex=windex)['password_added']
 
   def RemoveSavedPassword(self, password_dict, window_index=0):
     """Removes the password matching the provided password dictionary.