[PasswordManager] Fix Flaky BrowserTest related to dynamically created

chrome/browser/password_manager/password_manager_test_base.h

Index: chrome/browser/password_manager/password_manager_test_base.h
diff --git a/chrome/browser/password_manager/password_manager_test_base.h b/chrome/browser/password_manager/password_manager_test_base.h
index 29d6887a66e7ea81e99ff4a8cb1594394594d5e4..5ba3909a1eb913e85eb6dd3440fdc8e859ce232e 100644
--- a/chrome/browser/password_manager/password_manager_test_base.h
+++ b/chrome/browser/password_manager/password_manager_test_base.h
@@ -8,6 +8,8 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "components/password_manager/core/browser/password_store_consumer.h"
+#include "components/password_manager/core/browser/test_password_store.h"

[vabr (Chromium), 2015/08/05 08:37:35]

You do not need this.

[xunlu, 2015/08/05 18:37:13]

Done.

 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/test_utils.h"
 
@@ -44,6 +46,25 @@ class NavigationObserver : public content::WebContentsObserver {
   DISALLOW_COPY_AND_ASSIGN(NavigationObserver);
 };
 
+class PasswordStoreObserver : public password_manager::PasswordStoreConsumer {

[vabr (Chromium), 2015/08/05 08:37:35]

I suggest against naming the class *Observer. It confusingly hints at
PasswordStore::Observer, which it does not use. It also does not really observe
the store (it does not see anything else than the response to its own GetLogins
request).

My suggestion: DummyStoreConsumer
because it expresses more clearly what it inherits from, what it does, and that
it does not care about the actual response from the store.

[xunlu, 2015/08/05 18:37:13]

Done.

+ public:
+  explicit PasswordStoreObserver(password_manager::TestPasswordStore*);

[vabr (Chromium), 2015/08/05 08:37:35]

Please change TestPasswordStore references to PasswordStore in all of the class
code. You are not using anything not contained in the base PasswordStore
interface.

[xunlu, 2015/08/05 18:37:13]

Done.

+  ~PasswordStoreObserver() override;
+
+  // password_manager::PasswordStoreConsumer
+  void OnGetPasswordStoreResults(
+      ScopedVector<autofill::PasswordForm> results) override;
+
+  // Wait for test_password_store_ to respond to GetLogins();
+  void Wait();

[vabr (Chromium), 2015/08/05 08:37:35]

The name is too short and vague. Also, it seems strange to pass the passwords
store to the constructor and only use it in Wait(). I would suggest:
(1) Get rid of test_password_store_ member variable, do not pass a store in the
constructor.
(2) Instead, pass the store as an argument of the current Wait() method, and do
not store that argument anywhere.
(3) Rename Wait to RequestLoginsAndWait.

[xunlu, 2015/08/05 18:37:13]

Done.

+
+ private:
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+  password_manager::TestPasswordStore* test_password_store_;
+
+  DISALLOW_COPY_AND_ASSIGN(PasswordStoreObserver);
+};
+
 // Observes the save password prompt (bubble or infobar) for a specified
 // WebContents, keeps track of whether or not it is currently shown, and allows
 // accepting saving passwords through it.