Send IPC from renderer to browser on each Autocheckout page completion

components/autofill/content/browser/autocheckout_manager.cc

Index: components/autofill/content/browser/autocheckout_manager.cc
diff --git a/components/autofill/content/browser/autocheckout_manager.cc b/components/autofill/content/browser/autocheckout_manager.cc
index b06f41fc0f2b387be3d14c94588d457c172a3a96..00834ad152e2385743ea7656cb5bd5a5ba07e013 100644
--- a/components/autofill/content/browser/autocheckout_manager.cc
+++ b/components/autofill/content/browser/autocheckout_manager.cc
@@ -201,89 +201,76 @@ void AutocheckoutManager::FillForms() {
   RecordTimeTaken(page_meta_data_->current_page_number);
 }
 
-void AutocheckoutManager::OnClickFailed(AutocheckoutStatus status) {
-  // |in_autocheckout_flow_| get reset in |OnLoadedPageMetaData| for the last
-  // page, so when click failed on the last page, the value is already 'false'.
-  // This check stops crashing, a better solution should be sending an IPC
-  // message to browser when the renderer completes a step.
-  DCHECK(page_meta_data_->IsEndOfAutofillableFlow() || in_autocheckout_flow_);
+void AutocheckoutManager::OnAutocheckoutPageCompleted(
+    AutocheckoutStatus status) {
+  DVLOG(2) << "OnAutocheckoutPageCompleted, page_no: "
+           << page_meta_data_->current_page_number
+           << " status: "
+           << status;
+  if (!in_autocheckout_flow_)
+    return;
+
   DCHECK_NE(MISSING_FIELDMAPPING, status);
 
-  SendAutocheckoutStatus(status);
-  SetStepProgressForPage(page_meta_data_->current_page_number,
-                         AUTOCHECKOUT_STEP_FAILED);
+  SetStepProgressForPage(
+      page_meta_data_->current_page_number,
+      (status == SUCCESS) ? AUTOCHECKOUT_STEP_COMPLETED :
+          AUTOCHECKOUT_STEP_FAILED);
 
-  autofill_manager_->delegate()->OnAutocheckoutError();
-  in_autocheckout_flow_ = false;
+  if (page_meta_data_->IsEndOfAutofillableFlow() || status != SUCCESS)
+    EndAutocheckout(status);
 }
 
 void AutocheckoutManager::OnLoadedPageMetaData(
     scoped_ptr<AutocheckoutPageMetaData> page_meta_data) {
-  scoped_ptr<AutocheckoutPageMetaData> old_meta_data =
-      page_meta_data_.Pass();
-  page_meta_data_ = page_meta_data.Pass();
-
   // Don't log that the bubble could be displayed if the user entered an
   // Autocheckout flow and sees the first page of the flow again due to an
   // error.
-  if (IsStartOfAutofillableFlow() && !in_autocheckout_flow_) {
+  if (page_meta_data.get() && page_meta_data->IsStartOfAutofillableFlow() &&
+      !in_autocheckout_flow_) {
     metric_logger_->LogAutocheckoutBubbleMetric(
         AutofillMetrics::BUBBLE_COULD_BE_DISPLAYED);
   }
 
   // On the first page of an Autocheckout flow, when this function is called the
   // user won't have opted into the flow yet.
-  if (!in_autocheckout_flow_)
+  if (!in_autocheckout_flow_) {
+    page_meta_data_ = page_meta_data.Pass();
     return;
+  }
 
   AutocheckoutStatus status = SUCCESS;
 
   // Missing Autofill server results.
-  if (!page_meta_data_) {
-    in_autocheckout_flow_ = false;
+  if (!page_meta_data.get()) {
     status = MISSING_FIELDMAPPING;
-  } else if (page_meta_data_->IsStartOfAutofillableFlow()) {
+  } else if (page_meta_data->IsStartOfAutofillableFlow()) {
     // Not possible unless Autocheckout failed to proceed.
-    in_autocheckout_flow_ = false;
     status = CANNOT_PROCEED;
-  } else if (!page_meta_data_->IsInAutofillableFlow()) {
+  } else if (!page_meta_data->IsInAutofillableFlow()) {
     // Missing Autocheckout meta data in the Autofill server results.
-    in_autocheckout_flow_ = false;
     status = MISSING_FIELDMAPPING;
-  } else if (page_meta_data_->current_page_number <=
-                 old_meta_data->current_page_number) {
+  } else if (page_meta_data->current_page_number <=
+      page_meta_data_->current_page_number) {

[Raman Kakilate, 2013/07/15 15:33:16]

nit: this line is confusing to read, earlier "old_" was easier.

[benquan, 2013/07/15 19:19:13]

put old_ back

     // Not possible unless Autocheckout failed to proceed.
-    in_autocheckout_flow_ = false;
     status = CANNOT_PROCEED;
   }
 
   // Encountered an error during the Autocheckout flow, probably to
   // do with a problem on the previous page.
-  if (!in_autocheckout_flow_) {
-    if (old_meta_data) {
-      SetStepProgressForPage(old_meta_data->current_page_number,
-                             AUTOCHECKOUT_STEP_FAILED);
-    }
-    SendAutocheckoutStatus(status);
-    autofill_manager_->delegate()->OnAutocheckoutError();
+  if (status != SUCCESS) {
+    SetStepProgressForPage(page_meta_data_->current_page_number,
+                           AUTOCHECKOUT_STEP_FAILED);
+    EndAutocheckout(status);
+    page_meta_data_ = page_meta_data.Pass();

[Raman Kakilate, 2013/07/15 15:33:16]

shouldn't we be resetting page_meta_data_ here?

[benquan, 2013/07/15 19:19:13]

page_meta_data_ should refer to the page_metadata for the page.

     return;
   }
+  page_meta_data_ = page_meta_data.Pass();
 
-  SetStepProgressForPage(old_meta_data->current_page_number,
-                         AUTOCHECKOUT_STEP_COMPLETED);
   SetStepProgressForPage(page_meta_data_->current_page_number,
                          AUTOCHECKOUT_STEP_STARTED);
 
   FillForms();
-  // If the current page is the last page in the flow, set in-progress
-  // steps to 'completed', and send status.
-  if (page_meta_data_->IsEndOfAutofillableFlow()) {
-    SetStepProgressForPage(page_meta_data_->current_page_number,
-                           AUTOCHECKOUT_STEP_COMPLETED);
-    SendAutocheckoutStatus(status);
-    autofill_manager_->delegate()->OnAutocheckoutSuccess();
-    in_autocheckout_flow_ = false;
-  }
 }
 
 void AutocheckoutManager::OnFormsSeen() {
@@ -377,16 +364,6 @@ void AutocheckoutManager::ReturnAutocheckoutData(
                          AUTOCHECKOUT_STEP_STARTED);
 
   FillForms();
-
-  // If the current page is the last page in the flow, set in-progress
-  // steps to 'completed', and send status.
-  if (page_meta_data_->IsEndOfAutofillableFlow()) {
-    SetStepProgressForPage(page_meta_data_->current_page_number,
-                           AUTOCHECKOUT_STEP_COMPLETED);
-    SendAutocheckoutStatus(SUCCESS);
-    autofill_manager_->delegate()->OnAutocheckoutSuccess();
-    in_autocheckout_flow_ = false;
-  }
 }
 
 void AutocheckoutManager::set_metric_logger(
@@ -548,4 +525,20 @@ void AutocheckoutManager::RecordTimeTaken(int page_number) {
   last_step_completion_timestamp_ = base::TimeTicks().Now();
 }
 
+void AutocheckoutManager::EndAutocheckout(AutocheckoutStatus status) {
+  DCHECK(in_autocheckout_flow_);
+
+  DVLOG(2) << "EndAutocheckout at step: "
+           << page_meta_data_->current_page_number
+           << " with status: "
+           << status;
+
+  SendAutocheckoutStatus(status);
+  if (status == SUCCESS)
+    autofill_manager_->delegate()->OnAutocheckoutSuccess();
+  else
+    autofill_manager_->delegate()->OnAutocheckoutError();
+  in_autocheckout_flow_ = false;
+}
+
 }  // namespace autofill