Move the UI related code from AutofillExternalDelegate to AutofillManagerDelegate

Move the UI related code from AutofillExternalDelegate to AutofillManagerDelegate

Move the UI related code (autofill popup controlling) to AutofillManagerDelegate 
(implementated by TabAutofillManagerDelegate), which is a better place for UI
dependencies, and many other UI dependencies are already there.

Note: work in progress. Will modify tests

BUG=140037
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=184881

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

Thanks!  Looking good overall; just some nits, and waiting to see the test
changes.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
File chrome/browser/autofill/autofill_external_delegate.cc (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.cc:9: #include
"chrome/browser/ui/autofill/autofill_popup_controller_impl.h"
nit: Can this be removed?

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.cc:18: #include
"content/public/browser/web_contents_view.h"
nit: Is this still needed?

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.cc:247: void
AutofillExternalDelegate::HideAutofillPopup() {
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.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
File chrome/browser/autofill/autofill_external_delegate.h (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.h:24: class
AutofillPopupControllerImpl;
nit: Can this be removed?

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.h:105: virtual void
HideAutofillPopup();
nit: If you do keep this method, please reduce its visibility.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.h:166: // The current
bounding of Autofill popup.
nit: These bounds are for the form field that the user is interacting with, not
the Autofill popup.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.h:167: gfx::RectF
autofill_popup_bounding_;
nit: Please name this "element_bounds_".

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
File chrome/browser/autofill/autofill_manager_delegate.h (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_manager_delegate.h:116: // |identifiers| for
the element at |element_bounds|.
nit: Document |delegate| as well.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_manager_delegate.h:124: // Hide the Autofill
popup.
nit: Something more like "Hide the Autofill popup, if one is currently showing."

https://codereview.chromium.org/12340065/diff/1/chrome/browser/ui/autofill/ta...
File chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/ui/autofill/ta...
chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc:41:
autofill_popup_controller_->Hide();
Optional nit: Might be slightly cleaner to call HideAutofillPopup().

https://codereview.chromium.org/12340065/diff/1/chrome/browser/ui/autofill/ta...
File chrome/browser/ui/autofill/tab_autofill_manager_delegate.h (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/ui/autofill/ta...
chrome/browser/ui/autofill/tab_autofill_manager_delegate.h:80:
base::WeakPtr<AutofillPopupControllerImpl> autofill_popup_controller_;
Optional nit: I'd omit the "autofill_" prefix here, i.e. just name this variable
"popup_controller".

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

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
File chrome/browser/autofill/autofill_external_delegate.cc (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.cc:9: #include
"chrome/browser/ui/autofill/autofill_popup_controller_impl.h"
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> nit: Can this be removed?

Done.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.cc:18: #include
"content/public/browser/web_contents_view.h"
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> nit: Is this still needed?

Done.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
File chrome/browser/autofill/autofill_external_delegate.h (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.h:24: class
AutofillPopupControllerImpl;
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> nit: Can this be removed?

Done.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.h:166: // The current
bounding of Autofill popup.
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> nit: These bounds are for the form field that the user is interacting with,
not
> the Autofill popup.

Done.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.h:167: gfx::RectF
autofill_popup_bounding_;
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> nit: Please name this "element_bounds_".

Done.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
File chrome/browser/autofill/autofill_manager_delegate.h (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_manager_delegate.h:116: // |identifiers| for
the element at |element_bounds|.
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> nit: Document |delegate| as well.

Done.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_manager_delegate.h:124: // Hide the Autofill
popup.
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> nit: Something more like "Hide the Autofill popup, if one is currently
showing."

Done.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/ui/autofill/ta...
File chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/ui/autofill/ta...
chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc:41:
autofill_popup_controller_->Hide();
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> Optional nit: Might be slightly cleaner to call HideAutofillPopup().

Done.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/ui/autofill/ta...
File chrome/browser/ui/autofill/tab_autofill_manager_delegate.h (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/ui/autofill/ta...
chrome/browser/ui/autofill/tab_autofill_manager_delegate.h:80:
base::WeakPtr<AutofillPopupControllerImpl> autofill_popup_controller_;
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> Optional nit: I'd omit the "autofill_" prefix here, i.e. just name this
variable
> "popup_controller".

Done.

[Ilya Sherman, 2013-02-26 09:24:57]

LGTM, thanks.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
File chrome/browser/autofill/autofill_external_delegate.h (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.h:105: virtual void
HideAutofillPopup();
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> nit: If you do keep this method, please reduce its visibility.

^^^

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/au...
File chrome/browser/autofill/autofill_external_delegate_unittest.cc (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/au...
chrome/browser/autofill/autofill_external_delegate_unittest.cc:43: };
nit: "private: DISALLOW_COPY_AND_ASSIGN(...)"

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/au...
chrome/browser/autofill/autofill_external_delegate_unittest.cc:57: };
nit: "private: DISALLOW_COPY_AND_ASSIGN(...)"

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/au...
chrome/browser/autofill/autofill_external_delegate_unittest.cc:87: // execute it
with the given QueryId.
nit: Please update this comment.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
File chrome/browser/autofill/test_autofill_external_delegate.cc (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
chrome/browser/autofill/test_autofill_external_delegate.cc:51: */
Here too, delete the code rather than commenting it out.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
File chrome/browser/autofill/test_autofill_external_delegate.h (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
chrome/browser/autofill/test_autofill_external_delegate.h:42: */
Please delete the code rather than commenting it out.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
File chrome/browser/autofill/test_autofill_manager_delegate.cc (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
chrome/browser/autofill/test_autofill_manager_delegate.cc:62:
AutofillPopupDelegate* delegate) {}
nit: Alignment

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
File chrome/browser/autofill/test_autofill_manager_delegate.h (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
chrome/browser/autofill/test_autofill_manager_delegate.h:15: class
TestAutofillManagerDelegate : public AutofillManagerDelegate {
nit: IMO this is more of a "MockAutofillManagerDelegate", since all it does is
define the pure virtual functions with the minimal empty bodies; it doesn't
actually wire anything up to make methods like GetPersonalDataManager() work
nicely in tests.

[kaiwang, 2013-02-26 22:18:39]

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
File chrome/browser/autofill/autofill_external_delegate.cc (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.cc:247: void
AutofillExternalDelegate::HideAutofillPopup() {
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> 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.

Done.

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
File chrome/browser/autofill/autofill_external_delegate.h (right):

https://codereview.chromium.org/12340065/diff/1/chrome/browser/autofill/autof...
chrome/browser/autofill/autofill_external_delegate.h:105: virtual void
HideAutofillPopup();
On 2013/02/26 02:12:15, Ilya Sherman wrote:
> nit: If you do keep this method, please reduce its visibility.

Done.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/au...
File chrome/browser/autofill/autofill_external_delegate_unittest.cc (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/au...
chrome/browser/autofill/autofill_external_delegate_unittest.cc:43: };
On 2013/02/26 09:24:57, Ilya Sherman wrote:
> nit: "private: DISALLOW_COPY_AND_ASSIGN(...)"

Done.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/au...
chrome/browser/autofill/autofill_external_delegate_unittest.cc:57: };
On 2013/02/26 09:24:57, Ilya Sherman wrote:
> nit: "private: DISALLOW_COPY_AND_ASSIGN(...)"

Done.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/au...
chrome/browser/autofill/autofill_external_delegate_unittest.cc:87: // execute it
with the given QueryId.
On 2013/02/26 09:24:57, Ilya Sherman wrote:
> nit: Please update this comment.

Done.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
File chrome/browser/autofill/test_autofill_external_delegate.cc (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
chrome/browser/autofill/test_autofill_external_delegate.cc:51: */
On 2013/02/26 09:24:57, Ilya Sherman wrote:
> Here too, delete the code rather than commenting it out.

Done.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
File chrome/browser/autofill/test_autofill_external_delegate.h (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
chrome/browser/autofill/test_autofill_external_delegate.h:42: */
On 2013/02/26 09:24:57, Ilya Sherman wrote:
> Please delete the code rather than commenting it out.

Done.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
File chrome/browser/autofill/test_autofill_manager_delegate.cc (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
chrome/browser/autofill/test_autofill_manager_delegate.cc:62:
AutofillPopupDelegate* delegate) {}
On 2013/02/26 09:24:57, Ilya Sherman wrote:
> nit: Alignment

Done.

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
File chrome/browser/autofill/test_autofill_manager_delegate.h (right):

https://codereview.chromium.org/12340065/diff/4002/chrome/browser/autofill/te...
chrome/browser/autofill/test_autofill_manager_delegate.h:15: class
TestAutofillManagerDelegate : public AutofillManagerDelegate {
As you can see in this CL, there are many other classes sub-class this one and
mock some methods so we can set expectations.
I'd prefer to call them "Mock". This one is just a useless "Fake". So I followed
"TestAutofillExternalDelegate" to name it.

See: http://en.wikipedia.org/wiki/Mock_object
"Mock objects in this sense do a little more: their method implementations
contain assertions of their own. This means that a true mock, in this sense,
will examine the context of each call— perhaps checking the order in which its
methods are called, perhaps performing tests on the data passed into the method
calls as arguments."

[Ilya Sherman, 2013-02-26 22:55:51]

Still LGTM, just some last few nits:

https://chromiumcodereview.appspot.com/12340065/diff/41/chrome/browser/autofi...
File chrome/browser/autofill/autofill_external_delegate_browsertest.cc (right):

https://chromiumcodereview.appspot.com/12340065/diff/41/chrome/browser/autofi...
chrome/browser/autofill/autofill_external_delegate_browsertest.cc:57: virtual
~TestAutofillManager() {}
nit: Please leave a blank line below this one.

https://chromiumcodereview.appspot.com/12340065/diff/41/chrome/browser/autofi...
chrome/browser/autofill/autofill_external_delegate_browsertest.cc:131:
EXPECT_CALL(manager_delegate_, HideAutofillPopup()).Times(2);
nit: Please include the comment from lines 114-116 here as well.

https://chromiumcodereview.appspot.com/12340065/diff/41/chrome/browser/autofi...
File chrome/browser/autofill/autofill_external_delegate_unittest.cc (right):

https://chromiumcodereview.appspot.com/12340065/diff/41/chrome/browser/autofi...
chrome/browser/autofill/autofill_external_delegate_unittest.cc:43: };
nit: "private: DISALLOW_COPY_AND_ASSIGN(...)"

[Ilya Sherman, 2013-02-26 23:50:50]

Btw, note that you're going to have a merge conflict with [
https://codereview.chromium.org/12302034/ ], depending on which CL lands first.

[kaiwang, 2013-02-27 00:00:10]

https://codereview.chromium.org/12340065/diff/41/chrome/browser/autofill/auto...
File chrome/browser/autofill/autofill_external_delegate_browsertest.cc (right):

https://codereview.chromium.org/12340065/diff/41/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_external_delegate_browsertest.cc:57: virtual
~TestAutofillManager() {}
On 2013/02/26 22:55:51, Ilya Sherman wrote:
> nit: Please leave a blank line below this one.

Done.

https://codereview.chromium.org/12340065/diff/41/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_external_delegate_browsertest.cc:131:
EXPECT_CALL(manager_delegate_, HideAutofillPopup()).Times(2);
On 2013/02/26 22:55:51, Ilya Sherman wrote:
> nit: Please include the comment from lines 114-116 here as well.

Done.

https://codereview.chromium.org/12340065/diff/41/chrome/browser/autofill/auto...
File chrome/browser/autofill/autofill_external_delegate_unittest.cc (right):

https://codereview.chromium.org/12340065/diff/41/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_external_delegate_unittest.cc:43: };
On 2013/02/26 22:55:51, Ilya Sherman wrote:
> nit: "private: DISALLOW_COPY_AND_ASSIGN(...)"

Done.

[commit-bot: I haz the power, 2013-02-27 00:03:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaiwang@chromium.org/12340065/4021

[commit-bot: I haz the power, 2013-02-27 00:03:35]

Failed to apply patch for chrome/browser/autofill/autofill_external_delegate.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/autofill/autofill_external_delegate.cc
  Hunk #7 FAILED at 139.
  1 out of 11 hunks FAILED -- saving rejects to file
chrome/browser/autofill/autofill_external_delegate.cc.rej

Patch:       chrome/browser/autofill/autofill_external_delegate.cc
Index: chrome/browser/autofill/autofill_external_delegate.cc
diff --git a/chrome/browser/autofill/autofill_external_delegate.cc
b/chrome/browser/autofill/autofill_external_delegate.cc
index
507f08e075bf2e927dd1669c7745e6a8ab62da33..49407370a8ddf7d626615720fa6d4355a8761328
100644
--- a/chrome/browser/autofill/autofill_external_delegate.cc
+++ b/chrome/browser/autofill/autofill_external_delegate.cc
@@ -6,7 +6,6 @@
 #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"
 #include "chrome/common/autofill_messages.h"
 #include "chrome/common/chrome_constants.h"
 #include "content/public/browser/navigation_controller.h"
@@ -15,7 +14,6 @@
 #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"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebAutofillClient.h"
@@ -64,10 +62,7 @@ AutofillExternalDelegate::AutofillExternalDelegate(
           &(web_contents->GetController())));
 }
 
-AutofillExternalDelegate::~AutofillExternalDelegate() {
-  if (controller_)
-    controller_->Hide();
-}
+AutofillExternalDelegate::~AutofillExternalDelegate() {}
 
 void AutofillExternalDelegate::OnQuery(int query_id,
                                        const FormData& form,
@@ -78,8 +73,7 @@ void AutofillExternalDelegate::OnQuery(int query_id,
   autofill_query_field_ = field;
   display_warning_if_disabled_ = display_warning_if_disabled;
   autofill_query_id_ = query_id;
-
-  EnsurePopupForElement(element_bounds);
+  element_bounds_ = element_bounds;
 }
 
 void AutofillExternalDelegate::OnSuggestionsReturned(
@@ -88,7 +82,7 @@ void AutofillExternalDelegate::OnSuggestionsReturned(
     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);
@@ -129,13 +123,15 @@ void AutofillExternalDelegate::OnSuggestionsReturned(
 
   if (values.empty()) {
     // No suggestions, any popup currently showing is obsolete.
-    HideAutofillPopup();
+    autofill_manager_->delegate()->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(
+        element_bounds_, values, labels, icons, ids, this);
+  }
 }
 
 void AutofillExternalDelegate::OnShowPasswordSuggestions(
@@ -143,44 +139,18 @@ void AutofillExternalDelegate::OnShowPasswordSuggestions(
     const FormFieldData& field,
     const gfx::RectF& element_bounds) {
   autofill_query_field_ = field;
-  EnsurePopupForElement(element_bounds);
+  element_bounds_ = element_bounds;
 
   if (suggestions.empty()) {
-    HideAutofillPopup();
+    autofill_manager_->delegate()->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(
+      element_bounds_, suggestions, empty, empty, password_ids, this);
 }
 
 void AutofillExternalDelegate::SetCurrentDataListValues(
@@ -247,7 +217,7 @@ void AutofillExternalDelegate::DidAcceptSuggestion(const
string16& value,
     FillAutofillFormData(identifier, false);
   }
 
-  HideAutofillPopup();
+  autofill_manager_->delegate()->HideAutofillPopup();
 }
 
 void AutofillExternalDelegate::RemoveSuggestion(const string16& value,
@@ -261,7 +231,7 @@ void AutofillExternalDelegate::RemoveSuggestion(const
string16& value,
 }
 
 void AutofillExternalDelegate::DidEndTextFieldEditing() {
-  HideAutofillPopup();
+  autofill_manager_->delegate()->HideAutofillPopup();
 
   has_shown_autofill_popup_for_current_edit_ = false;
 }
@@ -272,13 +242,8 @@ void AutofillExternalDelegate::ClearPreviewedForm() {
     host->Send(new AutofillMsg_ClearPreviewedForm(host->GetRoutingID()));
 }
 
-void AutofillExternalDelegate::HideAutofillPopup() {
-  if (controller_)
-    controller_->Hide();
-}
-
 void AutofillExternalDelegate::Reset() {
-  HideAutofillPopup();
+  autofill_manager_->delegate()->HideAutofillPopup();
 
   password_autofill_manager_.Reset();
 }
@@ -409,9 +374,9 @@ void AutofillExternalDelegate::Observe(
     const content::NotificationDetails& details) {
   if (type == content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED) {
     if (!*content::Details<bool>(details).ptr())
-      HideAutofillPopup();
+      autofill_manager_->delegate()->HideAutofillPopup();
   } else if (type == content::NOTIFICATION_NAV_ENTRY_COMMITTED) {
-    HideAutofillPopup();
+    autofill_manager_->delegate()->HideAutofillPopup();
   } else {
     NOTREACHED();
   }

[commit-bot: I haz the power, 2013-02-27 00:20:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaiwang@chromium.org/12340065/14002

[commit-bot: I haz the power, 2013-02-27 02:50:48]

Retried try job too often on mac_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-02-27 02:53:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaiwang@chromium.org/12340065/14003

[commit-bot: I haz the power, 2013-02-27 05:18:10]

Change committed as 184881