WebView autofill implementation

components/autofill/core/browser/autofill_manager.cc

Index: components/autofill/core/browser/autofill_manager.cc
diff --git a/components/autofill/core/browser/autofill_manager.cc b/components/autofill/core/browser/autofill_manager.cc
index 933b7c5dd41a1549fca6c368a1235d2e756ba4c3..fbed176802e9a7656dc5c7877f6463cf5eee4c92 100644
--- a/components/autofill/core/browser/autofill_manager.cc
+++ b/components/autofill/core/browser/autofill_manager.cc
@@ -42,6 +42,7 @@
 #include "components/autofill/core/browser/autofill_manager_test_delegate.h"
 #include "components/autofill/core/browser/autofill_metrics.h"
 #include "components/autofill/core/browser/autofill_profile.h"
+#include "components/autofill/core/browser/autofill_provider.h"
 #include "components/autofill/core/browser/autofill_type.h"
 #include "components/autofill/core/browser/country_names.h"
 #include "components/autofill/core/browser/credit_card.h"
@@ -215,7 +216,8 @@ AutofillManager::AutofillManager(
     AutofillDriver* driver,
     AutofillClient* client,
     const std::string& app_locale,
-    AutofillDownloadManagerState enable_download_manager)
+    AutofillDownloadManagerState enable_download_manager,
+    AutofillProvider* provider)
     : driver_(driver),
       client_(client),
       payments_client_(base::MakeUnique<payments::PaymentsClient>(
@@ -249,6 +251,7 @@ AutofillManager::AutofillManager(
 #if defined(OS_ANDROID) || defined(OS_IOS)
       autofill_assistant_(this),
 #endif
+      provider_(provider),
       weak_ptr_factory_(this) {
   if (enable_download_manager == ENABLE_AUTOFILL_DOWNLOAD_MANAGER) {
     download_manager_.reset(new AutofillDownloadManager(driver, this));
@@ -367,6 +370,9 @@ void AutofillManager::OnFormsSeen(const std::vector<FormData>& forms,
   if (!driver_->RendererIsAvailable())
     return;
 
+  if (provider_)
+    return;
+
   bool enabled = IsAutofillEnabled();
   if (!has_logged_autofill_enabled_) {
     AutofillMetrics::LogIsAutofillEnabledAtPageLoad(enabled);
@@ -388,6 +394,11 @@ bool AutofillManager::OnWillSubmitForm(const FormData& form,
   if (!IsValidFormData(form))
     return false;
 
+  if (provider_) {
+    provider_->OnWillSubmitForm(this, form, timestamp);
+    return true;
+  }
+
   // We will always give Autocomplete a chance to save the data.
   std::unique_ptr<FormStructure> submitted_form = ValidateSubmittedForm(form);
   if (!submitted_form) {
@@ -418,6 +429,9 @@ bool AutofillManager::OnFormSubmitted(const FormData& form) {
   if (!IsValidFormData(form))
     return false;
 
+  if (provider_)
+    return true;
+
   // We will always give Autocomplete a chance to save the data.
   std::unique_ptr<FormStructure> submitted_form = ValidateSubmittedForm(form);
   if (!submitted_form) {
@@ -512,6 +526,11 @@ void AutofillManager::OnTextFieldDidChange(const FormData& form,
   if (!IsValidFormData(form) || !IsValidFormFieldData(field))
     return;
 
+  if (provider_) {
+    provider_->OnTextFieldDidChange(this, form, field, timestamp);

[sebsg, 2017/05/08 15:38:17]

I notice there are a lot of if statements like this. It's difficult for me to
check how much of the original AutofillManager you are using, but you seem to
want to change/stop the behavior in a lot of places. 

Do you think subclassing the AutofillManager would be feasible?

I just feels weird to have all these if and return statements.

[michaelbai, 2017/05/09 00:27:03]

We were planing to cherry-pick this patch to M59, also I have been told your
guys will refactorying those code (well, maybe password manager related code),
so I didn't want to change the current code too much.

I think I can do something like below, but in a following up patch if you want,

Basically, Autofill provider is only care about the interface between
AutofillManager and ContentAutofillDriver, and takes advantage of validation
code, so we can

- extract this interface and common code into an abstract class AutofillManager
- and rename current AutofillManager to AutofillManagerImpl
- Also create a new class like AutofillManagerProxy which subclass
AutofillManager also forward call to AutofillProvider,
- we create different implementation of AutofillManager in ContentAutofillDriver
based on if AutofillProvider is available.

Thought?

[Roger McFarlane (Chromium), 2017/05/10 02:31:54]

That generally seems like a good approach.

But, you're right, that would likely be too large to merge into M59... so this
hooked approach makes sense in the interim. It's just tricky to make sure we
understand which functionality you meant to override vs reuse (i.e., if you
forgot a hook location, it's hard for us as reviewers to notice).

+    return;
+  }
+
   if (test_delegate_)
     test_delegate_->OnTextFieldChanged();
 
@@ -561,6 +580,12 @@ void AutofillManager::OnQueryFormFieldAutofill(int query_id,
   gfx::RectF transformed_box =
       driver_->TransformBoundingBoxToViewportCoordinates(bounding_box);
 
+  if (provider_) {
+    provider_->OnQueryFormFieldAutofill(this, query_id, form, field,
+                                        transformed_box);
+    return;
+  }
+
   external_delegate_->OnQuery(query_id, form, field, transformed_box);
 
   // Need to refresh models before using the form_event_loggers.
@@ -810,6 +835,10 @@ void AutofillManager::FillCreditCardForm(int query_id,
 }
 
 void AutofillManager::OnFocusNoLongerOnForm() {
+  if (provider_) {
+    provider_->OnFocusNoLongerOnForm(this);
+    return;
+  }
   ProcessPendingFormForUpload();
 }
 
@@ -820,6 +849,11 @@ void AutofillManager::OnDidPreviewAutofillFormData() {
 
 void AutofillManager::OnDidFillAutofillFormData(const FormData& form,
                                                 const TimeTicks& timestamp) {
+  if (provider_) {
+    provider_->OnDidFillAutofillFormData(this, form, timestamp);
+    return;
+  }
+
   if (test_delegate_)
     test_delegate_->DidFillFormData();
 
@@ -863,7 +897,7 @@ void AutofillManager::DidShowSuggestions(bool is_new_popup,
 }
 
 void AutofillManager::OnHidePopup() {
-  if (!IsAutofillEnabled())
+  if (!IsAutofillEnabled() || provider_)
     return;
 
   autocomplete_history_manager_->CancelPendingQuery();
@@ -977,6 +1011,9 @@ void AutofillManager::SetTestDelegate(AutofillManagerTestDelegate* delegate) {
 
 void AutofillManager::OnSetDataList(const std::vector<base::string16>& values,
                                     const std::vector<base::string16>& labels) {
+  if (provider_)
+    return;
+
   if (!IsValidString16Vector(values) ||
       !IsValidString16Vector(labels) ||
       values.size() != labels.size())
@@ -1142,6 +1179,8 @@ void AutofillManager::OnDidGetUploadRiskData(const std::string& risk_data) {
 }
 
 void AutofillManager::OnDidEndTextFieldEditing() {
+  if (provider_)
+    return;
   external_delegate_->DidEndTextFieldEditing();
 }
 
@@ -1461,6 +1500,8 @@ void AutofillManager::Reset() {
 #if defined(OS_ANDROID) || defined(OS_IOS)
   autofill_assistant_.Reset();
 #endif
+  if (provider_)
+    provider_->Reset(this);
   has_logged_autofill_enabled_ = false;
   has_logged_address_suggestions_count_ = false;
   did_show_suggestions_ = false;
@@ -1723,6 +1764,13 @@ void AutofillManager::FillOrPreviewDataModelForm(
   driver_->SendFormDataToRenderer(query_id, action, result);
 }
 
+void AutofillManager::SendFormDataToRenderer(
+    int query_id,
+    AutofillDriver::RendererFormDataAction action,
+    const FormData& data) {
+  driver_->SendFormDataToRenderer(query_id, action, data);
+}
+
 std::unique_ptr<FormStructure> AutofillManager::ValidateSubmittedForm(
     const FormData& form) {
   std::unique_ptr<FormStructure> submitted_form(