Credentials chooser UI for Android.

chrome/browser/password_manager/account_chooser_infobar_delegate_android.h

+// Copyright 2015 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_ACCOUNT_CHOOSER_INFOBAR_DELEGATE_ANDROID_H_
+#define CHROME_BROWSER_PASSWORD_MANAGER_ACCOUNT_CHOOSER_INFOBAR_DELEGATE_ANDROID_H_
+
+#include "base/macros.h"
+#include "base/memory/scoped_vector.h"

[Peter Kasting, 2015/02/16 20:37:07]

Nit: This #include isn't needed, the type in question comes from
manage_passwords_ui_controller.h.

[melandory, 2015/02/17 16:16:58]

Done.

+#include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h"
+#include "components/infobars/core/infobar_delegate.h"
+
+namespace content {
+class WebContents;
+}
+
+namespace password_manager {
+enum class CredentialType : unsigned int;
+}
+
+namespace autofill {
+struct PasswordForm;
+}
+
+// Android-only infobar to notify that the generated password was saved.

[Peter Kasting, 2015/02/16 20:37:07]

This comment doesn't sound correct.  It sounds like you copy-and-pasted it from
another class.

[melandory, 2015/02/17 16:16:58]

Done.

+class AccountChooserInfoBarDelegateAndroid : public infobars::InfoBarDelegate {
+ public:
+  // Creates and shows the infobar. Implemented in the platform-specific file.

[Peter Kasting, 2015/02/16 20:37:07]

Nit: Please follow other infobar delegates' comment style (see next comment for
why I write "infobar service"):

Creates an account chooser infobar and delegate and adds the infobar to
|infobar_service|.

You should place the note about where this is implemented -- including the
precise filename -- in the .cc file where you would have otherwise defined this.
 That said, why does this have to be defined in a different file?  Why can't the
.cc file here #include account_chooser_infobar.h and just define the method in
question?

[melandory, 2015/02/17 16:16:58]

Done.

+  static void Create(content::WebContents* web_contents,

[Peter Kasting, 2015/02/16 20:37:07]

Pass an InfoBarService* instead of a WebContents*.

[melandory, 2015/02/17 16:16:58]

Done.

+                     ManagePasswordsUIController* ui_controller);
+
+  ~AccountChooserInfoBarDelegateAndroid() override = default;
+
+  const ScopedVector<autofill::PasswordForm>& local_credentials_forms() const {
+    return ui_controller_->local_credentials_forms();
+  }
+
+  void choose_credential(unsigned int credential_index,

[Peter Kasting, 2015/02/16 20:37:07]

"unsigned int" is banned by the Google style guide.  You should be using int or
size_t (probably the latter based on the .cc file).  This may mean you need to
fix the caller side.

Nit: Use CamelCase names for non-inlined functions.

[melandory, 2015/02/17 16:16:57]

Done.

+                         password_manager::CredentialType credential_type);
+
+ private:
+  explicit AccountChooserInfoBarDelegateAndroid(
+      ManagePasswordsUIController* ui_controller);
+
+  // InfoBarDelegate implementation:

[Peter Kasting, 2015/02/16 20:37:07]

Nit: Prefix "InfoBarDelegate" with "infobars::" and remove "implementation"

[melandory, 2015/02/17 16:16:58]

Done.

+  Type GetInfoBarType() const override;
+  AccountChooserInfoBarDelegateAndroid* AsAccountChooserInfoBarDelegateAndroid()
+      override;
+
+  // TODO(melandory): It looks like ManagePasswordUIController has logic which
+  // should be extracted from it (storing data, propagating user actions), as
+  // for example TranslateUIDelegate does it.

[Peter Kasting, 2015/02/16 20:37:07]

Nit: This comment sounds like it belongs in ManagePasswordUIController somewhere
instead of here.

[melandory, 2015/02/17 16:16:58]

Removed it for now.

+  // AccountChooserInfoBarDelegateAndroid doesn't own this pointer.

[Peter Kasting, 2015/02/16 20:37:07]

So who does own it?  "Owned by ____" would be shorter and clearer.

[melandory, 2015/02/17 16:16:58]

Done.

+  ManagePasswordsUIController* ui_controller_;
+
+  DISALLOW_COPY_AND_ASSIGN(AccountChooserInfoBarDelegateAndroid);
+};
+
+#endif  // CHROME_BROWSER_PASSWORD_MANAGER_ACCOUNT_CHOOSER_INFOBAR_DELEGATE_ANDROID_H_