Merge 210229 "Hide autofill popup before WebContentsImpl is dest..."

> Hide autofill popup before WebContentsImpl is destroyed.
> 
> TabAutofillManagerDelegate was initiating the autofill popup hiding process in
> its destructor. However, this process ends up calling back into the
> WebContentsImpl instance via a virtual method call on the base WebContents
> class. As TabAutofillManagerDelegate is a WebContentsUserData and thus has its
> destructor called as part of the WebContents base class destructor, the call
> into WebContents is occurring when the instance's vtable is that of
> WebContents, where the method is pure virtual.
> 
> This CL changes the autofill popup hiding process to occur on receiving the
> WebContentsDestroyed notification, while the vtable of the WebContents instance
> is still that of WebContentsImpl.
> 
> BUG=254490
> TEST=On Windows, cause an autofill popup to appear and close the tab. The app
> should not crash.
> 
> Review URL: https://chromiumcodereview.appspot.com/18272015

TBR=blundell@chromium.org

Review URL: https://codereview.chromium.org/19044005

git-svn-id: svn://svn.chromium.org/chrome/branches/1547/src@211373 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 16 insertions, 2 deletions

diff --git a/chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc b/chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc
index c66528c..c6e5637 100644
--- a/chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc
+++ b/chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc
@@ -38,7 +38,11 @@ TabAutofillManagerDelegate::TabAutofillManagerDelegate(
 }
 
 TabAutofillManagerDelegate::~TabAutofillManagerDelegate() {
-  HideAutofillPopup();
+  // NOTE: It is too late to clean up the autofill popup; that cleanup process
+  // requires that the WebContents instance still be valid and it is not at
+  // this point (in particular, the WebContentsImpl destructor has already
+  // finished running and we are now in the base class destructor).
+  DCHECK(!popup_controller_);
 }
 
 PersonalDataManager* TabAutofillManagerDelegate::GetPersonalDataManager() {
@@ -205,4 +209,9 @@ void TabAutofillManagerDelegate::DidNavigateMainFrame(
   HideAutocheckoutBubble();
 }
 
+void TabAutofillManagerDelegate::WebContentsDestroyed(
+    content::WebContents* web_contents) {
+  HideAutofillPopup();
+}
+
 }  // namespace autofill
diff --git a/chrome/browser/ui/autofill/tab_autofill_manager_delegate.h b/chrome/browser/ui/autofill/tab_autofill_manager_delegate.h
index bb118bc..564f0f8 100644
--- a/chrome/browser/ui/autofill/tab_autofill_manager_delegate.h
+++ b/chrome/browser/ui/autofill/tab_autofill_manager_delegate.h
@@ -80,6 +80,8 @@ class TabAutofillManagerDelegate
   virtual void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) OVERRIDE;
+  virtual void WebContentsDestroyed(
+      content::WebContents* web_contents) OVERRIDE;
 
   // Exposed for testing.
   AutofillDialogControllerImpl* GetDialogControllerForTesting() {
diff --git a/components/autofill/core/browser/autofill_external_delegate.cc b/components/autofill/core/browser/autofill_external_delegate.cc
index 29568c2..cf576ef 100644
--- a/components/autofill/core/browser/autofill_external_delegate.cc
+++ b/components/autofill/core/browser/autofill_external_delegate.cc
@@ -182,8 +182,11 @@ void AutofillExternalDelegate::OnPopupShown(
 
 void AutofillExternalDelegate::OnPopupHidden(
     content::KeyboardListener* listener) {
-  if (registered_keyboard_listener_with_ == web_contents_->GetRenderViewHost())
+  if ((!web_contents_->IsBeingDestroyed()) &&
+      (registered_keyboard_listener_with_ ==
+          web_contents_->GetRenderViewHost())) {
     web_contents_->GetRenderViewHost()->RemoveKeyboardListener(listener);
+  }
 
   registered_keyboard_listener_with_ = NULL;
 }