Hide autofill popup before WebContentsImpl is destroyed.

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.

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

[blundell, 2013-07-03 15:03:41]

Hi Ilya,

I'm pretty certain that this is the source of the crash, and that this latent
bug was exposed by my changing the AutofillExternalDelegate instance from being
a WCUD that happened to not outlive the TabAutofillManagerDelegate to being
owned by the AutofillDriverIOS, which is a WCUD that happens to outlive the
TabAutofillManagerDelegate.

[Ilya Sherman, 2013-07-03 19:56:48]

LGTM, thanks!

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

https://codereview.chromium.org/18272015/diff/1/chrome/browser/ui/autofill/ta...
chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc:45:
DCHECK(!popup_controller_.get());
nit: I don't think the ".get()" is needed here.

https://codereview.chromium.org/18272015/diff/1/components/autofill/core/brow...
File components/autofill/core/browser/autofill_external_delegate.cc (right):

https://codereview.chromium.org/18272015/diff/1/components/autofill/core/brow...
components/autofill/core/browser/autofill_external_delegate.cc:174:
web_contents_->GetRenderViewHost()->RemoveKeyboardListener(listener);
nit: Please add curly braces now that the condition for this if-stmt spans two
lines.

[commit-bot: I haz the power, 2013-07-04 08:29:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/18272015/6001

[blundell, 2013-07-04 08:32:45]

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

https://codereview.chromium.org/18272015/diff/1/chrome/browser/ui/autofill/ta...
chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc:45:
DCHECK(!popup_controller_.get());
On 2013/07/03 19:56:49, Ilya Sherman wrote:
> nit: I don't think the ".get()" is needed here.

Done.

https://codereview.chromium.org/18272015/diff/1/components/autofill/core/brow...
File components/autofill/core/browser/autofill_external_delegate.cc (right):

https://codereview.chromium.org/18272015/diff/1/components/autofill/core/brow...
components/autofill/core/browser/autofill_external_delegate.cc:174:
web_contents_->GetRenderViewHost()->RemoveKeyboardListener(listener);
On 2013/07/03 19:56:49, Ilya Sherman wrote:
> nit: Please add curly braces now that the condition for this if-stmt spans two
> lines.

Done.

[commit-bot: I haz the power, 2013-07-04 08:58:52]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-04 09:15:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/18272015/6001

[commit-bot: I haz the power, 2013-07-04 09:24:47]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-04 15:11:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/18272015/6001

[commit-bot: I haz the power, 2013-07-04 15:23:57]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-04 19:45:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/18272015/6001

[commit-bot: I haz the power, 2013-07-04 22:11:50]

Change committed as 210229