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..5b09fba79a5640f2a3d1c36fdb1e1d01956496b0 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) {
@@ -693,5 +703,169 @@ 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) {
+  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) {
+  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.

[Finnur, 2013/10/17 14:58:34]

Line 806 says the popup opened. Is the test incorrect or does the comment and
name of the test need to be changed?

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

I don't see a reply on this question...

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

Clarified, sorry missed this and added check that it's the popup in the regular
browser that's opened.

+IN_PROC_BROWSER_TEST_F(BrowserActionApiTest,
+                       MAYBE_TestOpenPopupIncognitoNotAllowed) {
+  {
+    // 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) {

[Finnur, 2013/10/17 14:58:34]

I thought the design doc said only the latest popup would stay on screen
(last-to-be-opened)?

[justinlin, 2013/10/17 18:32:53]

Right. Also, this should not overwrite any existing opened popups (i.e. it
should only display if there isn't already a popup showing).

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

That's fine. Does the design doc need to be updated? Or am I mis-remembering?

On 2013/10/17 18:32:53, justinlin wrote:

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

I thought it was. Guess not. I think it might have been discussed in the design
review as mfoltz@ had mentioned this as a constraint. Added to the design doc
"The popup should not preempt any existing popup that was already opened."

+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII(
+      "browser_action/popup")));
+
+  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());

[Finnur, 2013/10/17 14:58:34]

It is not clear from casual reading of this that this ensures the correct one
opened...

[justinlin, 2013/10/17 18:32:53]

Done.

+  }
+
+  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) {
+  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