Commit 9b92a6a2 authored by Peter Boström's avatar Peter Boström Committed by Commit Bot

Hide extensions container when none are installed

This change makes ExtensionsToolbarContainer invisible by default. It
turns visible when any extension is added and goes invisible if the last
extension is removed.

Bug: chromium:943702
Change-Id: Ia64d8db55d3cd53beff450941d1b7d2cba282ca0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1956186
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarCaroline Rising <corising@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737171}
parent 81fc6eaf
...@@ -191,8 +191,9 @@ void ExtensionService::AddProviderForTesting( ...@@ -191,8 +191,9 @@ void ExtensionService::AddProviderForTesting(
void ExtensionService::BlacklistExtensionForTest( void ExtensionService::BlacklistExtensionForTest(
const std::string& extension_id) { const std::string& extension_id) {
ExtensionIdSet blacklisted; ExtensionIdSet blacklisted;
ExtensionIdSet unchanged;
blacklisted.insert(extension_id); blacklisted.insert(extension_id);
// Don't change existing blocklisted extensions.
ExtensionIdSet unchanged = registry_->blacklisted_extensions().GetIDs();
UpdateBlacklistedExtensions(blacklisted, unchanged); UpdateBlacklistedExtensions(blacklisted, unchanged);
} }
......
...@@ -43,6 +43,13 @@ ...@@ -43,6 +43,13 @@
class ExtensionsMenuViewBrowserTest : public DialogBrowserTest { class ExtensionsMenuViewBrowserTest : public DialogBrowserTest {
protected: protected:
enum class ExtensionRemovalMethod {
kDisable,
kUninstall,
kBlocklist,
kTerminate,
};
Profile* profile() { return browser()->profile(); } Profile* profile() { return browser()->profile(); }
void LoadTestExtension(const std::string& extension, void LoadTestExtension(const std::string& extension,
...@@ -263,6 +270,51 @@ class ExtensionsMenuViewBrowserTest : public DialogBrowserTest { ...@@ -263,6 +270,51 @@ class ExtensionsMenuViewBrowserTest : public DialogBrowserTest {
views::test::WaitForAnimatingLayoutManager(GetExtensionsToolbarContainer()); views::test::WaitForAnimatingLayoutManager(GetExtensionsToolbarContainer());
} }
void RemoveExtension(ExtensionRemovalMethod method,
const std::string& extension_id) {
extensions::ExtensionService* const extension_service =
extensions::ExtensionSystem::Get(browser()->profile())
->extension_service();
switch (method) {
case ExtensionRemovalMethod::kDisable:
extension_service->DisableExtension(
extension_id, extensions::disable_reason::DISABLE_USER_ACTION);
break;
case ExtensionRemovalMethod::kUninstall:
extension_service->UninstallExtension(
extension_id, extensions::UNINSTALL_REASON_FOR_TESTING, nullptr);
break;
case ExtensionRemovalMethod::kBlocklist:
extension_service->BlacklistExtensionForTest(extension_id);
break;
case ExtensionRemovalMethod::kTerminate:
extension_service->TerminateExtension(extension_id);
break;
}
}
void VerifyContainerVisibility(ExtensionRemovalMethod method,
bool expected_visibility) {
// An empty container should not be shown.
EXPECT_FALSE(GetExtensionsToolbarContainer()->GetVisible());
// Loading the first extension should show the button (and container).
LoadTestExtension("extensions/uitest/long_name");
EXPECT_TRUE(GetExtensionsToolbarContainer()->IsDrawn());
// Add another extension so we can make sure that removing some don't change
// the visibility.
LoadTestExtension("extensions/uitest/window_open");
// Remove 1/2 extensions, should still be drawn.
RemoveExtension(method, extensions_[0]->id());
EXPECT_TRUE(GetExtensionsToolbarContainer()->IsDrawn());
// Removing the last extension. All actions now have the same state.
RemoveExtension(method, extensions_[1]->id());
EXPECT_EQ(expected_visibility, GetExtensionsToolbarContainer()->IsDrawn());
}
std::string ui_test_name_; std::string ui_test_name_;
base::test::ScopedFeatureList scoped_feature_list_; base::test::ScopedFeatureList scoped_feature_list_;
Browser* incognito_browser_ = nullptr; Browser* incognito_browser_ = nullptr;
...@@ -276,8 +328,29 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, InvokeUi_default) { ...@@ -276,8 +328,29 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, InvokeUi_default) {
ShowAndVerifyUi(); ShowAndVerifyUi();
} }
IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, InvokeUi_NoExtensions) { IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
ShowAndVerifyUi(); InvisibleWithoutExtension_Disable) {
VerifyContainerVisibility(ExtensionRemovalMethod::kDisable, false);
}
IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
InvisibleWithoutExtension_Uninstall) {
VerifyContainerVisibility(ExtensionRemovalMethod::kUninstall, false);
}
IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
InvisibleWithoutExtension_Blocklist) {
VerifyContainerVisibility(ExtensionRemovalMethod::kBlocklist, false);
}
IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
InvisibleWithoutExtension_Terminate) {
// TODO(pbos): Keep the container visible when extensions are terminated
// (crash). This lets users find and restart them. Then update this test
// expectation to be kept visible by terminated extensions. Also update the
// test name to reflect that the container should be visible with only
// terminated extensions.
VerifyContainerVisibility(ExtensionRemovalMethod::kTerminate, false);
} }
// Invokes the UI shown when a user has to reload a page in order to run an // Invokes the UI shown when a user has to reload a page in order to run an
...@@ -453,6 +526,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, ...@@ -453,6 +526,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
ManageExtensionsOpensExtensionsPage) { ManageExtensionsOpensExtensionsPage) {
// Ensure the menu is visible by adding an extension.
LoadTestExtension("extensions/trigger_actions/browser_action");
ShowUi(""); ShowUi("");
VerifyUi(); VerifyUi();
......
...@@ -41,6 +41,9 @@ ExtensionsToolbarContainer::ExtensionsToolbarContainer(Browser* browser) ...@@ -41,6 +41,9 @@ ExtensionsToolbarContainer::ExtensionsToolbarContainer(Browser* browser)
model_(ToolbarActionsModel::Get(browser_->profile())), model_(ToolbarActionsModel::Get(browser_->profile())),
model_observer_(this), model_observer_(this),
extensions_button_(new ExtensionsToolbarButton(browser_, this)) { extensions_button_(new ExtensionsToolbarButton(browser_, this)) {
// The container shouldn't show unless / until we have extensions available.
SetVisible(false);
model_observer_.Add(model_); model_observer_.Add(model_);
// Do not flip the Extensions icon in RTL. // Do not flip the Extensions icon in RTL.
extensions_button_->EnableCanvasFlippingForRTLUI(false); extensions_button_->EnableCanvasFlippingForRTLUI(false);
...@@ -283,6 +286,7 @@ void ExtensionsToolbarContainer::OnToolbarActionAdded( ...@@ -283,6 +286,7 @@ void ExtensionsToolbarContainer::OnToolbarActionAdded(
int index) { int index) {
CreateActionForId(action_id); CreateActionForId(action_id);
ReorderViews(); ReorderViews();
UpdateContainerVisibility();
} }
void ExtensionsToolbarContainer::OnToolbarActionRemoved( void ExtensionsToolbarContainer::OnToolbarActionRemoved(
...@@ -304,6 +308,8 @@ void ExtensionsToolbarContainer::OnToolbarActionRemoved( ...@@ -304,6 +308,8 @@ void ExtensionsToolbarContainer::OnToolbarActionRemoved(
UndoPopOut(); UndoPopOut();
icons_.erase(action_id); icons_.erase(action_id);
UpdateContainerVisibility();
} }
void ExtensionsToolbarContainer::OnToolbarActionMoved( void ExtensionsToolbarContainer::OnToolbarActionMoved(
...@@ -358,13 +364,13 @@ void ExtensionsToolbarContainer::CreateActions() { ...@@ -358,13 +364,13 @@ void ExtensionsToolbarContainer::CreateActions() {
CreateActionForId(action_id); CreateActionForId(action_id);
ReorderViews(); ReorderViews();
UpdateContainerVisibility();
} }
void ExtensionsToolbarContainer::CreateActionForId( void ExtensionsToolbarContainer::CreateActionForId(
const ToolbarActionsModel::ActionId& action_id) { const ToolbarActionsModel::ActionId& action_id) {
actions_.push_back( actions_.push_back(
model_->CreateActionForId(browser_, this, false, action_id)); model_->CreateActionForId(browser_, this, false, action_id));
auto icon = std::make_unique<ToolbarActionView>(actions_.back().get(), this); auto icon = std::make_unique<ToolbarActionView>(actions_.back().get(), this);
// Set visibility before adding to prevent extraneous animation. // Set visibility before adding to prevent extraneous animation.
icon->SetVisible(model_->IsActionPinned(action_id)); icon->SetVisible(model_->IsActionPinned(action_id));
...@@ -545,3 +551,9 @@ void ExtensionsToolbarContainer::SetExtensionIconVisibility( ...@@ -545,3 +551,9 @@ void ExtensionsToolbarContainer::SetExtensionIconVisibility(
views::Button::STATE_NORMAL, views::Button::STATE_NORMAL,
visible ? GetExtensionIcon(extension_view) : gfx::ImageSkia()); visible ? GetExtensionIcon(extension_view) : gfx::ImageSkia());
} }
void ExtensionsToolbarContainer::UpdateContainerVisibility() {
// The container (and extensions-menu button) should be visible if we have at
// least one extension.
SetVisible(!actions_.empty());
}
...@@ -155,6 +155,10 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView, ...@@ -155,6 +155,10 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView,
void SetExtensionIconVisibility(ToolbarActionsModel::ActionId id, void SetExtensionIconVisibility(ToolbarActionsModel::ActionId id,
bool visible); bool visible);
// Calls SetVisible to make sure that the container is showing only when there
// are extensions available.
void UpdateContainerVisibility();
// TabStripModelObserver: // TabStripModelObserver:
void OnTabStripModelChanged( void OnTabStripModelChanged(
TabStripModel* tab_strip_model, TabStripModel* tab_strip_model,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment