Move some Autofill processing logic on form submit to a background thread.

BUG=76992
TEST=Autofill continues to upload form data to the crowdsourcing server


Review URL: http://codereview.chromium.org/8387016

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

1 files changed, 82 insertions, 33 deletions

diff --git a/chrome/browser/autofill/autofill_metrics_unittest.cc b/chrome/browser/autofill/autofill_metrics_unittest.cc
index 26b7af0..fc86dc7 100644
--- a/chrome/browser/autofill/autofill_metrics_unittest.cc
+++ b/chrome/browser/autofill/autofill_metrics_unittest.cc
@@ -178,10 +178,11 @@ class TestAutofillManager : public AutofillManager {
   TestAutofillManager(TabContentsWrapper* tab_contents,
                       TestPersonalDataManager* personal_manager)
       : AutofillManager(tab_contents, personal_manager),
-        autofill_enabled_(true) {
+        autofill_enabled_(true),
+        did_finish_async_form_submit_(false),
+        message_loop_is_running_(false) {
     set_metric_logger(new MockAutofillMetrics);
   }
-  virtual ~TestAutofillManager() {}
 
   virtual bool IsAutofillEnabled() const { return autofill_enabled_; }
 
@@ -210,8 +211,46 @@ class TestAutofillManager : public AutofillManager {
     form_structures()->push_back(form_structure);
   }
 
+  void FormSubmitted(const FormData& form, const TimeTicks& timestamp) {
+    if (!OnFormSubmitted(form, timestamp))
+      return;
+
+    // Wait for the asynchronous FormSubmitted() call to complete.
+    if (!did_finish_async_form_submit_) {
+      // TODO(isherman): It seems silly to need this variable.  Is there some
+      // way I can just query the message loop's state?
+      message_loop_is_running_ = true;
+      MessageLoop::current()->Run();
+    } else {
+      did_finish_async_form_submit_ = false;
+    }
+  }
+
+  virtual void UploadFormDataAsyncCallback(
+      const FormStructure* submitted_form,
+      const base::TimeTicks& load_time,
+      const base::TimeTicks& interaction_time,
+      const base::TimeTicks& submission_time) OVERRIDE {
+    if (message_loop_is_running_) {
+      MessageLoop::current()->Quit();
+      message_loop_is_running_ = false;
+    } else {
+      did_finish_async_form_submit_ = true;
+    }
+
+    AutofillManager::UploadFormDataAsyncCallback(submitted_form,
+                                                 load_time,
+                                                 interaction_time,
+                                                 submission_time);
+  }
+
  private:
+  // AutofillManager is ref counted.
+  virtual ~TestAutofillManager() {}
+
   bool autofill_enabled_;
+  bool did_finish_async_form_submit_;
+  bool message_loop_is_running_;
 
   DISALLOW_COPY_AND_ASSIGN(TestAutofillManager);
 };
@@ -223,30 +262,33 @@ class AutofillMetricsTest : public TabContentsWrapperTestHarness {
   AutofillMetricsTest();
   virtual ~AutofillMetricsTest();
 
-  virtual void SetUp();
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
 
  protected:
   AutofillCCInfoBarDelegate* CreateDelegate(MockAutofillMetrics* metric_logger,
                                             CreditCard** created_card);
 
-  scoped_ptr<TestAutofillManager> autofill_manager_;
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread file_thread_;
+
+  scoped_refptr<TestAutofillManager> autofill_manager_;
   TestPersonalDataManager personal_data_;
 
  private:
-  content::TestBrowserThread browser_thread_;
-
   DISALLOW_COPY_AND_ASSIGN(AutofillMetricsTest);
 };
 
 AutofillMetricsTest::AutofillMetricsTest()
   : TabContentsWrapperTestHarness(),
-    browser_thread_(BrowserThread::UI, &message_loop_) {
+    ui_thread_(BrowserThread::UI, &message_loop_),
+    file_thread_(BrowserThread::FILE) {
 }
 
 AutofillMetricsTest::~AutofillMetricsTest() {
   // Order of destruction is important as AutofillManager relies on
   // PersonalDataManager to be around when it gets destroyed.
-  autofill_manager_.reset(NULL);
+  autofill_manager_ = NULL;
 }
 
 void AutofillMetricsTest::SetUp() {
@@ -256,8 +298,15 @@ void AutofillMetricsTest::SetUp() {
       profile, TestPersonalDataManager::Build);
 
   TabContentsWrapperTestHarness::SetUp();
-  autofill_manager_.reset(new TestAutofillManager(contents_wrapper(),
-                                                  &personal_data_));
+  autofill_manager_ = new TestAutofillManager(contents_wrapper(),
+                                              &personal_data_);
+
+  file_thread_.Start();
+}
+
+void AutofillMetricsTest::TearDown() {
+  file_thread_.Stop();
+  TabContentsWrapperTestHarness::TearDown();
 }
 
 AutofillCCInfoBarDelegate* AutofillMetricsTest::CreateDelegate(
@@ -420,8 +469,8 @@ TEST_F(AutofillMetricsTest, QualityMetrics) {
                   AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_SOME));
 
   // Simulate form submission.
-  EXPECT_NO_FATAL_FAILURE(autofill_manager_->OnFormSubmitted(form,
-                                                             TimeTicks::Now()));
+  EXPECT_NO_FATAL_FAILURE(autofill_manager_->FormSubmitted(form,
+                                                           TimeTicks::Now()));
 }
 
 // Test that we log the appropriate additional metrics when Autofill failed.
@@ -538,8 +587,8 @@ TEST_F(AutofillMetricsTest, QualityMetricsForFailure) {
   }
 
   // Simulate form submission.
-  EXPECT_NO_FATAL_FAILURE(autofill_manager_->OnFormSubmitted(form,
-                                                             TimeTicks::Now()));
+  EXPECT_NO_FATAL_FAILURE(autofill_manager_->FormSubmitted(form,
+                                                           TimeTicks::Now()));
 }
 
 // Test that we behave sanely when the cached form differs from the submitted
