Move the UI related code from AutofillExternalDelegate to AutofillManagerDelegate

chrome/browser/autofill/autofill_external_delegate.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autocomplete_history_manager.h"
 #include "chrome/browser/autofill/autofill_external_delegate.h"
 #include "chrome/browser/autofill/autofill_manager.h"
 #include "chrome/browser/ui/autofill/autofill_popup_controller_impl.h"

[Ilya Sherman, 2013/02/26 02:12:15]

nit: Can this be removed?

[kaiwang, 2013/02/26 05:24:12]

Done.

 #include "chrome/common/autofill_messages.h"
 #include "chrome/common/chrome_constants.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"

[Ilya Sherman, 2013/02/26 02:12:15]

nit: Is this still needed?

[kaiwang, 2013/02/26 05:24:12]

Done.

 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebAutofillClient.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_ANDROID)
 #include "content/public/browser/android/content_view_core.h"
 #endif
 
 using content::RenderViewHost;
@@ skipping 28 matching lines @@
   registrar_.Add(this,
                  content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED,
                  content::Source<content::WebContents>(web_contents));
   registrar_.Add(
       this,
       content::NOTIFICATION_NAV_ENTRY_COMMITTED,
       content::Source<content::NavigationController>(
           &(web_contents->GetController())));
 }
 
-AutofillExternalDelegate::~AutofillExternalDelegate() {
-  if (controller_)
-    controller_->Hide();
-}
+AutofillExternalDelegate::~AutofillExternalDelegate() {}
 
 void AutofillExternalDelegate::OnQuery(int query_id,
                                        const FormData& form,
                                        const FormFieldData& field,
                                        const gfx::RectF& element_bounds,
                                        bool display_warning_if_disabled) {
   autofill_query_form_ = form;
   autofill_query_field_ = field;
   display_warning_if_disabled_ = display_warning_if_disabled;
   autofill_query_id_ = query_id;
-
-  EnsurePopupForElement(element_bounds);
+  autofill_popup_bounding_ = element_bounds;
 }
 
 void AutofillExternalDelegate::OnSuggestionsReturned(
     int query_id,
     const std::vector<string16>& autofill_values,
     const std::vector<string16>& autofill_labels,
     const std::vector<string16>& autofill_icons,
     const std::vector<int>& autofill_unique_ids) {
-  if (query_id != autofill_query_id_ || !controller_)
+  if (query_id != autofill_query_id_)
     return;
 
   std::vector<string16> values(autofill_values);
   std::vector<string16> labels(autofill_labels);
   std::vector<string16> icons(autofill_icons);
   std::vector<int> ids(autofill_unique_ids);
 
   // Add a separator to go between the values and menu items.
   values.push_back(string16());
   labels.push_back(string16());
@@ skipping 25 matching lines @@
 
   InsertDataListValues(&values, &labels, &icons, &ids);
 
   if (values.empty()) {
     // No suggestions, any popup currently showing is obsolete.
     HideAutofillPopup();
     return;
   }
 
   // Send to display.
-  if (autofill_query_field_.is_focusable)
-    ApplyAutofillSuggestions(values, labels, icons, ids);
+  if (autofill_query_field_.is_focusable) {
+    autofill_manager_->delegate()->ShowAutofillPopup(
+        autofill_popup_bounding_, values, labels, icons, ids, this);
+  }
 }
 
 void AutofillExternalDelegate::OnShowPasswordSuggestions(
     const std::vector<string16>& suggestions,
     const FormFieldData& field,
     const gfx::RectF& element_bounds) {
   autofill_query_field_ = field;
-  EnsurePopupForElement(element_bounds);
+  autofill_popup_bounding_ = element_bounds;
 
   if (suggestions.empty()) {
     HideAutofillPopup();
     return;
   }
 
   std::vector<string16> empty(suggestions.size());
   std::vector<int> password_ids(suggestions.size(),
                                 WebAutofillClient::MenuItemIDPasswordEntry);
-  ApplyAutofillSuggestions(suggestions, empty, empty, password_ids);
-}
-
-void AutofillExternalDelegate::EnsurePopupForElement(
-    const gfx::RectF& element_bounds) {
-  // Convert element_bounds to be in screen space.
-  gfx::Rect client_area;
-  web_contents_->GetContainerBounds(&client_area);
-  gfx::RectF element_bounds_in_screen_space =
-      element_bounds + client_area.OffsetFromOrigin();
-
-  // |controller_| owns itself.
-  controller_ = AutofillPopupControllerImpl::GetOrCreate(
-      controller_,
-      this,
-      web_contents()->GetView()->GetContentNativeView(),
-      element_bounds_in_screen_space);
-}
-
-void AutofillExternalDelegate::ApplyAutofillSuggestions(
-    const std::vector<string16>& autofill_values,
-    const std::vector<string16>& autofill_labels,
-    const std::vector<string16>& autofill_icons,
-    const std::vector<int>& autofill_unique_ids) {
-  controller_->Show(autofill_values,
-                    autofill_labels,
-                    autofill_icons,
-                    autofill_unique_ids);
+  autofill_manager_->delegate()->ShowAutofillPopup(
+      autofill_popup_bounding_, suggestions, empty, empty, password_ids, this);
 }
 
 void AutofillExternalDelegate::SetCurrentDataListValues(
     const std::vector<string16>& data_list_values,
     const std::vector<string16>& data_list_labels,
     const std::vector<string16>& data_list_icons,
     const std::vector<int>& data_list_unique_ids) {
   data_list_values_ = data_list_values;
   data_list_labels_ = data_list_labels;
   data_list_icons_ = data_list_icons;
@@ skipping 71 matching lines @@
 
   has_shown_autofill_popup_for_current_edit_ = false;
 }
 
 void AutofillExternalDelegate::ClearPreviewedForm() {
   RenderViewHost* host = web_contents_->GetRenderViewHost();
   if (host)
     host->Send(new AutofillMsg_ClearPreviewedForm(host->GetRoutingID()));
 }
 
 void AutofillExternalDelegate::HideAutofillPopup() {

[Ilya Sherman, 2013/02/26 02:12:15]

Optional nit: Perhaps remove this method entirely, and just call
autofill_manager_->delegate()->HideAutofillPopup() from the current call sites? 
Up to you whether you think that's cleaner or overly verbose to repeat at each
of the call sites.

[kaiwang, 2013/02/26 22:18:40]

Done.

-  if (controller_)
-    controller_->Hide();
+  autofill_manager_->delegate()->HideAutofillPopup();
 }
 
 void AutofillExternalDelegate::Reset() {
   HideAutofillPopup();
 
   password_autofill_manager_.Reset();
 }
 
 void AutofillExternalDelegate::AddPasswordFormMapping(
       const FormFieldData& form,
@@ skipping 121 matching lines @@
     const content::NotificationDetails& details) {
   if (type == content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED) {
     if (!*content::Details<bool>(details).ptr())
       HideAutofillPopup();
   } else if (type == content::NOTIFICATION_NAV_ENTRY_COMMITTED) {
     HideAutofillPopup();
   } else {
     NOTREACHED();
   }
 }