[Extensions] Fix BrowserActionInteractiveTest.TabSwitchClosesPopup

chrome/browser/extensions/api/extension_action/browser_action_interactive_test.cc

 // Copyright 2013 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 "build/build_config.h"
 #include "chrome/browser/extensions/browser_action_test_util.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ skipping 270 matching lines @@
 }
 
 // Test that the extension popup is closed on browser tab switches.
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, TabSwitchClosesPopup) {
   if (!ShouldRunPopupTest())
     return;
 
   // Add a second tab to the browser and open an extension popup.
   chrome::NewTab(browser());
   ASSERT_EQ(2, browser()->tab_strip_model()->count());
+  EXPECT_EQ(browser()->tab_strip_model()->GetWebContentsAt(1),
+            browser()->tab_strip_model()->GetActiveWebContents());
   OpenExtensionPopupViaAPI();
 
-  // Press CTRL+TAB to change active tabs, the extension popup should close.
-  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(

[Devlin, 2016/03/14 21:21:34]

This strikes me as wrong in case this shortcut is ever not recognized on all
platforms.

-      browser(), ui::VKEY_TAB, true, false, false, false));
+  content::WindowedNotificationObserver observer(
+      extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
+      content::NotificationService::AllSources());
+  // Change active tabs, the extension popup should close.
+  browser()->tab_strip_model()->ActivateTabAt(0, true);
+  observer.Wait();
+

[asargent_no_longer_on_chrome, 2016/03/14 23:11:24]

Is there any easy way to verify that the actual
NOTIFICATION_EXTENSION_HOST_DESTROYED observed was for the one we caused to
popup, and not some other random one that might have been created and destroyed
for an unrelated reason? (Maybe looking at the actual source passed to the
observer?) If this is tricky it's probably not worth worrying about though. 

[Devlin, 2016/03/15 00:07:04]

Not an easy one.  The source is a browser context and the details is an
ExtensionHost*.  The problem is that the ExtensionHost*, being closed, is
probably invalid by the time we're done waiting (and since there's only one
profile set up here, checking the browser context seems unnecessary).

In a perfect world, we'd have better test utils for this, but I don't think
anyone's had the time to do it. :(

   EXPECT_FALSE(BrowserActionTestUtil(browser()).HasPopup());
 }
 
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
                        DeleteBrowserActionWithPopupOpen) {
   if (!ShouldRunPopupTest())
     return;
 
   // First, we open a popup.
   OpenExtensionPopupViaAPI();
@@ skipping 64 matching lines @@
 
   // Forcibly closing the browser HWND should not cause a crash.
   EXPECT_EQ(TRUE, ::CloseWindow(hwnd));
   EXPECT_EQ(TRUE, ::DestroyWindow(hwnd));
   EXPECT_EQ(FALSE, ::IsWindow(hwnd));
 }
 #endif  // OS_WIN
 
 }  // namespace
 }  // namespace extensions