@@ -696,8 +745,8 @@ TEST_F(AutofillMetricsTest, SaneMetricsWithCacheMismatch) {
                                std::string()));
 
   // Simulate form submission.
-  EXPECT_NO_FATAL_FAILURE(autofill_manager_->OnFormSubmitted(form,
-                                                             TimeTicks::Now()));
+  EXPECT_NO_FATAL_FAILURE(autofill_manager_->FormSubmitted(form,
+                                                           TimeTicks::Now()));
 }
 
 // Test that we don't log quality metrics for non-autofillable forms.
@@ -723,8 +772,8 @@ TEST_F(AutofillMetricsTest, NoQualityMetricsForNonAutofillableForms) {
   EXPECT_CALL(*autofill_manager_->metric_logger(),
               LogQualityMetric(AutofillMetrics::FIELD_SUBMITTED,
                                std::string())).Times(0);
-  EXPECT_NO_FATAL_FAILURE(autofill_manager_->OnFormSubmitted(form,
-                                                             TimeTicks::Now()));
+  EXPECT_NO_FATAL_FAILURE(autofill_manager_->FormSubmitted(form,
+                                                           TimeTicks::Now()));
 
   // Search forms are not auto-fillable.
   form.action = GURL("http://example.com/search?q=Elvis%20Presley");
@@ -736,8 +785,8 @@ TEST_F(AutofillMetricsTest, NoQualityMetricsForNonAutofillableForms) {
   EXPECT_CALL(*autofill_manager_->metric_logger(),
               LogQualityMetric(AutofillMetrics::FIELD_SUBMITTED,
                                std::string())).Times(0);
-  EXPECT_NO_FATAL_FAILURE(autofill_manager_->OnFormSubmitted(form,
-                                                             TimeTicks::Now()));
+  EXPECT_NO_FATAL_FAILURE(autofill_manager_->FormSubmitted(form,
+                                                           TimeTicks::Now()));
 }
 
 // Test that we recored the experiment id appropriately.
@@ -862,8 +911,8 @@ TEST_F(AutofillMetricsTest, QualityMetricsWithExperimentId) {
                                        ADDRESS_HOME_COUNTRY, experiment_id));
 
   // Simulate form submission.
-  EXPECT_NO_FATAL_FAILURE(autofill_manager_->OnFormSubmitted(form,
-                                                             TimeTicks::Now()));
+  EXPECT_NO_FATAL_FAILURE(autofill_manager_->FormSubmitted(form,
+                                                           TimeTicks::Now()));
 }
 
 // Test that the profile count is logged correctly.
@@ -1106,7 +1155,7 @@ TEST_F(AutofillMetricsTest, UserHappinessFormLoadAndSubmission) {
         *autofill_manager_->metric_logger(),
         LogUserHappinessMetric(
             AutofillMetrics::SUBMITTED_NON_FILLABLE_FORM)).Times(0);
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::Now());
+    autofill_manager_->FormSubmitted(form, TimeTicks::Now());
   }
 
   // Add more fields to the form.
@@ -1128,7 +1177,7 @@ TEST_F(AutofillMetricsTest, UserHappinessFormLoadAndSubmission) {
     EXPECT_CALL(*autofill_manager_->metric_logger(),
                 LogUserHappinessMetric(
                     AutofillMetrics::SUBMITTED_NON_FILLABLE_FORM));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::Now());
