Commit f1e0ecaf authored by Ioana Pandele's avatar Ioana Pandele Committed by Commit Bot

Revert "Hide extensions container when none are installed"

This reverts commit 9b92a6a2.

Reason for revert: Multiple ExtensionsMenuViewBrowserTest cases failing on Linux bots.

Bug:1047630

Original change's description:
> 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: Devlin <rdevlin.cronin@chromium.org>
> Reviewed-by: Caroline Rising <corising@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#737171}

TBR=rdevlin.cronin@chromium.org,pbos@chromium.org,corising@chromium.org

Change-Id: I012b5f1137c5af73af2cbe1ac2e4b0a123cd2759
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:943702
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032108Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737262}
parent 99a27f95
...@@ -191,9 +191,8 @@ void ExtensionService::AddProviderForTesting( ...@@ -191,9 +191,8 @@ 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,13 +43,6 @@ ...@@ -43,13 +43,6 @@
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,
...@@ -270,51 +263,6 @@ class ExtensionsMenuViewBrowserTest : public DialogBrowserTest { ...@@ -270,51 +263,6 @@ 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;
...@@ -328,29 +276,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, InvokeUi_default) { ...@@ -328,29 +276,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, InvokeUi_default) {
ShowAndVerifyUi(); ShowAndVerifyUi();
} }
IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, InvokeUi_NoExtensions) {
InvisibleWithoutExtension_Disable) { ShowAndVerifyUi();
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
...@@ -526,8 +453,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest, ...@@ -526,8 +453,6 @@ 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,9 +41,6 @@ ExtensionsToolbarContainer::ExtensionsToolbarContainer(Browser* browser) ...@@ -41,9 +41,6 @@ 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);
...@@ -286,7 +283,6 @@ void ExtensionsToolbarContainer::OnToolbarActionAdded( ...@@ -286,7 +283,6 @@ void ExtensionsToolbarContainer::OnToolbarActionAdded(
int index) { int index) {
CreateActionForId(action_id); CreateActionForId(action_id);
ReorderViews(); ReorderViews();
UpdateContainerVisibility();
} }
void ExtensionsToolbarContainer::OnToolbarActionRemoved( void ExtensionsToolbarContainer::OnToolbarActionRemoved(
...@@ -308,8 +304,6 @@ void ExtensionsToolbarContainer::OnToolbarActionRemoved( ...@@ -308,8 +304,6 @@ void ExtensionsToolbarContainer::OnToolbarActionRemoved(
UndoPopOut(); UndoPopOut();
icons_.erase(action_id); icons_.erase(action_id);
UpdateContainerVisibility();
} }
void ExtensionsToolbarContainer::OnToolbarActionMoved( void ExtensionsToolbarContainer::OnToolbarActionMoved(
...@@ -364,13 +358,13 @@ void ExtensionsToolbarContainer::CreateActions() { ...@@ -364,13 +358,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));
...@@ -551,9 +545,3 @@ void ExtensionsToolbarContainer::SetExtensionIconVisibility( ...@@ -551,9 +545,3 @@ 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,10 +155,6 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView, ...@@ -155,10 +155,6 @@ 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