Send IPC from renderer to browser on each Autocheckout page completion to guide Autocheckout manager to update step status and report Autocheckout status more accurately.

This also removes AutofillHostMsg_ClickFailed which is now covered by the new message.

BUG=260057

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

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

9 files changed, 101 insertions, 84 deletions

diff --git a/components/autofill/content/browser/autocheckout_manager.cc b/components/autofill/content/browser/autocheckout_manager.cc
index b06f41f..4f2455d 100644
--- a/components/autofill/content/browser/autocheckout_manager.cc
+++ b/components/autofill/content/browser/autocheckout_manager.cc
@@ -201,26 +201,29 @@ 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();
+  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
@@ -239,51 +242,33 @@ void AutocheckoutManager::OnLoadedPageMetaData(
   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 (IsStartOfAutofillableFlow()) {
     // Not possible unless Autocheckout failed to proceed.
-    in_autocheckout_flow_ = false;
     status = CANNOT_PROCEED;
   } 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) {
     // 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(old_meta_data->current_page_number,
+                           AUTOCHECKOUT_STEP_FAILED);
+    EndAutocheckout(status);
     return;
   }
 
-  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 +362,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 +523,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
diff --git a/components/autofill/content/browser/autocheckout_manager.h b/components/autofill/content/browser/autocheckout_manager.h
index e283047..9c98172 100644
--- a/components/autofill/content/browser/autocheckout_manager.h
+++ b/components/autofill/content/browser/autocheckout_manager.h
@@ -49,9 +49,10 @@ class AutocheckoutManager {
   // gathered from the requestAutocomplete dialog.
   void FillForms();
 
-  // Called when clicking a proceed element in an Autocheckout flow fails.
-  // |status| is the reason for the failure.
-  void OnClickFailed(AutocheckoutStatus status);
+  // Called to signal that the renderer has completed processing a page in the
+  // Autocheckout flow. |status| is the reason for the failure, or |SUCCESS| if
+  // there were no errors.
+  void OnAutocheckoutPageCompleted(AutocheckoutStatus status);
 
   // Sets |page_meta_data_| with the meta data for the current page.
   void OnLoadedPageMetaData(
@@ -128,6 +129,10 @@ class AutocheckoutManager {
   // towards |page_number|.
   void RecordTimeTaken(int page_number);
 
+  // Terminate the Autocheckout flow and send Autocheckout status to Wallet
+  // server.
+  void EndAutocheckout(AutocheckoutStatus status);
+
   AutofillManager* autofill_manager_;  // WEAK; owns us
 
   // Credit card verification code.
diff --git a/components/autofill/content/browser/autocheckout_manager_unittest.cc b/components/autofill/content/browser/autocheckout_manager_unittest.cc
index 171983a..b7ff8cd 100644
--- a/components/autofill/content/browser/autocheckout_manager_unittest.cc
+++ b/components/autofill/content/browser/autocheckout_manager_unittest.cc
@@ -587,7 +587,7 @@ TEST_F(AutocheckoutManagerTest, OnClickFailedTestMissingAdvance) {
       LogAutocheckoutBuyFlowMetric(
           AutofillMetrics::AUTOCHECKOUT_BUY_FLOW_MISSING_ADVANCE_ELEMENT))
       .Times(1);
-  autocheckout_manager_->OnClickFailed(MISSING_ADVANCE);
+  autocheckout_manager_->OnAutocheckoutPageCompleted(MISSING_ADVANCE);
   EXPECT_FALSE(autocheckout_manager_->in_autocheckout_flow());
   EXPECT_TRUE(
       autofill_manager_delegate_->request_autocomplete_dialog_open());
@@ -605,7 +605,7 @@ TEST_F(AutocheckoutManagerTest, OnClickFailedTestMissingClickBeforeFilling) {
       LogAutocheckoutBuyFlowMetric(AutofillMetrics::
           AUTOCHECKOUT_BUY_FLOW_MISSING_CLICK_ELEMENT_BEFORE_FORM_FILLING))
       .Times(1);
-  autocheckout_manager_->OnClickFailed(
+  autocheckout_manager_->OnAutocheckoutPageCompleted(
       MISSING_CLICK_ELEMENT_BEFORE_FORM_FILLING);
   EXPECT_FALSE(autocheckout_manager_->in_autocheckout_flow());
   EXPECT_TRUE(
@@ -621,7 +621,7 @@ TEST_F(AutocheckoutManagerTest, OnClickFailedTestMissingClickAfterFilling) {
       LogAutocheckoutBuyFlowMetric(AutofillMetrics::
           AUTOCHECKOUT_BUY_FLOW_MISSING_CLICK_ELEMENT_AFTER_FORM_FILLING))
       .Times(1);
-  autocheckout_manager_->OnClickFailed(
+  autocheckout_manager_->OnAutocheckoutPageCompleted(
       MISSING_CLICK_ELEMENT_AFTER_FORM_FILLING);
   EXPECT_FALSE(autocheckout_manager_->in_autocheckout_flow());
   EXPECT_TRUE(
@@ -682,6 +682,7 @@ TEST_F(AutocheckoutManagerTest, OnLoadedPageMetaDataRepeatedStartPage) {
   EXPECT_EQ(0U, process()->sink().message_count());
   EXPECT_TRUE(
       autofill_manager_delegate_->request_autocomplete_dialog_open());
+
   EXPECT_TRUE(autofill_manager_delegate_
       ->AutocheckoutStepExistsWithStatus(AUTOCHECKOUT_STEP_SHIPPING,
                                          AUTOCHECKOUT_STEP_FAILED));
@@ -750,31 +751,42 @@ TEST_F(AutocheckoutManagerTest, FullAutocheckoutFlow) {
   EXPECT_TRUE(autofill_manager_delegate_
       ->AutocheckoutStepExistsWithStatus(AUTOCHECKOUT_STEP_SHIPPING,
                                          AUTOCHECKOUT_STEP_STARTED));
-  // Go to second page.
-  EXPECT_CALL(*autofill_manager_delegate_, OnAutocheckoutSuccess()).Times(1);
-  autocheckout_manager_->OnLoadedPageMetaData(CreateInFlowMetaData());
-  EXPECT_TRUE(autocheckout_manager_->in_autocheckout_flow());
+  // Complete the first page.
+  autocheckout_manager_->OnAutocheckoutPageCompleted(SUCCESS);
   EXPECT_TRUE(autofill_manager_delegate_
       ->AutocheckoutStepExistsWithStatus(AUTOCHECKOUT_STEP_SHIPPING,
                                          AUTOCHECKOUT_STEP_COMPLETED));
+
+  // Go to the second page.
+  EXPECT_CALL(*autofill_manager_delegate_, OnAutocheckoutSuccess()).Times(1);
+  autocheckout_manager_->OnLoadedPageMetaData(CreateInFlowMetaData());
+  EXPECT_TRUE(autocheckout_manager_->in_autocheckout_flow());
   CheckFillFormsAndClickIpc();
   EXPECT_TRUE(autofill_manager_delegate_
       ->AutocheckoutStepExistsWithStatus(AUTOCHECKOUT_STEP_DELIVERY,
                                          AUTOCHECKOUT_STEP_STARTED));
-  // Go to third page.
+  autocheckout_manager_->OnAutocheckoutPageCompleted(SUCCESS);
+  EXPECT_TRUE(autofill_manager_delegate_
+      ->AutocheckoutStepExistsWithStatus(AUTOCHECKOUT_STEP_DELIVERY,
+                                         AUTOCHECKOUT_STEP_COMPLETED));
+
+  // Go to the third page.
   EXPECT_CALL(autocheckout_manager_->metric_logger(),
               LogAutocheckoutBuyFlowMetric(
                   AutofillMetrics::AUTOCHECKOUT_BUY_FLOW_SUCCESS)).Times(1);
   autocheckout_manager_->OnLoadedPageMetaData(CreateEndOfFlowMetaData());
   CheckFillFormsAndClickIpc();
+  EXPECT_TRUE(autocheckout_manager_->in_autocheckout_flow());
   EXPECT_TRUE(autofill_manager_delegate_
-      ->AutocheckoutStepExistsWithStatus(AUTOCHECKOUT_STEP_DELIVERY,
-                                         AUTOCHECKOUT_STEP_COMPLETED));
+      ->AutocheckoutStepExistsWithStatus(AUTOCHECKOUT_STEP_BILLING,
+                                         AUTOCHECKOUT_STEP_STARTED));
+  autocheckout_manager_->OnAutocheckoutPageCompleted(SUCCESS);
   EXPECT_FALSE(autocheckout_manager_->in_autocheckout_flow());
-  EXPECT_TRUE(autofill_manager_delegate_->request_autocomplete_dialog_open());
   EXPECT_TRUE(autofill_manager_delegate_
       ->AutocheckoutStepExistsWithStatus(AUTOCHECKOUT_STEP_BILLING,
                                          AUTOCHECKOUT_STEP_COMPLETED));
+
+  EXPECT_TRUE(autofill_manager_delegate_->request_autocomplete_dialog_open());
 }
 
 TEST_F(AutocheckoutManagerTest, CancelledAutocheckoutFlow) {
@@ -819,6 +831,7 @@ TEST_F(AutocheckoutManagerTest, SinglePageFlow) {
   autocheckout_manager_->MaybeShowAutocheckoutDialog(
       frame_url,
       AUTOCHECKOUT_BUBBLE_ACCEPTED);
+  autocheckout_manager_->OnAutocheckoutPageCompleted(SUCCESS);
   CheckFillFormsAndClickIpc();
   EXPECT_FALSE(autocheckout_manager_->in_autocheckout_flow());
   EXPECT_TRUE(autofill_manager_delegate_->request_autocomplete_dialog_open());
diff --git a/components/autofill/content/browser/autofill_driver_impl.cc b/components/autofill/content/browser/autofill_driver_impl.cc
index d8da58a..d2aab0c 100644
--- a/components/autofill/content/browser/autofill_driver_impl.cc
+++ b/components/autofill/content/browser/autofill_driver_impl.cc
@@ -191,8 +191,9 @@ bool AutofillDriverImpl::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_FORWARD(AutofillHostMsg_RequestAutocomplete,
                         autofill_manager_.get(),
                         AutofillManager::OnRequestAutocomplete)
-    IPC_MESSAGE_FORWARD(AutofillHostMsg_ClickFailed, autofill_manager_.get(),
-                        AutofillManager::OnClickFailed)
+    IPC_MESSAGE_FORWARD(AutofillHostMsg_AutocheckoutPageCompleted,
+                        autofill_manager_.get(),
+                        AutofillManager::OnAutocheckoutPageCompleted)
     IPC_MESSAGE_FORWARD(AutofillHostMsg_MaybeShowAutocheckoutBubble,
                         autofill_manager_.get(),
                         AutofillManager::OnMaybeShowAutocheckoutBubble)
diff --git a/components/autofill/content/renderer/autofill_agent.cc b/components/autofill/content/renderer/autofill_agent.cc
index 0a5f886..5a2fbb0a 100644
--- a/components/autofill/content/renderer/autofill_agent.cc
+++ b/components/autofill/content/renderer/autofill_agent.cc
@@ -14,7 +14,6 @@
 #include "components/autofill/content/renderer/form_autofill_util.h"
 #include "components/autofill/content/renderer/page_click_tracker.h"
 #include "components/autofill/content/renderer/password_autofill_agent.h"
-#include "components/autofill/core/common/autocheckout_status.h"
 #include "components/autofill/core/common/autofill_constants.h"
 #include "components/autofill/core/common/autofill_messages.h"
 #include "components/autofill/core/common/autofill_switches.h"
@@ -248,8 +247,10 @@ void AutofillAgent::DidFailProvisionalLoad(WebFrame* frame,
 void AutofillAgent::DidCommitProvisionalLoad(WebFrame* frame,
                                              bool is_new_navigation) {
   in_flight_request_form_.reset();
-  if (!frame->parent())
+  if (!frame->parent() && autocheckout_click_in_progress_) {
     autocheckout_click_in_progress_ = false;
+    CompleteAutocheckoutPage(SUCCESS);
+  }
 }
 
 void AutofillAgent::FrameDetached(WebFrame* frame) {
@@ -685,8 +686,7 @@ void AutofillAgent::OnFillFormsAndClick(
   for (size_t i = 0; i < click_elements_before_form_fill.size(); ++i) {
     if (!ClickElement(topmost_frame_->document(),
                       click_elements_before_form_fill[i])) {
-      Send(new AutofillHostMsg_ClickFailed(routing_id(),
-           MISSING_CLICK_ELEMENT_BEFORE_FORM_FILLING));
+      CompleteAutocheckoutPage(MISSING_CLICK_ELEMENT_BEFORE_FORM_FILLING);
       return;
     }
   }
@@ -699,15 +699,16 @@ void AutofillAgent::OnFillFormsAndClick(
   for (size_t i = 0; i < click_elements_after_form_fill.size(); ++i) {
     if (!ClickElement(topmost_frame_->document(),
                       click_elements_after_form_fill[i])) {
-      Send(new AutofillHostMsg_ClickFailed(routing_id(),
-           MISSING_CLICK_ELEMENT_AFTER_FORM_FILLING));
+      CompleteAutocheckoutPage(MISSING_CLICK_ELEMENT_AFTER_FORM_FILLING);
       return;
     }
   }
 
   // Exit early if there is nothing to click.
-  if (click_element_descriptor.retrieval_method == WebElementDescriptor::NONE)
+  if (click_element_descriptor.retrieval_method == WebElementDescriptor::NONE) {
+    CompleteAutocheckoutPage(SUCCESS);
     return;
+  }
 
   // It's possible that clicking the element to proceed in an Autocheckout
   // flow will not actually proceed to the next step in the flow, e.g. there
@@ -721,9 +722,7 @@ void AutofillAgent::OnFillFormsAndClick(
                      &AutofillAgent::ClickFailed);
   if (!ClickElement(topmost_frame_->document(),
                     click_element_descriptor)) {
-    click_timer_.Stop();
-    Send(new AutofillHostMsg_ClickFailed(routing_id(),
-                                         MISSING_ADVANCE));
+    CompleteAutocheckoutPage(MISSING_ADVANCE);
   }
 }
 
@@ -734,9 +733,14 @@ void AutofillAgent::OnAutocheckoutSupported() {
   MaybeShowAutocheckoutBubble();
 }
 
+void AutofillAgent::CompleteAutocheckoutPage(
+    autofill::AutocheckoutStatus status) {
+  click_timer_.Stop();
+  Send(new AutofillHostMsg_AutocheckoutPageCompleted(routing_id(), status));
+}
+
 void AutofillAgent::ClickFailed() {
-  Send(new AutofillHostMsg_ClickFailed(routing_id(),
-                                       CANNOT_PROCEED));
+  CompleteAutocheckoutPage(CANNOT_PROCEED);
 }
 
 void AutofillAgent::ShowSuggestions(const WebInputElement& element,
diff --git a/components/autofill/content/renderer/autofill_agent.h b/components/autofill/content/renderer/autofill_agent.h
index 757ff8a..d376139 100644
--- a/components/autofill/content/renderer/autofill_agent.h
+++ b/components/autofill/content/renderer/autofill_agent.h
@@ -15,6 +15,7 @@
 #include "base/timer/timer.h"
 #include "components/autofill/content/renderer/form_cache.h"
 #include "components/autofill/content/renderer/page_click_listener.h"
+#include "components/autofill/core/common/autocheckout_status.h"
 #include "components/autofill/core/common/forms_seen_state.h"
 #include "content/public/renderer/render_view_observer.h"
 #include "third_party/WebKit/public/web/WebAutofillClient.h"
@@ -143,6 +144,9 @@ class AutofillAgent : public content::RenderViewObserver,
   // Called when |topmost_frame_| is supported for Autocheckout.
   void OnAutocheckoutSupported();
 
+  // Called when an Autocheckout page is completed by the renderer.
+  void CompleteAutocheckoutPage(autofill::AutocheckoutStatus status);
+
   // Called when clicking an Autocheckout proceed element fails to do anything.
   void ClickFailed();
 
diff --git a/components/autofill/core/browser/autofill_manager.cc b/components/autofill/core/browser/autofill_manager.cc
index 719b4fe..ef48003 100644
--- a/components/autofill/core/browser/autofill_manager.cc
+++ b/components/autofill/core/browser/autofill_manager.cc
@@ -776,8 +776,9 @@ void AutofillManager::OnDidEndTextFieldEditing() {
   external_delegate_->DidEndTextFieldEditing();
 }
 
-void AutofillManager::OnClickFailed(autofill::AutocheckoutStatus status) {
-  autocheckout_manager_.OnClickFailed(status);
+void AutofillManager::OnAutocheckoutPageCompleted(
+    autofill::AutocheckoutStatus status) {
+  autocheckout_manager_.OnAutocheckoutPageCompleted(status);
 }
 
 std::string AutofillManager::GetAutocheckoutURLPrefix() const {
diff --git a/components/autofill/core/browser/autofill_manager.h b/components/autofill/core/browser/autofill_manager.h
index 409dc0c..934b991 100644
--- a/components/autofill/core/browser/autofill_manager.h
+++ b/components/autofill/core/browser/autofill_manager.h
@@ -169,9 +169,8 @@ class AutofillManager : public AutofillDownloadManager::Observer {
   void OnRequestAutocomplete(const FormData& form,
                              const GURL& frame_url);
 
-  // Called to signal clicking an element failed in some way during an
-  // Autocheckout flow.
-  void OnClickFailed(autofill::AutocheckoutStatus status);
+  // Called to signal a page is completed in renderer in the Autocheckout flow.
+  void OnAutocheckoutPageCompleted(autofill::AutocheckoutStatus status);
 
   // Shows the Autocheckout bubble if conditions are right. See comments for
   // AutocheckoutManager::MaybeShowAutocheckoutBubble. Input element requesting
diff --git a/components/autofill/core/common/autofill_messages.h b/components/autofill/core/common/autofill_messages.h
index 9d3b87d..92bece6 100644
--- a/components/autofill/core/common/autofill_messages.h
+++ b/components/autofill/core/common/autofill_messages.h
@@ -268,10 +268,9 @@ IPC_MESSAGE_ROUTED0(AutofillHostMsg_DidEndTextFieldEditing)
 // Instructs the browser to hide the Autofill UI.
 IPC_MESSAGE_ROUTED0(AutofillHostMsg_HideAutofillUi)
 
-// Sent when the renderer attempts to click an element in an Autocheckout flow
-// and either the element could not be found or the click did not have the
-// desired effect.
-IPC_MESSAGE_ROUTED1(AutofillHostMsg_ClickFailed,
+// Sent when the renderer filled an Autocheckout page and clicked the proceed
+// button or if there was an error.
+IPC_MESSAGE_ROUTED1(AutofillHostMsg_AutocheckoutPageCompleted,
                     autofill::AutocheckoutStatus /* status */)
 
 // Instructs the browser to show the password generation bubble at the