+    autofill_manager_->FormSubmitted(form, TimeTicks::Now());
   }
 
   // Fill in two of the fields.
@@ -1141,7 +1190,7 @@ TEST_F(AutofillMetricsTest, UserHappinessFormLoadAndSubmission) {
     EXPECT_CALL(*autofill_manager_->metric_logger(),
                 LogUserHappinessMetric(
                     AutofillMetrics::SUBMITTED_NON_FILLABLE_FORM));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::Now());
+    autofill_manager_->FormSubmitted(form, TimeTicks::Now());
   }
 
   // Fill in the third field.
@@ -1153,7 +1202,7 @@ TEST_F(AutofillMetricsTest, UserHappinessFormLoadAndSubmission) {
     EXPECT_CALL(*autofill_manager_->metric_logger(),
                 LogUserHappinessMetric(
                     AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_NONE));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::Now());
+    autofill_manager_->FormSubmitted(form, TimeTicks::Now());
   }
 
 
@@ -1166,7 +1215,7 @@ TEST_F(AutofillMetricsTest, UserHappinessFormLoadAndSubmission) {
     EXPECT_CALL(*autofill_manager_->metric_logger(),
                 LogUserHappinessMetric(
                     AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_SOME));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::Now());
+    autofill_manager_->FormSubmitted(form, TimeTicks::Now());
   }
 
   // Mark all of the fillable fields as autofilled.
@@ -1179,7 +1228,7 @@ TEST_F(AutofillMetricsTest, UserHappinessFormLoadAndSubmission) {
     EXPECT_CALL(*autofill_manager_->metric_logger(),
                 LogUserHappinessMetric(
                     AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_ALL));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::Now());
+    autofill_manager_->FormSubmitted(form, TimeTicks::Now());
   }
 
   // Clear out the third field's value.
@@ -1191,7 +1240,7 @@ TEST_F(AutofillMetricsTest, UserHappinessFormLoadAndSubmission) {
     EXPECT_CALL(*autofill_manager_->metric_logger(),
                 LogUserHappinessMetric(
                     AutofillMetrics::SUBMITTED_NON_FILLABLE_FORM));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::Now());
+    autofill_manager_->FormSubmitted(form, TimeTicks::Now());
   }
 }
 
@@ -1344,7 +1393,7 @@ TEST_F(AutofillMetricsTest, FormFillDuration) {
     EXPECT_CALL(*autofill_manager_->metric_logger(),
                 LogFormFillDurationFromInteractionWithoutAutofill(_)).Times(0);
     autofill_manager_->OnFormsSeen(forms, TimeTicks::FromInternalValue(1));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::FromInternalValue(17));
+    autofill_manager_->FormSubmitted(form, TimeTicks::FromInternalValue(17));
     autofill_manager_->Reset();
     Mock::VerifyAndClearExpectations(autofill_manager_->metric_logger());
   }
@@ -1364,7 +1413,7 @@ TEST_F(AutofillMetricsTest, FormFillDuration) {
     autofill_manager_->OnFormsSeen(forms, TimeTicks::FromInternalValue(1));
     autofill_manager_->OnTextFieldDidChange(form, form.fields.front(),
                                             TimeTicks::FromInternalValue(3));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::FromInternalValue(17));
+    autofill_manager_->FormSubmitted(form, TimeTicks::FromInternalValue(17));
     autofill_manager_->Reset();
     Mock::VerifyAndClearExpectations(autofill_manager_->metric_logger());
   }
@@ -1385,7 +1434,7 @@ TEST_F(AutofillMetricsTest, FormFillDuration) {
     autofill_manager_->OnFormsSeen(forms, TimeTicks::FromInternalValue(1));
     autofill_manager_->OnDidFillAutofillFormData(
         TimeTicks::FromInternalValue(5));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::FromInternalValue(17));
+    autofill_manager_->FormSubmitted(form, TimeTicks::FromInternalValue(17));
     autofill_manager_->Reset();
     Mock::VerifyAndClearExpectations(autofill_manager_->metric_logger());
   }
@@ -1409,7 +1458,7 @@ TEST_F(AutofillMetricsTest, FormFillDuration) {
         TimeTicks::FromInternalValue(5));
     autofill_manager_->OnTextFieldDidChange(form, form.fields.front(),
                                             TimeTicks::FromInternalValue(3));
-    autofill_manager_->OnFormSubmitted(form, TimeTicks::FromInternalValue(17));
+    autofill_manager_->FormSubmitted(form, TimeTicks::FromInternalValue(17));
     autofill_manager_->Reset();
     Mock::VerifyAndClearExpectations(autofill_manager_->metric_logger());
   }