Add metrics to measure time elapsed between form load and form submission with or without Autofill.

chrome/browser/autofill/form_structure.cc

Index: chrome/browser/autofill/form_structure.cc
diff --git a/chrome/browser/autofill/form_structure.cc b/chrome/browser/autofill/form_structure.cc
index 97e706d114cf7601e5a7c7ed71ea8280c7353ee5..e7d5b28688a4c53a2ce009a41f30ff025656a86a 100644
--- a/chrome/browser/autofill/form_structure.cc
+++ b/chrome/browser/autofill/form_structure.cc
@@ -12,6 +12,7 @@
 #include "base/stringprintf.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
+#include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autofill_metrics.h"
 #include "chrome/browser/autofill/autofill_type.h"
@@ -646,7 +647,9 @@ void FormStructure::UpdateFromCache(const FormStructure& cached_form) {
 }
 
 void FormStructure::LogQualityMetrics(
-    const AutofillMetrics& metric_logger) const {
+    const AutofillMetrics& metric_logger,
+    const base::TimeTicks& interaction_time,
+    const base::TimeTicks& submission_time) const {
   std::string experiment_id = server_experiment_id();
   metric_logger.LogServerExperimentIdForUpload(experiment_id);
 
@@ -778,15 +781,27 @@ void FormStructure::LogQualityMetrics(
   if (num_detected_field_types < kRequiredFillableFields) {
     metric_logger.LogUserHappinessMetric(
         AutofillMetrics::SUBMITTED_NON_FILLABLE_FORM);
-  } else if (did_autofill_all_possible_fields) {
-    metric_logger.LogUserHappinessMetric(
-        AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_ALL);
-  } else if (did_autofill_some_possible_fields) {
-    metric_logger.LogUserHappinessMetric(
-        AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_SOME);
   } else {
-    metric_logger.LogUserHappinessMetric(
-        AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_NONE);
+    if (did_autofill_all_possible_fields) {
+      metric_logger.LogUserHappinessMetric(
+          AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_ALL);
+    } else if (did_autofill_some_possible_fields) {
+      metric_logger.LogUserHappinessMetric(
+          AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_SOME);
+    } else {
+      metric_logger.LogUserHappinessMetric(
+          AutofillMetrics::SUBMITTED_FILLABLE_FORM_AUTOFILLED_NONE);
+    }
+
+    // Only log timing metrics if both timestamps are set and seem plausible.
+    if (!interaction_time.is_null() && !submission_time.is_null() &&

[dhollowa, 2011/08/31 22:40:52]

Could you explain why this is needed.  It seems like this could mask bugs.

[Ilya Sherman, 2011/08/31 23:21:32]

Done.

[dhollowa, 2011/08/31 23:42:20]

Sorry, my thinking was to let the bad data propagate through to the metrics so
we could actually detect bugs in the data.

+        submission_time > interaction_time) {
+      base::TimeDelta time_delta = submission_time - interaction_time;
+      if (did_autofill_some_possible_fields)
+        metric_logger.LogAutofilledFormSubmittedAfterDuration(time_delta);
+      else
+        metric_logger.LogNonAutofilledFormSubmittedAfterDuration(time_delta);
+    }
   }
 }