Make extensions that desire to act pop out if in overflow

chrome/browser/extensions/extension_toolbar_model_unittest.cc

Index: chrome/browser/extensions/extension_toolbar_model_unittest.cc
diff --git a/chrome/browser/extensions/extension_toolbar_model_unittest.cc b/chrome/browser/extensions/extension_toolbar_model_unittest.cc
index 6fbaa48ea68cdae5702570caf04f8492ea39ff19..c7385387ff60d30c44b3c82769320da20ebbc187 100644
--- a/chrome/browser/extensions/extension_toolbar_model_unittest.cc
+++ b/chrome/browser/extensions/extension_toolbar_model_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
+#include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_service_test_base.h"
 #include "chrome/browser/extensions/extension_toolbar_model.h"
@@ -17,8 +18,10 @@
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/common/extensions/api/extension_action/action_info.h"
 #include "components/crx_file/id_util.h"
+#include "content/public/test/web_contents_tester.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
@@ -94,6 +97,7 @@ class ExtensionToolbarModelTestObserver
   size_t removed_count() const { return removed_count_; }
   size_t moved_count() const { return moved_count_; }
   int highlight_mode_count() const { return highlight_mode_count_; }
+  size_t reorder_count() const { return reorder_count_; }
 
  private:
   // ExtensionToolbarModel::Observer:
@@ -123,6 +127,10 @@ class ExtensionToolbarModelTestObserver
     highlight_mode_count_ += is_highlighting ? 1 : -1;
   }
 
+  void OnToolbarReorderNecessary(content::WebContents* web_contents) override {
+    ++reorder_count_;
+  }
+
   Browser* GetBrowser() override { return NULL; }
 
   ExtensionToolbarModel* model_;
@@ -132,14 +140,16 @@ class ExtensionToolbarModelTestObserver
   size_t moved_count_;
   // Int because it could become negative (if something goes wrong).
   int highlight_mode_count_;
+  size_t reorder_count_;
 };
 
 ExtensionToolbarModelTestObserver::ExtensionToolbarModelTestObserver(
     ExtensionToolbarModel* model) : model_(model),
