Make hiding an extension action cause it to go to the overflow menu

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/macros.h"
 #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_service.h"
 #include "chrome/browser/extensions/extension_service_test_base.h"
 #include "chrome/browser/extensions/extension_toolbar_model.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/extension_action/action_info.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/extension.h"
@@ skipping 98 matching lines @@
   // Adds or removes the given |extension| and verify success.
   testing::AssertionResult AddExtension(
       scoped_refptr<const Extension> extension) WARN_UNUSED_RESULT;
   testing::AssertionResult RemoveExtension(
       scoped_refptr<const Extension> extension) WARN_UNUSED_RESULT;
 
   // Adds three extensions, all with browser actions.
   testing::AssertionResult AddBrowserActionExtensions() WARN_UNUSED_RESULT;
 
   // Adds three extensions, one each for browser action, page action, and no
-  // action.
+  // action, and are added in that order.
   testing::AssertionResult AddActionExtensions() WARN_UNUSED_RESULT;
 
   // Returns the extension at the given index in the toolbar model, or NULL
   // if one does not exist.
   const Extension* GetExtensionAtIndex(size_t index) const;
 
   ExtensionToolbarModel* toolbar_model() { return toolbar_model_.get(); }
 
   const ExtensionToolbarModelTestObserver* observer() const {
     return model_observer_.get();
@@ skipping 545 matching lines @@
   ASSERT_TRUE(AddActionExtensions());
 
   // With the switch on, extensions with page actions and no action should also
   // be displayed in the toolbar.
   EXPECT_EQ(3u, num_toolbar_items());
   EXPECT_EQ(browser_action(), GetExtensionAtIndex(0u));
   EXPECT_EQ(page_action(), GetExtensionAtIndex(1u));
   EXPECT_EQ(no_action(), GetExtensionAtIndex(2u));
 }
 
+// Test that hiding actions on the toolbar results in their removal from the
+// model when the redesign switch is not enabled.
+TEST_F(ExtensionToolbarModelUnitTest,
+       ExtensionToolbarActionsVisibilityNoSwitch) {
+  Init();
+
+  ASSERT_TRUE(AddBrowserActionExtensions());
+  // Sanity check: Order should start as A B C.
+  EXPECT_EQ(3u, num_toolbar_items());
+  EXPECT_EQ(browser_action_a(), GetExtensionAtIndex(0u));
+  EXPECT_EQ(browser_action_b(), GetExtensionAtIndex(1u));
+  EXPECT_EQ(browser_action_c(), GetExtensionAtIndex(2u));
+
+  ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
+
+  // By default, all actions should be visible.
+  EXPECT_TRUE(ExtensionActionAPI::GetBrowserActionVisibility(
+                  prefs, browser_action_a()->id()));
+  EXPECT_TRUE(ExtensionActionAPI::GetBrowserActionVisibility(
+                  prefs, browser_action_b()->id()));
+  EXPECT_TRUE(ExtensionActionAPI::GetBrowserActionVisibility(
+                  prefs, browser_action_c()->id()));
+
+  // Hiding an action should result in its removal from the toolbar.
+  ExtensionActionAPI::SetBrowserActionVisibility(
+      prefs, browser_action_b()->id(), false);
+  EXPECT_FALSE(ExtensionActionAPI::GetBrowserActionVisibility(
+                   prefs, browser_action_b()->id()));
+  // Thus, there should now only be two items on the toolbar - A and C.
+  EXPECT_EQ(2u, num_toolbar_items());
+  EXPECT_EQ(browser_action_a(), GetExtensionAtIndex(0u));
+  EXPECT_EQ(browser_action_c(), GetExtensionAtIndex(1u));
+
+  // Reseting the visibility to 'true' should result in the extension being

[Finnur, 2014/08/19 10:18:04]

nit: Two t's in Resetting. Same below.

[Devlin, 2014/08/19 16:54:40]

Whoops!  Done.

+  // added back at its original position.
+  ExtensionActionAPI::SetBrowserActionVisibility(
+      prefs, browser_action_b()->id(), true);
+  EXPECT_TRUE(ExtensionActionAPI::GetBrowserActionVisibility(
+                  prefs, browser_action_b()->id()));
+  // So the toolbar order should be A B C.
+  EXPECT_EQ(3u, num_toolbar_items());
+  EXPECT_EQ(browser_action_a(), GetExtensionAtIndex(0u));
+  EXPECT_EQ(browser_action_b(), GetExtensionAtIndex(1u));
+  EXPECT_EQ(browser_action_c(), GetExtensionAtIndex(2u));
+}
+
+// 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_EQ(-1, toolbar_model()->GetVisibleIconCount());  // -1 = 'all'

[Finnur, 2014/08/19 10:18:04]

nit: End comments with period.

[Devlin, 2014/08/19 16:54:40]

Done.

+  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(2, toolbar_model()->GetVisibleIconCount());
+  EXPECT_EQ(extension_a, GetExtensionAtIndex(0u));
+  EXPECT_EQ(extension_c, GetExtensionAtIndex(1u));
+  EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
+
+  // Removing a second extension should result in it being sent to the overflow

[Finnur, 2014/08/19 10:18:04]

'Removing' is misleading (you're not removing the extension).
'Hiding'?

[Devlin, 2014/08/19 16:54:39]

Good point.  Done.

+  // 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(1, toolbar_model()->GetVisibleIconCount());
+  EXPECT_EQ(extension_c, GetExtensionAtIndex(0u));
+  EXPECT_EQ(extension_a, GetExtensionAtIndex(1u));
+  EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
+
+  // Reseting 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(2, toolbar_model()->GetVisibleIconCount());
+  EXPECT_EQ(extension_c, GetExtensionAtIndex(0u));
+  EXPECT_EQ(extension_a, GetExtensionAtIndex(1u));
+  EXPECT_EQ(extension_b, GetExtensionAtIndex(2u));
+
+  // Reseting 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_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));
+}
+
 }  // namespace extensions