Implement initial chrome.browserAction.openPopup API.

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

Index: chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
diff --git a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
index 06017801b9eabeeadf4879fe04e0e154f337a984..76608446970d9278b7ad029fac81361f85551ff6 100644
--- a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
@@ -17,11 +17,13 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/extensions/permissions/permissions_data.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/notification_service.h"
@@ -90,6 +92,14 @@ class BrowserActionApiTest : public ExtensionApiTest {
     return ExtensionActionManager::Get(browser()->profile())->
         GetBrowserAction(extension);
   }
+
+  bool IsGrantedForTab(const Extension* extension,
+                       const content::WebContents* web_contents) {
+    return PermissionsData::HasAPIPermissionForTab(
+        extension,
+        SessionID::IdForTab(web_contents),
+        APIPermission::kTab);
+  }
 };
 
 IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, Basic) {
@@ -120,7 +130,8 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, Basic) {
 
   ExtensionService* service = extensions::ExtensionSystem::Get(
       browser()->profile())->extension_service();
-  service->toolbar_model()->ExecuteBrowserAction(extension, browser(), NULL);
+  service->toolbar_model()->ExecuteBrowserAction(
+      extension, browser(), NULL, true);
 
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
@@ -590,7 +601,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DISABLED_CloseBackgroundPage) {
 
   // Click the browser action.
   extensions::ExtensionSystem::Get(browser()->profile())->extension_service()->
-      toolbar_model()->ExecuteBrowserAction(extension, browser(), NULL);
+      toolbar_model()->ExecuteBrowserAction(extension, browser(), NULL, true);
 
   // It can take a moment for the background page to actually get destroyed
   // so we wait for the notification before checking that it's really gone
@@ -693,5 +704,178 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, TestTriggerBrowserAction) {
   EXPECT_EQ(result, "red");
 }
 
+
+// http://crbug.com/177163
+#if defined(OS_WIN) && !defined(NDEBUG)
+#define MAYBE_TestOpenPopup DISABLED_TestOpenPopup
+#else
+#define MAYBE_TestOpenPopup TestOpenPopup
+#endif
+IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, MAYBE_TestOpenPopup) {
+  browser()->window()->Show();

[Finnur, 2013/10/18 10:31:01]

I thought this was implicit in the test framework. Why are these call needed
everywhere? Makes me wonder if there's a race condition in there...

[justinlin, 2013/10/19 06:01:48]

OK, thought they might be needed, maybe it's not required, will fix tests..

+  ExtensionTestMessageListener listener("ready", true);
+  {
+    // Show first popup in original window and expect it to have loaded.
+    content::WindowedNotificationObserver frame_observer(
+        content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+        content::NotificationService::AllSources());
+
+    ASSERT_TRUE(RunExtensionSubtest("browser_action/open_popup",
+                                    "open_popup_succeeds.html")) << message_;
+    frame_observer.Wait();
+    EXPECT_TRUE(GetBrowserActionsBar().HasPopup());
+  }
+
+  EXPECT_TRUE(listener.WaitUntilSatisfied());
+
+  {
+    // Open a new window.
+    content::WindowedNotificationObserver frame_observer(
+        content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+        content::NotificationService::AllSources());
+    browser()->OpenURL(content::OpenURLParams(
+        GURL("about:"), content::Referrer(), NEW_WINDOW,
+        content::PAGE_TRANSITION_TYPED, false));
+#if defined(OS_WIN)
+    // Hide all the buttons to test that it opens even when browser action is
+    // in the overflow bucket.
+    // TODO(justinlin): Implement for other platforms.
+    GetBrowserActionsBar().SetIconVisibilityCount(0);
+#endif
+    frame_observer.Wait();
+  }
+
+  ResultCatcher catcher;
+  {
+    content::WindowedNotificationObserver frame_observer(
+        content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+        content::NotificationService::AllSources());
+    // Show second popup in new window.
+    listener.Reply("");
+    frame_observer.Wait();
+    EXPECT_TRUE(GetBrowserActionsBar().HasPopup());
+  }
+  ASSERT_TRUE(catcher.GetNextResult()) << message_;
+}
+
+// http://crbug.com/177163
+#if defined(OS_WIN) && !defined(NDEBUG)
+#define MAYBE_TestOpenPopupIncognito DISABLED_TestOpenPopupIncognito
+#else
+#define MAYBE_TestOpenPopupIncognito TestOpenPopupIncognito
+#endif
+IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, MAYBE_TestOpenPopupIncognito) {
+  browser()->window()->Show();
+  content::WindowedNotificationObserver frame_observer(
+      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+      content::NotificationService::AllSources());
+  ASSERT_TRUE(RunExtensionSubtest("browser_action/open_popup",
+                                  "open_popup_succeeds.html",
+                                  kFlagEnableIncognito | kFlagUseIncognito))
+      << message_;
+  frame_observer.Wait();
+  EXPECT_TRUE(GetBrowserActionsBar().HasPopup());
+}
+
+// http://crbug.com/177163
+#if defined(OS_WIN) && !defined(NDEBUG)
+#define MAYBE_TestOpenPopupIncognitoNotAllowed DISABLED_TestOpenPopupIncognitoNotAllowed
+#else
+#define MAYBE_TestOpenPopupIncognitoNotAllowed TestOpenPopupIncognitoNotAllowed
+#endif
+// Test that the popup does not show in an incognito window if extension is not
+// enabled for incognito.
+IN_PROC_BROWSER_TEST_F(BrowserActionApiTest,
+                       MAYBE_TestOpenPopupIncognitoNotAllowed) {
+  browser()->window()->Show();
+  {
+    // Open an incognito window.
+    content::WindowedNotificationObserver frame_observer(
+        content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+        content::NotificationService::AllSources());
+    browser()->OpenURL(content::OpenURLParams(
+        GURL("about:"), content::Referrer(), OFF_THE_RECORD,
+        content::PAGE_TRANSITION_TYPED, false));
+    frame_observer.Wait();
+  }
+
+  {
+    // Load popup.
+    content::WindowedNotificationObserver frame_observer(
+        content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+        content::NotificationService::AllSources());
+    ASSERT_TRUE(RunExtensionSubtest("browser_action/open_popup",
+                                    "open_popup_succeeds.html")) << message_;
+    frame_observer.Wait();
+    EXPECT_TRUE(GetBrowserActionsBar().HasPopup());
+  }
+}
+
+// http://crbug.com/177163
+#if defined(OS_WIN) && !defined(NDEBUG)
+#define MAYBE_TestOpenPopupDoesNotCloseOtherPopups DISABLED_TestOpenPopupDoesNotCloseOtherPopups
+#else
+#define MAYBE_TestOpenPopupDoesNotCloseOtherPopups TestOpenPopupDoesNotCloseOtherPopups
+#endif
+IN_PROC_BROWSER_TEST_F(BrowserActionApiTest,
+                       MAYBE_TestOpenPopupDoesNotCloseOtherPopups) {
+  browser()->window()->Show();
+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII(
+      "browser_action/popup")));
+  const Extension* extension = GetSingleLoadedExtension();

