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 e1d1c69ec46cf967a3f353a8961db30b4e73d949..428b95fb480256c42f9610d16513c9771a3a1dc2 100644
--- a/chrome/browser/extensions/extension_toolbar_model_unittest.cc
+++ b/chrome/browser/extensions/extension_toolbar_model_unittest.cc
@@ -6,14 +6,17 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.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"
 #include "chrome/browser/extensions/extension_toolbar_model_factory.h"
 #include "chrome/browser/extensions/test_extension_system.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"
@@ -88,6 +91,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:
@@ -117,6 +121,10 @@ class ExtensionToolbarModelTestObserver
     highlight_mode_count_ += is_highlighting ? 1 : -1;
   }
 
+  void ToolbarReorderNecessary(content::WebContents* web_contents) override {
+    ++reorder_count_;
+  }
+
   Browser* GetBrowser() override { return NULL; }
 
   ExtensionToolbarModel* model_;
@@ -126,6 +134,7 @@ 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(
@@ -133,7 +142,8 @@ ExtensionToolbarModelTestObserver::ExtensionToolbarModelTestObserver(
                                     inserted_count_(0u),
                                     removed_count_(0u),
                                     moved_count_(0u),
-                                    highlight_mode_count_(0) {
+                                    highlight_mode_count_(0),
+                                    reorder_count_(0u) {

[Peter Kasting, 2014/10/30 22:05:59]

Nit: None of these lines should use 0u, they should just be 0.  (We only need to
use 0u explicitly in certain EXPECT/ASSERT statements due to how the macro
expansion works.)

[Devlin, 2014/10/31 17:44:09]

Done.

   model_->AddObserver(this);
 }
 
@@ -918,4 +928,171 @@ TEST_F(ExtensionToolbarModelUnitTest,
   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_EQ(-1, toolbar_model()->GetVisibleIconCount());  // -1 = 'all'.
+  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(1, toolbar_model()->GetVisibleIconCount());
+  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(1, 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(2, 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(1, 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(1, 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(1, 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(2, 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(2, 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(0, 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(1, toolbar_model()->GetVisibleIconCountForTab(web_contents));
+  EXPECT_EQ(3u, observer()->reorder_count());
+}
+
 }  // namespace extensions