Pop up requestAutocomplete UI when autofill server hints chrome client that it is in a multipage au…

chrome/browser/autofill/autofill_flow_infobar_delegate.h

+// Copyright (c) 2012 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_AUTOFILL_AUTOFILL_FLOW_INFOBAR_DELEGATE_H_
+#define CHROME_BROWSER_AUTOFILL_AUTOFILL_FLOW_INFOBAR_DELEGATE_H_
+
+#include "base/basictypes.h"
+#include "base/gtest_prod_util.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/string16.h"
+#include "chrome/browser/api/infobars/confirm_infobar_delegate.h"
+#include "chrome/browser/autofill/autofill_manager.h"

[Ilya Sherman, 2013/01/15 06:31:52]

nit: Forward-declare

[Raman Kakilate, 2013/01/15 23:02:33]

Done.

+#include "chrome/browser/autofill/autofill_metrics.h"
+#include "chrome/common/form_data.h"
+#include "googleurl/src/gurl.h"
+#include "webkit/glue/window_open_disposition.h"

[Ilya Sherman, 2013/01/15 06:31:52]

nit: I'd omit this, as it's only used by a method that you're overriding, i.e.
it should already have been included by the parent class's header file.

[Raman Kakilate, 2013/01/15 23:02:33]

Done.

+
+namespace content {
+struct LoadCommittedDetails;
+struct SSLStatus;
+}
+
+// An InfoBar delegate that enables the user to allow or deny storing credit
+// card information gathered from a form submission.
+class AutofillFlowInfoBarDelegate : public ConfirmInfoBarDelegate {

[Ilya Sherman, 2013/01/15 06:31:52]

nit: Since Alex has checked in other code that uses the term "Autocheckout", I'd
prefer for all the uses of "AutofillFlow" in this CL to be "Autocheckout"
instead.

[Raman Kakilate, 2013/01/15 23:02:33]

Done.

+ public:
+  // Creates an autofillflow infobar delegate and adds it to |infobar_service|.
+  static void Create(InfoBarService* infobar_service,
+                     AutofillManager* autofill_manager,
+                     const AutofillMetrics* metric_logger,

[Ilya Sherman, 2013/01/15 06:31:52]

nit: Pass by const-ref, not const-pointer.

[Raman Kakilate, 2013/01/15 23:02:33]

I have been consistent with autofill_cc_inforbar_delegate. Will try to change
both of them in another CL.

[Ilya Sherman, 2013/01/15 23:24:46]

This isn't desirable consistency, since the style in that file is wrong.  Let's
just fix it here while you're here, and then fix it there in a separate CL.

[Albert Bodenhamer, 2013/01/16 01:08:26]

+1 I'm not sure why it was done that way in the other file.

On 2013/01/15 23:24:46, Ilya Sherman wrote:

[Raman Kakilate, 2013/01/16 19:20:12]

probably my sloppy c++ skills at show here.

* AutofillMetrics disallows COPY and ASSIGN.
* For passing it as const reference, object needs to be copy'able ?

Other option is pass is as simple pointer instead of const pointer.

What should be correct way to handle this ?
On 2013/01/16 01:08:26, Albert Bodenhamer wrote:

[Ilya Sherman, 2013/01/17 01:21:44]

The object does not need to be copyable to be passed by const-reference.  A
reference is just a pointer that can't be NULL, plus some syntactic sugar.  You
can store the the logger either as a const-reference (preferred, IMO) or a
const-pointer.  If you try to store it as a direct member instead, that will
introduce a copy, which will fail because the type is not copyable.

+                     const GURL& source_url,
+                     const content::SSLStatus& ssl_status);

[Ilya Sherman, 2013/01/15 06:31:52]

nit: Const params should precede non-const ones.

[Raman Kakilate, 2013/01/15 23:02:33]

Done.

+
+#if defined(UNIT_TEST)
+  static scoped_ptr<ConfirmInfoBarDelegate> Create(
+      AutofillManager* autofill_manager,
+      const AutofillMetrics* metric_logger,
+      const GURL& source_url,
+      const content::SSLStatus& ssl_status) {
+    return scoped_ptr<ConfirmInfoBarDelegate>(new AutofillFlowInfoBarDelegate(
+        NULL, autofill_manager, metric_logger, source_url, ssl_status));
+  }
+#endif
+
+ private:
+  AutofillFlowInfoBarDelegate(InfoBarService* infobar_service,
+                              AutofillManager* autofill_manager,
+                              const AutofillMetrics* metric_logger,
+                              const GURL& source_url,
+                              const content::SSLStatus& ssl_status);
+
+  virtual ~AutofillFlowInfoBarDelegate();
+
+  // Logs UMA metric for user action type.
+  void LogUserAction(AutofillMetrics::InfoBarMetric user_action);
+
+  // ConfirmInfoBarDelegate:
+  virtual void InfoBarDismissed() OVERRIDE;
+  virtual gfx::Image* GetIcon() const OVERRIDE;
+  virtual Type GetInfoBarType() const OVERRIDE;
+  virtual bool ShouldExpireInternal(
+      const content::LoadCommittedDetails& details) const OVERRIDE;
+  virtual string16 GetMessageText() const OVERRIDE;
+  virtual string16 GetButtonLabel(InfoBarButton button) const OVERRIDE;
+  virtual bool Accept() OVERRIDE;
+  virtual bool Cancel() OVERRIDE;
+  virtual string16 GetLinkText() const OVERRIDE;
+  virtual bool LinkClicked(WindowOpenDisposition disposition) OVERRIDE;
+
+  // For logging UMA metrics.
+  // Weak reference. Owned by the AutofillManager that initiated this infobar.
+  const AutofillMetrics* metric_logger_;
+
+  // To callback AutofillManager's ShowAutofillFlowDialog.
+  AutofillManager* autofill_manager_;
+
+  // URL of the page which triggered infobar.
+  GURL source_url_;
+
+  // SSL status of the page which triggered infobar.
+  content::SSLStatus ssl_status_;
+
+  // Did the user ever explicitly accept or dismiss this infobar?
+  bool had_user_interaction_;
+
+  // AutofillFlow Formdata to be used.

[Ilya Sherman, 2013/01/15 06:31:52]

nit: This comment is almost completely redundant with the code.  Perhaps: "The
form the that will be filled if the user accepts the infobar"?

[Raman Kakilate, 2013/01/15 23:02:33]

Removed.

+  FormData autofill_flow_form_data_;
+
+  FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest, AutofillFlowInfoBar);
+
+  DISALLOW_COPY_AND_ASSIGN(AutofillFlowInfoBarDelegate);
+};
+
+#endif  // CHROME_BROWSER_AUTOFILL_AUTOFILL_FLOW_INFOBAR_DELEGATE_H_