Close ExtensionPopup on tab switches; add test.

Close ExtensionPopup on tab switches; add test.

Make ExtensionPopup a TabStripModelObserver.
Close the ExtensionPopup's Widget on ActiveTabChanged.
Add an interactive_ui_test for this behavior.

BUG=303479
TEST=Extension popups close on switching tabs via CTRL+TAB, CTRL+1, CTRL+2, etc.
R=sky@chromium.org,jyasskin@chromium.org

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

[msw, 2013-12-20 00:10:12]

Hey Scott and Jeffrey, please take a look; thanks!
Scott: extension_popup.h|cc
Jeffrey: browser_action_interactive_test.cc

[sky, 2013-12-20 00:14:50]

LGTM - but remove the tab_strip_model() checks. I suspect browser() isn't needed
either, but you'll need to verify that.

https://codereview.chromium.org/119633002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/extensions/extension_popup.cc (right):

https://codereview.chromium.org/119633002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/extensions/extension_popup.cc:100: if (browser &&
browser->tab_strip_model())
Can browser really be NULL here? browser->tab_strip_model should never to NULL.

[Jeffrey Yasskin, 2013-12-20 00:30:57]

lgtm

[msw, 2013-12-20 00:46:27]

Done; landing.

https://codereview.chromium.org/119633002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/extensions/extension_popup.cc (right):

https://codereview.chromium.org/119633002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/extensions/extension_popup.cc:100: if (browser &&
browser->tab_strip_model())
On 2013/12/20 00:14:51, sky (OOO until the 6th) wrote:
> Can browser really be NULL here? browser->tab_strip_model should never to
NULL.

Done, I removed both checks. I tracked down that the ExtensionViewViews |host|
should never be constructed with a NULL |browser_| for extension popups, which
are VIEW_TYPE_EXTENSION_POPUP, as per extensions::CreateViewHostForExtension:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...
That's the only instantiation site, and its |browser_| member is never modified.
+CC jamescook for any additional comments or concerns.

[commit-bot: I haz the power, 2013-12-20 00:49:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/119633002/40001

[James Cook, 2013-12-20 01:01:06]

Extension popups should always have a browser_, so that part should be fine.
Thanks for checking.

[commit-bot: I haz the power, 2013-12-20 08:49:11]

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

[commit-bot: I haz the power, 2013-12-20 19:04:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/119633002/40001

[commit-bot: I haz the power, 2013-12-20 20:44:21]

Change committed as 242170