Load Passwords and Autofill in the corresponding sub page.

chrome/test/data/webui/settings/settings_passwords_section_browsertest.js

Index: chrome/test/data/webui/settings/settings_passwords_section_browsertest.js
diff --git a/chrome/test/data/webui/settings/settings_passwords_section_browsertest.js b/chrome/test/data/webui/settings/settings_passwords_section_browsertest.js
index 215f67e00fe0fd89f4b88c26266fa173d748be90..c16c2ac0fceff13472091996618db9d2143da085 100644
--- a/chrome/test/data/webui/settings/settings_passwords_section_browsertest.js
+++ b/chrome/test/data/webui/settings/settings_passwords_section_browsertest.js
@@ -35,10 +35,18 @@ SettingsPasswordSectionBrowserTest.prototype = {
   setUp: function() {
     PolymerTest.prototype.setUp.call(this);
 
+    // Override the PasswordManagerImpl for testing.
+    this.passwordManager = new TestPasswordManager();

[dpapad, 2017/01/28 00:10:30]

Some drive by questions: Can we use the Mocha suite()'s setup and teardown
instead of this one? This would simplify things by eliminating the need to do
var self = this; as follows

function helperFunction1(passwordManager) {
  //setup password manager here.
}

function validatePasswordList(...) {
  //  helpers as top-level functions, not methods.
}

suite('mytestsuite', function() {
  var passwordManager = null;

  setup(function() {
    passwordManager = .....;
  });

  test('mytest', function() {
    // refer to passwordManager here, or helper functions.
  });
});

This is also more robust since a new PasswordManager is created between Mocha
test cases (no need to cleanup after previous PasswordManager).

[hcarmona, 2017/02/14 00:59:38]

Awesome!

+    PasswordManagerImpl.instance_ = this.passwordManager;
+
     // Test is run on an individual element that won't have a page language.
     this.accessibilityAuditConfig.auditRulesToIgnore.push('humanLangMissing');
   },
 
+  tearDown: function() {
+    PasswordManagerImpl.instance_ = null;

[hcarmona, 2017/01/26 21:34:47]

Moved this here to clean up after the test.

[Dan Beam, 2017/01/27 01:11:32]

we're just re-creating on setup, why does this matter?

[hcarmona, 2017/01/27 18:28:16]

PasswordManagerImpl.instance_ is global. If 2 tests rely on this and it's not
reset, then we can get into a state where one test will depend on the other.
This leads to weird failures that depend on test order.

[Dan Beam, 2017/01/27 23:55:22]

i mean, you can leave this in, but nobody else does it ;)

overriding a global instance:
https://cs.chromium.org/search/?q=%22instance_+%3D+%22+chrome/test/data/webui...

tearing down a global instance:
https://cs.chromium.org/search/?q=%22instance_+%3D+null%22+chrome/test/data/w...

[hcarmona, 2017/02/14 00:59:38]

Going w/ dpapad@'s suggestion for Mocha suite setup and removing the teardown
since it doesn't make sense to keep it. Tests that need it will override it.

+  },
+
   /**
    * Helper method that validates a that elements in the password list match
    * the expected data.
@@ -106,10 +114,12 @@ SettingsPasswordSectionBrowserTest.prototype = {
    * @private
    */
   createPasswordsSection_: function(passwordList, exceptionList) {
+    // Override the PasswordManager data for testing.
+    this.passwordManager.data.passwords = passwordList;
+    this.passwordManager.data.exceptions = exceptionList;
+
     // Create a passwords-section to use for testing.
     var passwordsSection = document.createElement('passwords-section');
-    passwordsSection.savedPasswords = passwordList;
-    passwordsSection.passwordExceptions = exceptionList;
     document.body.appendChild(passwordsSection);
     this.flushPasswordSection_(passwordsSection);
     return passwordsSection;
@@ -247,17 +257,16 @@ TEST_F('SettingsPasswordSectionBrowserTest', 'uiTests', function() {
       assert(firstNode);
       var firstPassword = passwordList[0];
 
-      // Listen for the remove event. If this event isn't received, the test
-      // will time out and fail.
-      passwordsSection.addEventListener('remove-saved-password',
-                                        function(event) {
+      self.passwordManager.onRemoveSavedPassword = function(detail) {

[dpapad, 2017/01/28 00:10:30]

The PasswordManager class seems to be just like any other "browser proxy".  Can
you use TestBrowserProxy to intercept calls to the browser instead? I would also
suggest renaming PasswordManager to PasswordBrowserProxy or something like that,
given that it calls chrome.* APIs.

         // Verify that the event matches the expected value.
-        assertEquals(firstPassword.loginPair.originUrl,
-                     event.detail.originUrl);
-        assertEquals(firstPassword.loginPair.username,
-                     event.detail.username);
+        assertEquals(firstPassword.loginPair.originUrl, detail.originUrl);
+        assertEquals(firstPassword.loginPair.username, detail.username);
+
+        // Clean up after self.
+        self.passwordManager.onRemoveSavedPassword = null;
+
         done();
-      });
+      };
 
       // Click the remove button on the first password.
       MockInteractions.tap(firstNode.querySelector('#passwordMenu'));
@@ -397,19 +406,20 @@ TEST_F('SettingsPasswordSectionBrowserTest', 'uiTests', function() {
             exceptions[index].querySelector('#removeExceptionButton'));
       };
 
-      // Listen for the remove event. If this event isn't received, the test
-      // will time out and fail.
-      passwordsSection.addEventListener('remove-password-exception',

[Dan Beam, 2017/01/27 01:11:32]

why can't we just use events like we were before?

[hcarmona, 2017/01/27 18:28:16]

The event was fired so the parent element could handle it because the
passwordsSection did not know about the passwordManager. The passwordsSection
can interact directly with the passwordManager now that it has a reference to
it.

I can put the event back in, but it seems unnecessary now.

-                                        function(event) {
+      self.passwordManager.onRemoveException = function(detail) {
         // Verify that the event matches the expected value.
         assertTrue(index < exceptionList.length);
-        assertEquals(exceptionList[index].exceptionUrl, event.detail);
+        assertEquals(exceptionList[index].exceptionUrl, detail);
 
-        if (++index < exceptionList.length)
+        if (++index < exceptionList.length) {
           clickRemoveButton();  // Click 'remove' on all passwords, one by one.
-        else
+        } else {
+          // Clean up after self.
+          self.passwordManager.onRemoveException = null;
+
           done();
-      });
+        }
+      };
 
       // Start removing.
       clickRemoveButton();