Revert of Make extensions that desire to act pop out if in overflow

chrome/browser/extensions/extension_toolbar_model_unittest.cc

 // Copyright 2014 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 "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #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"
 #include "chrome/browser/extensions/extension_toolbar_model_factory.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/test_extension_dir.h"
 #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"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/value_builder.h"
 
@@ skipping 55 matching lines @@
 class ExtensionToolbarModelTestObserver
     : public ExtensionToolbarModel::Observer {
  public:
   explicit ExtensionToolbarModelTestObserver(ExtensionToolbarModel* model);
   ~ExtensionToolbarModelTestObserver() override;
 
   size_t inserted_count() const { return inserted_count_; }
   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:
   void ToolbarExtensionAdded(const Extension* extension, int index) override {
     ++inserted_count_;
   }
 
   void ToolbarExtensionRemoved(const Extension* extension) override {
     ++removed_count_;
   }
 
   void ToolbarExtensionMoved(const Extension* extension, int index) override {
     ++moved_count_;
   }
 
   void ToolbarExtensionUpdated(const Extension* extension) override {}
 
   bool ShowExtensionActionPopup(const Extension* extension,
                                 bool grant_active_tab) override {
     return false;
   }
 
   void ToolbarVisibleCountChanged() override {}
 
   void ToolbarHighlightModeChanged(bool is_highlighting) override {
     // Add one if highlighting, subtract one if not.
     highlight_mode_count_ += is_highlighting ? 1 : -1;
   }
 
-  void OnToolbarReorderNecessary(content::WebContents* web_contents) override {
-    ++reorder_count_;
-  }
-
   Browser* GetBrowser() override { return NULL; }
 
   ExtensionToolbarModel* model_;
 
   size_t inserted_count_;
   size_t removed_count_;
   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_(0),
-                                    removed_count_(0),
-                                    moved_count_(0),
-                                    highlight_mode_count_(0),
-                                    reorder_count_(0) {
+                                    inserted_count_(0u),
+                                    removed_count_(0u),
+                                    moved_count_(0u),
+                                    highlight_mode_count_(0) {
   model_->AddObserver(this);
 }
 
 ExtensionToolbarModelTestObserver::~ExtensionToolbarModelTestObserver() {
   model_->RemoveObserver(this);
 }
 
 }  // namespace
 
 class ExtensionToolbarModelUnitTest : public ExtensionServiceTestBase {
@@ skipping 535 matching lines @@
 
 // Test that the extension toolbar maintains the proper size, even after a pref
 // change.
 TEST_F(ExtensionToolbarModelUnitTest, ExtensionToolbarSizeAfterPrefChange) {
   Init();
 
   // Add the three browser action extensions.
   ASSERT_TRUE(AddBrowserActionExtensions());
   EXPECT_EQ(3u, num_toolbar_items());
 
-  // Should be at max size.
-  EXPECT_TRUE(toolbar_model()->all_icons_visible());
-  EXPECT_EQ(num_toolbar_items(), toolbar_model()->visible_icon_count());
+  // Should be at max size (-1).
+  EXPECT_EQ(-1, toolbar_model()->GetVisibleIconCount());
   toolbar_model()->OnExtensionToolbarPrefChange();
   // Should still be at max size.
-  EXPECT_TRUE(toolbar_model()->all_icons_visible());
-  EXPECT_EQ(num_toolbar_items(), toolbar_model()->visible_icon_count());
+  EXPECT_EQ(-1, toolbar_model()->GetVisibleIconCount());
 }
 
 // Test that, in the absence of the extension-action-redesign switch, the
 // model only contains extensions with browser actions.
 TEST_F(ExtensionToolbarModelUnitTest, TestToolbarExtensionTypesNoSwitch) {
   Init();
   ASSERT_TRUE(AddActionExtensions());
 
   EXPECT_EQ(1u, num_toolbar_items());
   EXPECT_EQ(browser_action(), GetExtensionAtIndex(0u));
@@ skipping 92 matching lines @@
   EXPECT_EQ(0u, incognito_observer.moved_count());
 
   // We should have two items, C and B, and the order should be preserved from
   // the original model.
   EXPECT_EQ(2u, incognito_model->toolbar_items().size());
   EXPECT_EQ(browser_action_c(), GetExtensionAtIndex(0u, incognito_model));
   EXPECT_EQ(browser_action_b(), GetExtensionAtIndex(1u, incognito_model));
 
   // 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(1u, incognito_model->visible_icon_count());
+  EXPECT_EQ(1, incognito_model->GetVisibleIconCount());
   // The regular model should still have two icons visible.
-  EXPECT_EQ(2u, toolbar_model()->visible_icon_count());
+  EXPECT_EQ(2, toolbar_model()->GetVisibleIconCount());
 
   // Changing the incognito model size should not affect the regular model.
   incognito_model->SetVisibleIconCount(0);
-  EXPECT_EQ(0u, incognito_model->visible_icon_count());
-  EXPECT_EQ(2u, toolbar_model()->visible_icon_count());
+  EXPECT_EQ(0, incognito_model->GetVisibleIconCount());
+  EXPECT_EQ(2, toolbar_model()->GetVisibleIconCount());
 
-  // Expanding the incognito model to 2 should register as "all icons"
+  // Expanding the incognito model to 2 should register as "all icons" (-1),
   // since it is all of the incognito-enabled extensions.
   incognito_model->SetVisibleIconCount(2u);
-  EXPECT_EQ(2u, incognito_model->visible_icon_count());
-  EXPECT_TRUE(incognito_model->all_icons_visible());
+  EXPECT_EQ(-1, incognito_model->GetVisibleIconCount());
 
   // Moving icons in the incognito toolbar should not affect the regular
   // toolbar. Incognito currently has C B...
   incognito_model->MoveExtensionIcon(browser_action_b()->id(), 0u);
   // So now it should be B C...
   EXPECT_EQ(1u, incognito_observer.moved_count());
   EXPECT_EQ(browser_action_b(), GetExtensionAtIndex(0u, incognito_model));
   EXPECT_EQ(browser_action_c(), GetExtensionAtIndex(1u, incognito_model));
   // ... and the regular toolbar should be unaffected.
   EXPECT_EQ(browser_action_a(), GetExtensionAtIndex(0u));
@@ skipping 76 matching lines @@
   {
     TestExtensionRegistryObserver observer(registry(), extension_b);
     util::SetIsIncognitoEnabled(extension_b, profile(), true);
     observer.WaitForExtensionLoaded();
   }
 
   // Now, we should have one icon in the incognito bar. But, since B is
   // 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(0u, incognito_model->visible_icon_count());
+  EXPECT_EQ(0, incognito_model->GetVisibleIconCount());
 
   // Also enable extension a for incognito (again, wait for the reload).
   {
     TestExtensionRegistryObserver observer(registry(), extension_a);
     util::SetIsIncognitoEnabled(extension_a, profile(), true);
     observer.WaitForExtensionLoaded();
   }
 
   // Now, both extensions should be enabled in incognito mode. In addition, the
   // incognito toolbar should have expanded to show extension a (since it isn't
   // overflowed in the main bar).
   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(1u, incognito_model->visible_icon_count());
+  EXPECT_EQ(1, incognito_model->GetVisibleIconCount());
 }
 
 // Test that hiding actions on the toolbar results in sending them to the
 // overflow menu when the redesign switch is enabled.
 TEST_F(ExtensionToolbarModelUnitTest,
        ExtensionToolbarActionsVisibilityWithSwitch) {
   FeatureSwitch::ScopedOverride enable_redesign(
       FeatureSwitch::extension_action_redesign(), true);
   Init();
 
   // We choose to use all types of extensions here, since the misnamed
   // BrowserActionVisibility is now for toolbar visibility.
   ASSERT_TRUE(AddActionExtensions());
 
   // For readability, alias extensions A B C.
   const Extension* extension_a = browser_action();
   const Extension* extension_b = page_action();
   const Extension* extension_c = no_action();
 
   // Sanity check: Order should start as A B C, with all three visible.
   EXPECT_EQ(3u, num_toolbar_items());
-  EXPECT_TRUE(toolbar_model()->all_icons_visible());
+  EXPECT_EQ(-1, toolbar_model()->GetVisibleIconCount());  // -1 = 'all'.
   EXPECT_EQ(extension_a, GetExtensionAtIndex(0u));
   EXPECT_EQ(extension_b, GetExtensionAtIndex(1u));
   EXPECT_EQ(extension_c, GetExtensionAtIndex(2u));
 
   ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
 
   // By default, all actions should be visible.
   EXPECT_TRUE(ExtensionActionAPI::GetBrowserActionVisibility(
                   prefs, extension_a->id()));
   EXPECT_TRUE(ExtensionActionAPI::GetBrowserActionVisibility(
                   prefs, extension_c->id()));
   EXPECT_TRUE(ExtensionActionAPI::GetBrowserActionVisibility(
                   prefs, extension_b->id()));
 
   // Hiding an action should result in it being sent to the overflow menu.
   ExtensionActionAPI::SetBrowserActionVisibility(
       prefs, extension_b->id(), false);
 
   // Thus, the order should be A C B, with B in the overflow.
   EXPECT_EQ(3u, num_toolbar_items());
-  EXPECT_EQ(2u, toolbar_model()->visible_icon_count());
+  EXPECT_EQ(2, toolbar_model()->GetVisibleIconCount());
   EXPECT_EQ(extension_a, GetExtensionAtIndex(0u));
   EXPECT_EQ(extension_c, GetExtensionAtIndex(1u));
   EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
 
   // Hiding an extension's action should result in it being sent to the overflow
   // as well, but as the _first_ extension in the overflow.
   ExtensionActionAPI::SetBrowserActionVisibility(
       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(1u, toolbar_model()->visible_icon_count());
+  EXPECT_EQ(1, toolbar_model()->GetVisibleIconCount());
   EXPECT_EQ(extension_c, GetExtensionAtIndex(0u));
   EXPECT_EQ(extension_a, GetExtensionAtIndex(1u));
   EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
 
   // Resetting A's visibility to true should send it back to the visible icons
   // (and should grow visible icons by 1), but it should be added to the end of
   // the visible icon list (not to its original position).
   ExtensionActionAPI::SetBrowserActionVisibility(
       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(2u, toolbar_model()->visible_icon_count());
+  EXPECT_EQ(2, toolbar_model()->GetVisibleIconCount());
   EXPECT_EQ(extension_c, GetExtensionAtIndex(0u));
   EXPECT_EQ(extension_a, GetExtensionAtIndex(1u));
   EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
 
   // Resetting B to be visible should make the order C A B, with no overflow.
   ExtensionActionAPI::SetBrowserActionVisibility(
       prefs, extension_b->id(), true);
   EXPECT_EQ(3u, num_toolbar_items());
-  EXPECT_TRUE(toolbar_model()->all_icons_visible());
+  EXPECT_EQ(-1, toolbar_model()->GetVisibleIconCount());  // -1 = 'all'
   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