-                                    inserted_count_(0u),
-                                    removed_count_(0u),
-                                    moved_count_(0u),
-                                    highlight_mode_count_(0) {
+                                    inserted_count_(0),
+                                    removed_count_(0),
+                                    moved_count_(0),
+                                    highlight_mode_count_(0),
+                                    reorder_count_(0) {
   model_->AddObserver(this);
 }
 
@@ -695,11 +705,13 @@ TEST_F(ExtensionToolbarModelUnitTest, ExtensionToolbarSizeAfterPrefChange) {
   ASSERT_TRUE(AddBrowserActionExtensions());
   EXPECT_EQ(3u, num_toolbar_items());
 
-  // Should be at max size (-1).
-  EXPECT_EQ(-1, toolbar_model()->GetVisibleIconCount());
+  // Should be at max size.
+  EXPECT_TRUE(toolbar_model()->all_icons_visible());
+  EXPECT_EQ(num_toolbar_items(), toolbar_model()->visible_icon_count());
   toolbar_model()->OnExtensionToolbarPrefChange();
   // Should still be at max size.
-  EXPECT_EQ(-1, toolbar_model()->GetVisibleIconCount());
+  EXPECT_TRUE(toolbar_model()->all_icons_visible());
+  EXPECT_EQ(num_toolbar_items(), toolbar_model()->visible_icon_count());
 }
 
 // Test that, in the absence of the extension-action-redesign switch, the
@@ -812,19 +824,20 @@ TEST_F(ExtensionToolbarModelUnitTest, ExtensionToolbarIncognitoModeTest) {
 
   // Extensions in the overflow menu in the regular toolbar should remain in
   // overflow in the incognito toolbar. So, we should have C [B].
-  EXPECT_EQ(1, incognito_model->GetVisibleIconCount());
+  EXPECT_EQ(1u, incognito_model->visible_icon_count());
   // The regular model should still have two icons visible.
-  EXPECT_EQ(2, toolbar_model()->GetVisibleIconCount());
+  EXPECT_EQ(2u, toolbar_model()->visible_icon_count());
 
   // Changing the incognito model size should not affect the regular model.
   incognito_model->SetVisibleIconCount(0);
-  EXPECT_EQ(0, incognito_model->GetVisibleIconCount());
-  EXPECT_EQ(2, toolbar_model()->GetVisibleIconCount());
+  EXPECT_EQ(0u, incognito_model->visible_icon_count());
+  EXPECT_EQ(2u, toolbar_model()->visible_icon_count());
 
-  // Expanding the incognito model to 2 should register as "all icons" (-1),
+  // Expanding the incognito model to 2 should register as "all icons"
   // since it is all of the incognito-enabled extensions.
   incognito_model->SetVisibleIconCount(2u);
-  EXPECT_EQ(-1, incognito_model->GetVisibleIconCount());
+  EXPECT_EQ(2u, incognito_model->visible_icon_count());
+  EXPECT_TRUE(incognito_model->all_icons_visible());
 
   // Moving icons in the incognito toolbar should not affect the regular
   // toolbar. Incognito currently has C B...
@@ -921,7 +934,7 @@ TEST_F(ExtensionToolbarModelUnitTest,
   // overflowed in the main bar, it shouldn't be visible.
   EXPECT_EQ(1u, incognito_model->toolbar_items().size());
   EXPECT_EQ(extension_b, GetExtensionAtIndex(0u, incognito_model)->id());
-  EXPECT_EQ(0, incognito_model->GetVisibleIconCount());
+  EXPECT_EQ(0u, incognito_model->visible_icon_count());
 
   // Also enable extension a for incognito (again, wait for the reload).
   {
@@ -936,7 +949,7 @@ TEST_F(ExtensionToolbarModelUnitTest,
   EXPECT_EQ(2u, incognito_model->toolbar_items().size());
   EXPECT_EQ(extension_a, GetExtensionAtIndex(0u, incognito_model)->id());
   EXPECT_EQ(extension_b, GetExtensionAtIndex(1u, incognito_model)->id());
-  EXPECT_EQ(1, incognito_model->GetVisibleIconCount());
+  EXPECT_EQ(1u, incognito_model->visible_icon_count());
 }
 
 // Test that hiding actions on the toolbar results in sending them to the
@@ -958,7 +971,7 @@ TEST_F(ExtensionToolbarModelUnitTest,
 
   // Sanity check: Order should start as A B C, with all three visible.
   EXPECT_EQ(3u, num_toolbar_items());
-  EXPECT_EQ(-1, toolbar_model()->GetVisibleIconCount());  // -1 = 'all'.
+  EXPECT_TRUE(toolbar_model()->all_icons_visible());
   EXPECT_EQ(extension_a, GetExtensionAtIndex(0u));
   EXPECT_EQ(extension_b, GetExtensionAtIndex(1u));
   EXPECT_EQ(extension_c, GetExtensionAtIndex(2u));
@@ -979,7 +992,7 @@ TEST_F(ExtensionToolbarModelUnitTest,
 
   // Thus, the order should be A C B, with B in the overflow.
   EXPECT_EQ(3u, num_toolbar_items());
-  EXPECT_EQ(2, toolbar_model()->GetVisibleIconCount());
+  EXPECT_EQ(2u, toolbar_model()->visible_icon_count());
   EXPECT_EQ(extension_a, GetExtensionAtIndex(0u));
   EXPECT_EQ(extension_c, GetExtensionAtIndex(1u));
   EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
@@ -990,7 +1003,7 @@ TEST_F(ExtensionToolbarModelUnitTest,
       prefs, extension_a->id(), false);
   // Thus, the order should be C A B, with A and B in the overflow.
   EXPECT_EQ(3u, num_toolbar_items());
-  EXPECT_EQ(1, toolbar_model()->GetVisibleIconCount());
+  EXPECT_EQ(1u, toolbar_model()->visible_icon_count());
   EXPECT_EQ(extension_c, GetExtensionAtIndex(0u));
   EXPECT_EQ(extension_a, GetExtensionAtIndex(1u));
   EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
@@ -1002,7 +1015,7 @@ TEST_F(ExtensionToolbarModelUnitTest,
       prefs, extension_a->id(), true);
   // So order is C A B, with only B in the overflow.
   EXPECT_EQ(3u, num_toolbar_items());
-  EXPECT_EQ(2, toolbar_model()->GetVisibleIconCount());
+  EXPECT_EQ(2u, toolbar_model()->visible_icon_count());
   EXPECT_EQ(extension_c, GetExtensionAtIndex(0u));
   EXPECT_EQ(extension_a, GetExtensionAtIndex(1u));
   EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
@@ -1011,10 +1024,178 @@ TEST_F(ExtensionToolbarModelUnitTest,
   ExtensionActionAPI::SetBrowserActionVisibility(
       prefs, extension_b->id(), true);
   EXPECT_EQ(3u, num_toolbar_items());
-  EXPECT_EQ(-1, toolbar_model()->GetVisibleIconCount());  // -1 = 'all'
+  EXPECT_TRUE(toolbar_model()->all_icons_visible());
   EXPECT_EQ(extension_c, GetExtensionAtIndex(0u));
   EXPECT_EQ(extension_a, GetExtensionAtIndex(1u));
   EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
 }
 
+// Test that toolbar actions can pop themselves out of overflow if they want
+// to act on a given web contents.
+TEST_F(ExtensionToolbarModelUnitTest, ToolbarActionsPopOutToAct) {
+  // Extensions popping themselves out to act is part of the toolbar redesign,
+  // and hidden behind a flag.
+  FeatureSwitch::ScopedOverride enable_redesign(
+      FeatureSwitch::extension_action_redesign(), true);
+  Init();
+
+  ASSERT_TRUE(AddActionExtensions());
+
+  // We should start in the order of "browser action" "page action" "no action"
+  // and have all extensions visible.
+  EXPECT_EQ(3u, num_toolbar_items());
+  EXPECT_TRUE(toolbar_model()->all_icons_visible());
+  EXPECT_EQ(browser_action(), GetExtensionAtIndex(0u));
+  EXPECT_EQ(page_action(), GetExtensionAtIndex(1u));
+  EXPECT_EQ(no_action(), GetExtensionAtIndex(2u));
+
+  // Shrink the model to only show one action, and move the page action to the
+  // end.
+  toolbar_model()->SetVisibleIconCount(1);
+  toolbar_model()->MoveExtensionIcon(page_action()->id(), 2u);
+
+  // Quickly verify that the move/visible count worked.
+  EXPECT_EQ(1u, toolbar_model()->visible_icon_count());
+  EXPECT_EQ(browser_action(), GetExtensionAtIndex(0u));
+  EXPECT_EQ(no_action(), GetExtensionAtIndex(1u));
+  EXPECT_EQ(page_action(), GetExtensionAtIndex(2u));
+
+  // Create two test web contents, and a session tab helper for each. We need
+  // a session tab helper, since we rely on tab ids.
+  content::WebContents* web_contents =
+      content::WebContentsTester::CreateTestWebContents(profile(), NULL);
+  ASSERT_TRUE(web_contents);
+  SessionTabHelper::CreateForWebContents(web_contents);
+  content::WebContents* second_web_contents =
+      content::WebContentsTester::CreateTestWebContents(profile(), NULL);
+  ASSERT_TRUE(second_web_contents);
+  SessionTabHelper::CreateForWebContents(second_web_contents);
+
+  // Find the tab ids, ensure that the two web contents have different ids, and
+  // verify that neither is -1 (invalid).
+  int tab_id = SessionTabHelper::IdForTab(web_contents);
+  int second_tab_id = SessionTabHelper::IdForTab(second_web_contents);
+  EXPECT_NE(tab_id, second_tab_id);
+  EXPECT_NE(-1, second_tab_id);
+  EXPECT_NE(-1, tab_id);
+
+  // First, check the model order for the first tab. Since we haven't changed
+  // anything (i.e., no extensions want to act), this should be the same as we
+  // left it: "browser action", "no action", "page action", with only one
+  // visible.
+  ExtensionList tab_order = toolbar_model()->GetItemOrderForTab(web_contents);
+  ASSERT_EQ(3u, tab_order.size());
+  EXPECT_EQ(browser_action(), tab_order[0]);
+  EXPECT_EQ(no_action(), tab_order[1]);
+  EXPECT_EQ(page_action(), tab_order[2]);
+  EXPECT_EQ(1u, toolbar_model()->GetVisibleIconCountForTab(web_contents));
+  // And we should have no notifications to reorder the toolbar.
+  EXPECT_EQ(0u, observer()->reorder_count());
+
+  // Make "page action" want to act by making it's page action visible on the
+  // first tab, and notify the API of the change.
+  ExtensionActionManager* action_manager =
+      ExtensionActionManager::Get(profile());
+  ExtensionAction* action = action_manager->GetExtensionAction(*page_action());
+  ASSERT_TRUE(action);
+  action->SetIsVisible(tab_id, true);
+  ExtensionActionAPI* extension_action_api = ExtensionActionAPI::Get(profile());
+  extension_action_api->NotifyChange(action, web_contents, profile());
+
+  // This should result in "page action" being popped out of the overflow menu.
+  // This has two visible effects:
+  // - page action should move to the second index (the one right after the last
+  //   originally-visible).
+  // - The visible count should increase by one (so page action is visible).
+  tab_order = toolbar_model()->GetItemOrderForTab(web_contents);
+  ASSERT_EQ(3u, tab_order.size());
+  EXPECT_EQ(browser_action(), tab_order[0]);
+  EXPECT_EQ(page_action(), tab_order[1]);
+  EXPECT_EQ(no_action(), tab_order[2]);
+  EXPECT_EQ(2u, toolbar_model()->GetVisibleIconCountForTab(web_contents));
+  // We should also have been told to reorder the toolbar.
+  EXPECT_EQ(1u, observer()->reorder_count());
+
+  // This should not have any effect on the second tab, which should still have
+  // the original order and visible count.
+  tab_order = toolbar_model()->GetItemOrderForTab(second_web_contents);
+  ASSERT_EQ(3u, tab_order.size());
+  EXPECT_EQ(browser_action(), tab_order[0]);
+  EXPECT_EQ(no_action(), tab_order[1]);
+  EXPECT_EQ(page_action(), tab_order[2]);
+  EXPECT_EQ(1u,
+            toolbar_model()->GetVisibleIconCountForTab(second_web_contents));
+
+  // Now, set the action to be hidden again, and notify of the change.
+  action->SetIsVisible(tab_id, false);
+  extension_action_api->NotifyChange(action, web_contents, profile());
+  // The order and visible count should return to normal (the page action should
+  // move back to its original index in overflow). So, order should be "browser
+  // action", "no action", "page action".
+  tab_order = toolbar_model()->GetItemOrderForTab(web_contents);
+  ASSERT_EQ(3u, tab_order.size());
+  EXPECT_EQ(browser_action(), tab_order[0]);
+  EXPECT_EQ(no_action(), tab_order[1]);
+  EXPECT_EQ(page_action(), tab_order[2]);
+  EXPECT_EQ(1u, toolbar_model()->GetVisibleIconCountForTab(web_contents));
+  // This should also result in a reorder.
+  EXPECT_EQ(2u, observer()->reorder_count());
+
+  // Move page action to the first index (so it's naturally visible), and make
+  // it want to act.
+  toolbar_model()->MoveExtensionIcon(page_action()->id(), 0u);
+  action->SetIsVisible(tab_id, true);
+  extension_action_api->NotifyChange(action, web_contents, profile());
+  // Since the action is already visible, this should have no effect - the order
+  // and visible count should remain unchanged. Order is "page action", "browser
+  // action", "no action".
+  tab_order = toolbar_model()->GetItemOrderForTab(web_contents);
+  ASSERT_EQ(3u, tab_order.size());
+  EXPECT_EQ(page_action(), tab_order[0]);
+  EXPECT_EQ(browser_action(), tab_order[1]);
+  EXPECT_EQ(no_action(), tab_order[2]);
+  EXPECT_EQ(1u, toolbar_model()->GetVisibleIconCountForTab(web_contents));
+
+  // We should still be able to increase the size of the model, and to move the
+  // page action.
+  toolbar_model()->SetVisibleIconCount(2);
+  toolbar_model()->MoveExtensionIcon(page_action()->id(), 1u);
+  tab_order = toolbar_model()->GetItemOrderForTab(web_contents);
+  ASSERT_EQ(3u, tab_order.size());
+  EXPECT_EQ(browser_action(), tab_order[0]);
+  EXPECT_EQ(page_action(), tab_order[1]);
+  EXPECT_EQ(no_action(), tab_order[2]);
+  EXPECT_EQ(2u, toolbar_model()->GetVisibleIconCountForTab(web_contents));
+
+  // Neither of the above operations should have precipitated a reorder.
+  EXPECT_EQ(2u, observer()->reorder_count());
+
+  // If we moved the page action, the move should remain in effect even after
+  // the action no longer wants to act.
+  action->SetIsVisible(tab_id, false);
+  extension_action_api->NotifyChange(action, web_contents, profile());
+  tab_order = toolbar_model()->GetItemOrderForTab(web_contents);
+  ASSERT_EQ(3u, tab_order.size());
+  EXPECT_EQ(browser_action(), tab_order[0]);
+  EXPECT_EQ(page_action(), tab_order[1]);
+  EXPECT_EQ(no_action(), tab_order[2]);
+  EXPECT_EQ(2u, toolbar_model()->GetVisibleIconCountForTab(web_contents));
+  // The above change should *not* require a reorder, because the extension is
+  // in a new, visible spot and doesn't need to change its position.
+  EXPECT_EQ(2u, observer()->reorder_count());
+
+  // Test the edge case of having no icons visible.
+  toolbar_model()->SetVisibleIconCount(0);
+  EXPECT_EQ(0u, toolbar_model()->GetVisibleIconCountForTab(web_contents));
+  action->SetIsVisible(tab_id, true);
+  extension_action_api->NotifyChange(action, web_contents, profile());
+  tab_order = toolbar_model()->GetItemOrderForTab(web_contents);
+  ASSERT_EQ(3u, tab_order.size());
+  EXPECT_EQ(page_action(), tab_order[0]);
+  EXPECT_EQ(browser_action(), tab_order[1]);
+  EXPECT_EQ(no_action(), tab_order[2]);
+  EXPECT_EQ(1u, toolbar_model()->GetVisibleIconCountForTab(web_contents));
+  EXPECT_EQ(3u, observer()->reorder_count());
+}
+
 }  // namespace extensions