[Finnur, 2013/10/18 10:31:01]

Maybe ASSERT the extension pointer?

[justinlin, 2013/10/19 06:01:48]

Done. Doesn't really seem needed though, since if that's NULL, the test will
just crash/fail..

[Finnur, 2013/10/21 08:12:12]

If it fails, it means the env is to blame (not your test) and you don't have to
go read all the code below to figure out what happened. I find it helpful to
have these kinds of checks up front because then you know immediately if your
test/code is to blame or if the problem lies in the environment (another good
example: asserting that the web server started up properly).

+
+  ExtensionTestMessageListener listener("ready", true);
+  {
+    // Show first popup in original window and expect it to have loaded.
+    content::WindowedNotificationObserver frame_observer(
+        content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+        content::NotificationService::AllSources());
+
+    ASSERT_TRUE(RunExtensionSubtest("browser_action/open_popup",
+                                    "open_popup_fails.html")) << message_;
+    frame_observer.Wait();
+  }
+
+  EXPECT_TRUE(listener.WaitUntilSatisfied());
+
+  {
+    content::WindowedNotificationObserver frame_observer(
+        content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+        content::NotificationService::AllSources());
+    ASSERT_TRUE(OpenPopup(0));
+    frame_observer.Wait();
+    EXPECT_TRUE(GetBrowserActionsBar().HasPopup());
+    const int tab_id = ExtensionTabUtil::GetTabId(
+        browser()->tab_strip_model()->GetActiveWebContents());
+    ASSERT_TRUE(GetBrowserAction(*extension)->HasPopup(tab_id));
+  }
+
+  ResultCatcher catcher;
+  listener.Reply("");
+  ASSERT_TRUE(catcher.GetNextResult()) << message_;
+}
+
+// http://crbug.com/177163
+#if defined(OS_WIN) && !defined(NDEBUG)
+#define MAYBE_TestOpenPopupDoesNotGrant DISABLED_TestOpenPopupDoesNotGrant
+#else
+#define MAYBE_TestOpenPopupDoesNotGrant TestOpenPopupDoesNotGrant
+#endif
+IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, MAYBE_TestOpenPopupDoesNotGrant) {
+  browser()->window()->Show();
+  content::WindowedNotificationObserver frame_observer(
+      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+      content::NotificationService::AllSources());
+  ASSERT_TRUE(RunExtensionSubtest("browser_action/open_popup",
+                                  "open_popup_succeeds.html")) << message_;
+  frame_observer.Wait();
+
+  ExtensionService* service = extensions::ExtensionSystem::Get(
+      browser()->profile())->extension_service();
+  ASSERT_FALSE(IsGrantedForTab(
+      service->GetExtensionById(last_loaded_extension_id_, false),
+      browser()->tab_strip_model()->GetActiveWebContents()));
+}
+
 }  // namespace
 }  // namespace extensions