Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(205)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/autofill/autofill_metrics_unittest.cc

Issue 8387016: Move some Autofill processing logic on form submit to a background thread. (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Don't rely on any particular order of evaluation for the arguments to PostTaskAndReply Created 9 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/autofill/autofill_manager_unittest.cc ('k') | chrome/browser/ui/tab_contents/tab_contents_wrapper.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/autofill/autofill_metrics_unittest.cc
diff --git a/chrome/browser/autofill/autofill_metrics_unittest.cc b/chrome/browser/autofill/autofill_metrics_unittest.cc
index 26b7af03ea69ed69624764ffedf5d77de2edeb82..fc86dc7a1c8d9df38fd46b0cec3891cc8e354c4e 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());
}
« no previous file with comments | « chrome/browser/autofill/autofill_manager_unittest.cc ('k') | chrome/browser/ui/tab_contents/tab_contents_wrapper.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698