[Password Manager] Update Confirmation UI for saved password change for Chrome on Android.

chrome/browser/password_manager/update_password_infobar_delegate.h

+// 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.
+
+#ifndef CHROME_BROWSER_PASSWORD_MANAGER_UPDATE_PASSWORD_INFOBAR_DELEGATE_H_
+#define CHROME_BROWSER_PASSWORD_MANAGER_UPDATE_PASSWORD_INFOBAR_DELEGATE_H_
+
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/password_manager/password_manager_infobar_delegate.h"
+#include "chrome/browser/ui/passwords/manage_passwords_state.h"
+#include "components/password_manager/core/browser/password_form_manager.h"
+
+namespace content {
+class WebContents;
+}
+
+// If PasswordManager encounters the login which is already known or user fills
+// the password change form, then user is prompted with an infobar asking if
+// already saved credentials should be updated or not.
+// In case several credentials are stored for the web site, user can choose
+// which one should be updated.
+class UpdatePasswordInfoBarDelegate : public PasswordManagerInfoBarDelegate {
+ public:
+  static void Create(
+      content::WebContents* web_contents,
+      scoped_ptr<password_manager::PasswordFormManager> form_to_update);
+
+  ~UpdatePasswordInfoBarDelegate() override;
+
+  base::string16 branding() const { return branding_; }
+

[Peter Kasting, 2016/02/03 00:56:27]

Nit: Blank line unnecessary

[melandory, 2016/02/12 19:19:37]

Done.

+  bool is_smartlock_branding_enabled() const {
+    return is_smartlock_branding_enabled_;
+  }
+
+  // Returns true if infobar for the multi credentials case should be shown, e.g
+  // user changing a password on a password change form and has several
+  // credentials save for the web site.

[Peter Kasting, 2016/02/03 00:56:27]

Nit: Maybe "Returns true if an infobar should be shown when there are multiple
matching credentials, e.g. when a user uses a password change form on a site
with several saved credentials."

Although that assumes I've understood the intent of this function correctly. 
Maybe it's not actually "given that there are multiple accounts, return whether
to show the infobar" and more "given that we're showing an infobar, return
whether it will talk about multiple accounts".

[melandory, 2016/02/12 19:19:37]

We show infobar even in case of one credentials. This helper functions tell us
that we should show special case of update infobar -> update infobar for case,
when user has multiple credentials stored

[Peter Kasting, 2016/02/13 01:44:52]

This comment doesn't communicate that clearly at all.

Maybe "Returns whether the infobar should show the credentials from multiple
accounts [where?], e.g. when the user is trying to change the password on a site
with multiple saved credentials."

+  bool ShowMultipleAccounts() const;
+
+  const std::vector<const autofill::PasswordForm*>& GetCurrentForms() const {

[Peter Kasting, 2016/02/03 00:56:27]

Do not inline CamelCase functions.  This should be defined out-of-line.

[melandory, 2016/02/12 19:19:37]

Done.

+    return passwords_data_.GetCurrentForms();
+  }
+
+  // Returns the username of the saved credentials in case when there is only

[Peter Kasting, 2016/02/03 00:56:27]

Nit: in -> in the

[melandory, 2016/02/12 19:19:37]

Done.

+  // one credential pair stored.
+  base::string16 GetUsernameForOneAccountCase();
+
+  // ConfirmInfoBarDelegate:
+  infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;
+  base::string16 GetButtonLabel(InfoBarButton button) const override;
+  bool Accept() override;
+  bool Cancel() override;

[Peter Kasting, 2016/02/03 00:56:27]

Nit: Do these really need to be public?  Make them private if possible.

[melandory, 2016/02/12 19:19:36]

Done.

+
+ private:
+  UpdatePasswordInfoBarDelegate(
+      content::WebContents* web_contents,
+      scoped_ptr<password_manager::PasswordFormManager> form_to_update,
+      bool is_smartlock_branding_enabled);
+
+  // Updates either pending credentials of form with |form_index| in case
+  // multiple choice available.

[Peter Kasting, 2016/02/03 00:56:27]

This comment makes no sense to me.

I don't think we should have this helper anyway; it's very short and called only
once, and its contents would be better just inlined into the lone caller.

In fact, I would probably inline the two methods below as well, since they share
common prologue code (get the form manager) and epilogue (call Update() on it)
and thus that could be common-factored out of the inlined implementations.

[melandory, 2016/02/12 19:19:37]

Done.

+  void UpdatePassword(int form_index);
+
+  // Updates the pending credentials.
+  void UpdatePasswordWithPending();
+
+  // Updates the form with index |form_index| in case of multiple credentials.

[Peter Kasting, 2016/02/03 00:56:27]

Nit: in -> in the

[melandory, 2016/02/12 19:19:37]

Done.

+  void UpdatePasswordWithCurrentForm(int form_index);
+
+  ManagePasswordsState passwords_data_;
+  base::string16 branding_;
+  bool is_smartlock_branding_enabled_;
+
+  DISALLOW_COPY_AND_ASSIGN(UpdatePasswordInfoBarDelegate);
+};
+
+#endif  // CHROME_BROWSER_PASSWORD_MANAGER_UPDATE_PASSWORD_INFOBAR_DELEGATE_H_