Refactoring multipage autofill data out of every form and into one common location.

BUG=


Review URL: https://chromiumcodereview.appspot.com/12040088

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@179351 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 160 insertions, 94 deletions

diff --git a/chrome/browser/autofill/autocheckout_page_meta_data.cc b/chrome/browser/autofill/autocheckout_page_meta_data.cc
new file mode 100644
index 0000000..90531bc
--- /dev/null
+++ b/chrome/browser/autofill/autocheckout_page_meta_data.cc
@@ -0,0 +1,23 @@
+// Copyright (c) 2013 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.
+
+#include "chrome/browser/autofill/autocheckout_page_meta_data.h"
+
+namespace autofill {
+
+AutocheckoutPageMetaData::AutocheckoutPageMetaData()
+    : current_page_number(-1),
+      total_pages(-1) {}
+
+AutocheckoutPageMetaData::~AutocheckoutPageMetaData() {}
+
+bool AutocheckoutPageMetaData::IsStartOfAutofillableFlow() const {
+  return current_page_number == 0 && total_pages > 0;
+}
+
+bool AutocheckoutPageMetaData::IsInAutofillableFlow() const {
+  return current_page_number >= 0 && current_page_number < total_pages;
+}
+
+}  // namesapce autofill
diff --git a/chrome/browser/autofill/autocheckout_page_meta_data.h b/chrome/browser/autofill/autocheckout_page_meta_data.h
new file mode 100644
index 0000000..d178028
--- /dev/null
+++ b/chrome/browser/autofill/autocheckout_page_meta_data.h
@@ -0,0 +1,45 @@
+// Copyright (c) 2013 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_AUTOCHECKOUT_PAGE_META_DATA_H_
+#define CHROME_BROWSER_AUTOFILL_AUTOCHECKOUT_PAGE_META_DATA_H_
+
+#include "base/memory/scoped_ptr.h"
+#include "chrome/common/autofill/web_element_descriptor.h"
+
+namespace autofill {
+
+// Container for multipage Autocheckout data.
+struct AutocheckoutPageMetaData {
+  AutocheckoutPageMetaData();
+  ~AutocheckoutPageMetaData();
+
+  // Returns true if the Autofill server says that the current page is start of
+  // a multipage Autofill flow.
+  bool IsStartOfAutofillableFlow() const;
+
+  // Returns true if the Autofill server says that the current page is in a
+  // multipage Autofill flow.
+  bool IsInAutofillableFlow() const;
+
+  // Page number of the multipage Autofill flow this form belongs to
+  // (zero-indexed). If this form doesn't belong to any autofill flow, it is set
+  // to -1.
+  int current_page_number;
+
+  // Total number of pages in the multipage Autofill flow. If this form doesn't
+  // belong to any autofill flow, it is set to -1.
+  int total_pages;
+
+  // The proceed element of the multipage Autofill flow. Can be null if the
+  // current page is the last page of a flow or isn't a member of a flow.
+  scoped_ptr<WebElementDescriptor> proceed_element_descriptor;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AutocheckoutPageMetaData);
+};
+
+}  // namespace autofill
+
+#endif  // CHROME_BROWSER_AUTOFILL_AUTOCHECKOUT_PAGE_META_DATA_H_
diff --git a/chrome/browser/autofill/autocheckout_page_meta_data_unittest.cc b/chrome/browser/autofill/autocheckout_page_meta_data_unittest.cc
new file mode 100644
index 0000000..355c1b7
--- /dev/null
+++ b/chrome/browser/autofill/autocheckout_page_meta_data_unittest.cc
@@ -0,0 +1,48 @@
+// Copyright (c) 2013 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.
+
+#include "chrome/browser/autofill/autocheckout_page_meta_data.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+void SetPageDetails(autofill::AutocheckoutPageMetaData* meta_data,
+                    int current_page,
+                    int total) {
+  meta_data->current_page_number = current_page;
+  meta_data->total_pages = total;
+}
+
+}  // namespace
+
+namespace autofill {
+
+TEST(AutocheckoutPageMetaDataTest, AutofillableFlow) {
+
+  AutocheckoutPageMetaData page_meta_data;
+  EXPECT_FALSE(page_meta_data.IsStartOfAutofillableFlow());
+  EXPECT_FALSE(page_meta_data.IsInAutofillableFlow());
+
+  SetPageDetails(&page_meta_data, -1, 0);
+  EXPECT_FALSE(page_meta_data.IsStartOfAutofillableFlow());
+  EXPECT_FALSE(page_meta_data.IsInAutofillableFlow());
+
+  SetPageDetails(&page_meta_data, 0, 0);
+  EXPECT_FALSE(page_meta_data.IsStartOfAutofillableFlow());
+  EXPECT_FALSE(page_meta_data.IsInAutofillableFlow());
+
+  SetPageDetails(&page_meta_data, 0, 1);
+  EXPECT_TRUE(page_meta_data.IsStartOfAutofillableFlow());
+  EXPECT_TRUE(page_meta_data.IsInAutofillableFlow());
+
+  SetPageDetails(&page_meta_data, 1, 2);
+  EXPECT_FALSE(page_meta_data.IsStartOfAutofillableFlow());
+  EXPECT_TRUE(page_meta_data.IsInAutofillableFlow());
+
+  SetPageDetails(&page_meta_data, 2, 2);
+  EXPECT_FALSE(page_meta_data.IsStartOfAutofillableFlow());
+  EXPECT_FALSE(page_meta_data.IsInAutofillableFlow());
+}
+
+}  // namespace autofill
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index d244f87..cbdcbae 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -581,7 +581,7 @@ void AutofillManager::OnQueryFormFieldAutofill(int query_id,
     // If form is known to be at the start of the autofillable flow (i.e, when
     // Autofill server said so), then trigger payments UI while also returning
     // standard autofill suggestions to renderer process.
-    if (form_structure->IsStartOfAutofillableFlow()) {
+    if (page_meta_data_.IsStartOfAutofillableFlow()) {
       AutocheckoutInfoBarDelegate::Create(
           *metric_logger_,
           form.origin,
@@ -875,6 +875,7 @@ void AutofillManager::OnLoadedServerPredictions(
   // Parse and store the server predictions.
   FormStructure::ParseQueryResponse(response_xml,
                                     form_structures_.get(),
+                                    &page_meta_data_,
                                     *metric_logger_);
 
   // If the corresponding flag is set, annotate forms with the predicted types.
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index 2092457..2acfbba 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -23,6 +23,7 @@
 #include "base/time.h"
 #include "chrome/browser/api/sync/profile_sync_service_observer.h"
 #include "chrome/browser/autofill/autocheckout_manager.h"
+#include "chrome/browser/autofill/autocheckout_page_meta_data.h"
 #include "chrome/browser/autofill/autocomplete_history_manager.h"
 #include "chrome/browser/autofill/autofill_download.h"
 #include "chrome/browser/autofill/autofill_manager_delegate.h"
@@ -386,6 +387,10 @@ class AutofillManager : public content::WebContentsObserver,
   // Our copy of the form data.
   ScopedVector<FormStructure> form_structures_;
 
+  // To be passed to FormStructure::ParseQueryResponse to gather the page meta
+  // data.
+  autofill::AutocheckoutPageMetaData page_meta_data_;
+
   // GUID to ID mapping.  We keep two maps to convert back and forth.
   mutable std::map<PersonalDataManager::GUIDPair, int> guid_id_map_;
   mutable std::map<int, PersonalDataManager::GUIDPair> id_guid_map_;
diff --git a/chrome/browser/autofill/autofill_metrics_unittest.cc b/chrome/browser/autofill/autofill_metrics_unittest.cc
index 7ca9144..0141830 100644
--- a/chrome/browser/autofill/autofill_metrics_unittest.cc
+++ b/chrome/browser/autofill/autofill_metrics_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autocheckout_infobar_delegate.h"
+#include "chrome/browser/autofill/autocheckout_page_meta_data.h"
 #include "chrome/browser/autofill/autofill_cc_infobar_delegate.h"
 #include "chrome/browser/autofill/autofill_common_test.h"
 #include "chrome/browser/autofill/autofill_manager.h"
@@ -1301,9 +1302,12 @@ TEST_F(AutofillMetricsTest, ServerQueryExperimentIdForQuery) {
   EXPECT_CALL(metric_logger,
               LogServerQueryMetric(
                   AutofillMetrics::QUERY_RESPONSE_MATCHED_LOCAL_HEURISTICS));
+  autofill::AutocheckoutPageMetaData page_meta_data;
   FormStructure::ParseQueryResponse(
       "<autofillqueryresponse></autofillqueryresponse>",
-      std::vector<FormStructure*>(), metric_logger);
+      std::vector<FormStructure*>(),
+      &page_meta_data,
+      metric_logger);
 
   // Experiment "ar1" specified.
   EXPECT_CALL(metric_logger,
@@ -1317,7 +1321,9 @@ TEST_F(AutofillMetricsTest, ServerQueryExperimentIdForQuery) {
                   AutofillMetrics::QUERY_RESPONSE_MATCHED_LOCAL_HEURISTICS));
   FormStructure::ParseQueryResponse(
       "<autofillqueryresponse experimentid=\"ar1\"></autofillqueryresponse>",
-      std::vector<FormStructure*>(), metric_logger);
+      std::vector<FormStructure*>(),
+      &page_meta_data,
+      metric_logger);
 }
 
 // Verify that we correctly log user happiness metrics dealing with form loading
diff --git a/chrome/browser/autofill/form_structure.cc b/chrome/browser/autofill/form_structure.cc
index 28d3535..4f11055 100644
--- a/chrome/browser/autofill/form_structure.cc
+++ b/chrome/browser/autofill/form_structure.cc
@@ -16,12 +16,13 @@
 #include "base/stringprintf.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
-#include "chrome/common/chrome_switches.h"
+#include "chrome/browser/autofill/autocheckout_page_meta_data.h"
 #include "chrome/browser/autofill/autofill_metrics.h"
 #include "chrome/browser/autofill/autofill_type.h"
 #include "chrome/browser/autofill/autofill_xml_parser.h"
 #include "chrome/browser/autofill/field_types.h"
 #include "chrome/browser/autofill/form_field.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/form_data.h"
 #include "chrome/common/form_data_predictions.h"
 #include "chrome/common/form_field_data.h"
@@ -230,8 +231,6 @@ FormStructure::FormStructure(const FormData& form)
       checkable_field_count_(0),
       upload_required_(USE_UPLOAD_RATES),
       server_experiment_id_("no server response"),
-      current_page_number_(-1),
-      total_pages_(-1),
       has_author_specified_types_(false),
       experimental_form_filling_enabled_(
           CommandLine::ForCurrentProcess()->HasSwitch(
@@ -414,9 +413,11 @@ bool FormStructure::EncodeQueryRequest(
 }
 
 // static
-void FormStructure::ParseQueryResponse(const std::string& response_xml,
-                                       const std::vector<FormStructure*>& forms,
-                                       const AutofillMetrics& metric_logger) {
+void FormStructure::ParseQueryResponse(
+    const std::string& response_xml,
+    const std::vector<FormStructure*>& forms,
+    autofill::AutocheckoutPageMetaData* page_meta_data,
+    const AutofillMetrics& metric_logger) {
   metric_logger.LogServerQueryMetric(AutofillMetrics::QUERY_RESPONSE_RECEIVED);
 
   // Parse the field types from the server response to the query.
@@ -430,6 +431,16 @@ void FormStructure::ParseQueryResponse(const std::string& response_xml,
   if (!parse_handler.succeeded())
     return;
 
+  page_meta_data->current_page_number = parse_handler.current_page_number();
+  page_meta_data->total_pages = parse_handler.total_pages();
+  if (parse_handler.proceed_element_descriptor()) {
+    page_meta_data->proceed_element_descriptor.reset(
+        new autofill::WebElementDescriptor(
+            *parse_handler.proceed_element_descriptor()));
+  } else {
+    page_meta_data->proceed_element_descriptor.reset();
+  }
+
   metric_logger.LogServerQueryMetric(AutofillMetrics::QUERY_RESPONSE_PARSED);
   metric_logger.LogServerExperimentIdForQuery(experiment_id);
 
@@ -444,13 +455,6 @@ void FormStructure::ParseQueryResponse(const std::string& response_xml,
     FormStructure* form = *iter;
     form->upload_required_ = upload_required;
     form->server_experiment_id_ = experiment_id;
-    form->current_page_number_ = parse_handler.current_page_number();
-    form->total_pages_ = parse_handler.total_pages();
-    if (parse_handler.proceed_element_descriptor()) {
-      form->proceed_element_descriptor_.reset(
-          new autofill::WebElementDescriptor(
-              *parse_handler.proceed_element_descriptor()));
-    }
 
     for (std::vector<AutofillField*>::iterator field = form->fields_.begin();
          field != form->fields_.end(); ++field, ++current_info) {
@@ -1055,14 +1059,6 @@ void FormStructure::ParseFieldTypesFromAutocompleteAttributes(
   }
 }
 
-bool FormStructure::IsStartOfAutofillableFlow() const {
-  return current_page_number_ == 0 && total_pages_ > 0;
-}
-
-bool FormStructure::IsInAutofillableFlow() const {
-  return current_page_number_ >= 0 && current_page_number_ < total_pages_;
-}
-
 void FormStructure::IdentifySections(bool has_author_specified_sections) {
   if (fields_.empty())
     return;
diff --git a/chrome/browser/autofill/form_structure.h b/chrome/browser/autofill/form_structure.h
index 72485af..8b16282 100644
--- a/chrome/browser/autofill/form_structure.h
+++ b/chrome/browser/autofill/form_structure.h
@@ -33,6 +33,10 @@ enum UploadRequired {
 
 class AutofillMetrics;
 
+namespace autofill {
+struct AutocheckoutPageMetaData;
+}
+
 namespace base {
 class TimeTicks;
 }
@@ -68,9 +72,11 @@ class FormStructure {
 
   // Parses the field types from the server query response. |forms| must be the
   // same as the one passed to EncodeQueryRequest when constructing the query.
-  static void ParseQueryResponse(const std::string& response_xml,
-                                 const std::vector<FormStructure*>& forms,
-                                 const AutofillMetrics& metric_logger);
+  static void ParseQueryResponse(
+      const std::string& response_xml,
+      const std::vector<FormStructure*>& forms,
+      autofill::AutocheckoutPageMetaData* page_meta_data,
+      const AutofillMetrics& metric_logger);
 
   // Fills |forms| with the details from the given |form_structures| and their
   // fields' predicted types.
@@ -126,20 +132,6 @@ class FormStructure {
   void ParseFieldTypesFromAutocompleteAttributes(bool* found_types,
                                                  bool* found_sections);
 
-  // Returns true if the autofill server says that the current page is start of
-  // the autofillable flow.
-  bool IsStartOfAutofillableFlow() const;
-
-  // Returns true if the autofill server says that the current page is in the
-  // autofillable flow.
-  bool IsInAutofillableFlow() const;
-
-  // Returns the proceed element for multipage Autofill flows if the current
-  // page is part of such a flow or NULL otherwise.
-  const autofill::WebElementDescriptor* proceed_element_descriptor() const {
-    return proceed_element_descriptor_.get();
-  }
-
   const AutofillField* field(size_t index) const;
   AutofillField* field(size_t index);
   size_t field_count() const;
@@ -228,17 +220,6 @@ class FormStructure {
   // GET or POST.
   RequestMethod method_;
 
-  // Page number of the autofill flow this form belongs to (zero-indexed).
-  // If this form doesn't belong to any autofill flow, it is set to -1.
-  int current_page_number_;
-
-  // Total number of pages in the autofill flow. If this form doesn't belong
-  // to any autofill flow, it is set to -1.
-  int total_pages_;
-
-  // Proceed element for multipage Autofill flow.
-  scoped_ptr<autofill::WebElementDescriptor> proceed_element_descriptor_;
-
   // Whether the form includes any field types explicitly specified by the site
   // author, via the |autocompletetype| attribute.
   bool has_author_specified_types_;
diff --git a/chrome/browser/autofill/form_structure_unittest.cc b/chrome/browser/autofill/form_structure_unittest.cc
index aca45d1..4dbc3ce 100644
--- a/chrome/browser/autofill/form_structure_unittest.cc
+++ b/chrome/browser/autofill/form_structure_unittest.cc
@@ -61,13 +61,6 @@ class FormStructureTest {
   static std::string Hash64Bit(const std::string& str) {
     return FormStructure::Hash64Bit(str);
   }
-
-  static void SetPageDetails(FormStructure* form,
-                             int page_number,
-                             int total_pages) {
-    form->current_page_number_ = page_number;
-    form->total_pages_ = total_pages;
-  }
 };
 
 TEST(FormStructureTest, FieldCount) {
@@ -96,41 +89,6 @@ TEST(FormStructureTest, FieldCount) {
   EXPECT_EQ(3U, form_structure.field_count());
 }
 
-TEST(FormStructureTest, AutofillFlowInfo) {
-  FormData form;
-  form.method = ASCIIToUTF16("post");
-
-  FormFieldData field;
-  field.label = ASCIIToUTF16("username");
-  field.name = ASCIIToUTF16("username");
-  field.form_control_type = "text";
-  form.fields.push_back(field);
-
-  FormStructure form_structure(form);
-  EXPECT_FALSE(form_structure.IsStartOfAutofillableFlow());
-  EXPECT_FALSE(form_structure.IsInAutofillableFlow());
-
-  FormStructureTest::SetPageDetails(&form_structure, -1, 0);
-  EXPECT_FALSE(form_structure.IsStartOfAutofillableFlow());
-  EXPECT_FALSE(form_structure.IsInAutofillableFlow());
-
-  FormStructureTest::SetPageDetails(&form_structure, 0, 0);
-  EXPECT_FALSE(form_structure.IsStartOfAutofillableFlow());
-  EXPECT_FALSE(form_structure.IsInAutofillableFlow());
-
-  FormStructureTest::SetPageDetails(&form_structure, 0, 1);
-  EXPECT_TRUE(form_structure.IsStartOfAutofillableFlow());
-  EXPECT_TRUE(form_structure.IsInAutofillableFlow());
-
-  FormStructureTest::SetPageDetails(&form_structure, 1, 2);
-  EXPECT_FALSE(form_structure.IsStartOfAutofillableFlow());
-  EXPECT_TRUE(form_structure.IsInAutofillableFlow());
-
-  FormStructureTest::SetPageDetails(&form_structure, 2, 2);
-  EXPECT_FALSE(form_structure.IsStartOfAutofillableFlow());
-  EXPECT_FALSE(form_structure.IsInAutofillableFlow());
-}
-
 TEST(FormStructureTest, AutofillCount) {
   FormData form;
   form.method = ASCIIToUTF16("post");
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 661292c..7afd1fa 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -181,6 +181,8 @@
         'browser/autofill/autocheckout_infobar_delegate.h',
         'browser/autofill/autocheckout_manager.cc',
         'browser/autofill/autocheckout_manager.h',
+        'browser/autofill/autocheckout_page_meta_data.cc',
+        'browser/autofill/autocheckout_page_meta_data.h',
         'browser/autofill/autocomplete_history_manager.cc',
         'browser/autofill/autocomplete_history_manager.h',
         'browser/autofill/autofill-inl.h',
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index 7fa9bae..141a568 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -434,6 +434,7 @@
         'browser/autocomplete/shortcuts_provider_unittest.cc',
         'browser/autofill/address_field_unittest.cc',
         'browser/autofill/address_unittest.cc',
+        'browser/autofill/autocheckout_page_meta_data_unittest.cc',
         'browser/autofill/autocomplete_history_manager_unittest.cc',
         'browser/autofill/autofill_country_unittest.cc',
         'browser/autofill/autofill_download_unittest.cc',