Add a password handler to get the list of passwords in md-settings.

chrome/browser/resources/settings/passwords_and_forms_page/passwords_private_api.js

Index: chrome/browser/resources/settings/passwords_and_forms_page/passwords_private_api.js
diff --git a/chrome/browser/resources/settings/passwords_and_forms_page/passwords_private_api.js b/chrome/browser/resources/settings/passwords_and_forms_page/passwords_private_api.js
new file mode 100644
index 0000000000000000000000000000000000000000..92fca41e98951fc52edfda2aa576078254d78ae9
--- /dev/null
+++ b/chrome/browser/resources/settings/passwords_and_forms_page/passwords_private_api.js
@@ -0,0 +1,51 @@
+// Copyright 2016 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.
+
+cr.exportPath('settings');

[michaelpg, 2016/01/16 07:12:48]

exportPath is redundant with cr.define

[hcarmona, 2016/01/20 02:32:50]

Done.

+
+cr.define('settings', function() {
+  /**
+   * API to encapsulate messaging between JS and C++ for password management.
+   * @constructor
+   */
+  function PasswordsPrivateApi() {}

[michaelpg, 2016/01/16 07:12:48]

why is this a function/constructor?

if you just want the namespace:

    cr.define('settings.PasswordsPrivateApi', function() {
      function foo() {}
      function bar() {}
      return {foo: foo, bar: bar};
    });

[hcarmona, 2016/01/20 02:32:50]

Good to know. Updated.

+
+  /**
+   * Encapsulate the calls between JS and C++ to update the list of passwords.

[michaelpg, 2016/01/16 07:12:48]

use indicative comments for functions (e.g., Encapsulates the calls...)

[hcarmona, 2016/01/20 02:32:50]

Done.

+   * @param {function(!Array<!Object>)} passwordListCallback Called with the
+   *     list of passwords.
+   * @param {function(!Array<!Object>)} exceptionListCallback Called with the
+   *     list of exceptions.
+   */
+  PasswordsPrivateApi.updatePasswordLists = function(passwordListCallback,
+                                                     exceptionListCallback) {

[stevenjb, 2016/01/19 21:34:28]

API methods with multiple callback arguments are unusual. It is unclear what the
expected behavior should be if one or the other fails.

Can this API be defined to either make this two separate methods, or to provide
all of the data in a single callback?

[hcarmona, 2016/01/20 02:32:50]

Yes, we can do something else with this API. Not 100% sure what the
right approach is.

I like the idea of decoupling the update of these 2 lists and having 2
functions one to update each list.

That change doesn't look too complex, but would touch shared code
between the new and old settings. I can make a separate CL for that.
Should that be a prerequisite for this CL?

+    PasswordsPrivateApi.setSavedPasswordsList = passwordListCallback;

[michaelpg, 2016/01/16 07:12:48]

this is really weird. can you save the callback as a local variable or private
member, and call it from setSavedPasswordList? same for below

[hcarmona, 2016/01/20 02:32:50]

I'm not sure what you mean about this being "weird". I'm trying to see
how adding a local or private variable will improve this. This just
assigns the callback to the passed in functions, and adding another
variable to hold the callback seems odd to me because we can just call
the callback directly.

[dpapad, 2016/01/20 17:56:07]

Agree with Hector. What is the benefit of adding this extra indirection level
here?

[michaelpg, 2016/01/20 21:20:56]

You're overriding a method defined at line 40. So calling the function (eg from
C++) is a no-op until updatePasswordLists has been called by the javascript. At
least document that on that line. Furthermore the C++ needs to know not to call
the method multiple times in case the callback registered by updatePasswordLists
isn't intended to be used more than once.

What if updatePasswordLists is called twice, from two different places, before
the C++ round-trip completes? Only the 2nd callback will be called, and it will
be called twice?

[hcarmona, 2016/01/20 23:13:53]

I've added a comment that the callbacks will be no-op until they've
been set and that they may be called multiple times.

Right now, this code makes the assumptions that you mentioned. Would
the approach at http://crrev.com/1609923003 be better for this? I had
that as a POC for the discussion on C++/JS, but I think it makes sense
to use it here instead of the current approach.

+    PasswordsPrivateApi.setPasswordExceptionsList = exceptionListCallback;
+    chrome.send('updatePasswordLists');
+  };
+
+  /**
+   * Callback for displaying a password.
+   * @param {number} index
+   * @param {string} password
+   */
+  PasswordsPrivateApi.showPassword = function(index, password) {};

[michaelpg, 2016/01/16 07:12:48]

do you need a TODO here?

[hcarmona, 2016/01/20 02:32:50]

Done.

+
+  /**
+   * Callback for updating the list of passwords.
+   * @param {!Array<!Object>} passwordList The list of passwords that was
+   *     retrieved by calling the private APIs.
+   */
+  PasswordsPrivateApi.setSavedPasswordsList = function(passwordList) {};
+
+
+  /**
+   * Callback for updating the list of site exceptions.
+   * @param {!Array<!Object>} exceptionList The list of sites that passwords
+   *     will not be saved for.
+   */
+  PasswordsPrivateApi.setPasswordExceptionsList = function(exceptionList) {};
+
+  return { PasswordsPrivateApi: PasswordsPrivateApi, };
+});