[Extensions] Don't force toolbar icons for default-installed extensions

BUG=591732

Review URL: https://codereview.chromium.org/1757363002

Cr-Commit-Position: refs/heads/master@{#379051}
(cherry picked from commit 42eb5e2ced5dd13c85b2e5bee8a2d67463dea53d)

Review URL: https://codereview.chromium.org/1769463003 .

Cr-Commit-Position: refs/branch-heads/2623@{#576}
Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907}

2 files changed, 53 insertions, 0 deletions

diff --git a/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc b/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc
index 82c2d05..f65dd78 100644
--- a/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc
+++ b/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc
@@ -42,6 +42,7 @@
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
 #include "extensions/common/feature_switch.h"
+#include "extensions/common/manifest.h"
 #include "extensions/common/value_builder.h"
 
 namespace {
@@ -171,6 +172,9 @@ class ToolbarActionsModelUnitTest
                                        const ToolbarActionsModel* model) const;
   const std::string GetActionIdAtIndex(size_t index) const;
 
+  // Returns true if the |toobar_model_| has an action with the given |id|.
+  bool ModelHasActionForId(const std::string& id) const;
+
   void SetMockActionsFactory(MockComponentToolbarActionsFactory* factory);
 
   ToolbarActionsModel* toolbar_model() { return toolbar_model_; }
@@ -384,6 +388,15 @@ const std::string ToolbarActionsModelUnitTest::GetActionIdAtIndex(
   return GetActionIdAtIndex(index, toolbar_model_);
 }
 
+bool ToolbarActionsModelUnitTest::ModelHasActionForId(
+    const std::string& id) const {
+  for (const auto& item : toolbar_model_->toolbar_items()) {
+    if (item.id == id)
+      return true;
+  }
+  return false;
+}
+
 testing::AssertionResult ToolbarActionsModelUnitTest::AddAndVerifyExtensions(
     const extensions::ExtensionList& extensions) {
   for (extensions::ExtensionList::const_iterator iter = extensions.begin();
@@ -921,6 +934,44 @@ TEST_F(ToolbarActionsModelUnitTest, TestToolbarExtensionTypesSwitch) {
   EXPECT_EQ(browser_action()->id(), GetActionIdAtIndex(0u));
   EXPECT_EQ(page_action()->id(), GetActionIdAtIndex(1u));
   EXPECT_EQ(no_action()->id(), GetActionIdAtIndex(2u));
+
+  // Extensions that are installed by default shouldn't be given an icon.
+  extensions::DictionaryBuilder default_installed_manifest;
+  default_installed_manifest.Set("name", "default installed")
+      .Set("description", "A default installed extension")
+      .Set("manifest_version", 2)
+      .Set("version", "1.0.0.0");
+  scoped_refptr<const extensions::Extension> default_installed_extension =
+      extensions::ExtensionBuilder()
+          .SetManifest(default_installed_manifest.Build())
+          .SetID(crx_file::id_util::GenerateId("default"))
+          .SetLocation(extensions::Manifest::INTERNAL)
+          .AddFlags(extensions::Extension::WAS_INSTALLED_BY_DEFAULT)
+          .Build();
+  EXPECT_TRUE(AddExtension(default_installed_extension.get()));
+  EXPECT_EQ(3u, num_toolbar_items());
+  EXPECT_FALSE(ModelHasActionForId(default_installed_extension->id()));
+
+  // Component extensions shouldn't be given an icon.
+  scoped_refptr<const extensions::Extension> component_extension_no_action =
+      extensions::extension_action_test_util::CreateActionExtension(
+          "component ext no action",
+          extensions::extension_action_test_util::NO_ACTION,
+          extensions::Manifest::COMPONENT);
+  EXPECT_TRUE(AddExtension(component_extension_no_action.get()));
+  EXPECT_EQ(3u, num_toolbar_items());
+  EXPECT_FALSE(ModelHasActionForId(component_extension_no_action->id()));
+
+  // Sanity check: A new extension that's installed from the webstore should
+  // have an icon.
+  scoped_refptr<const extensions::Extension> internal_extension_no_action =
+      extensions::extension_action_test_util::CreateActionExtension(
+          "internal ext no action",
+          extensions::extension_action_test_util::NO_ACTION,
+          extensions::Manifest::INTERNAL);
+  EXPECT_TRUE(AddExtension(internal_extension_no_action.get()));
+  EXPECT_EQ(4u, num_toolbar_items());
+  EXPECT_TRUE(ModelHasActionForId(internal_extension_no_action->id()));
 }
 
 // Test that hiding actions on the toolbar results in their removal from the
diff --git a/chrome/common/extensions/manifest_handlers/extension_action_handler.cc b/chrome/common/extensions/manifest_handlers/extension_action_handler.cc
index a976b00..15682c6 100644
--- a/chrome/common/extensions/manifest_handlers/extension_action_handler.cc
+++ b/chrome/common/extensions/manifest_handlers/extension_action_handler.cc
@@ -63,6 +63,8 @@ bool ExtensionActionHandler::Parse(Extension* extension,
       return true;  // Do nothing if the switch is off.
     if (Manifest::IsComponentLocation(extension->location()))
       return true;  // Don't synthesize actions for component extensions.
+    if (extension->was_installed_by_default())
+      return true;  // Don't synthesize actions for default extensions.
     if (extension->manifest()->HasKey(
             manifest_keys::kSynthesizeExtensionAction)) {
       *error = base::ASCIIToUTF16(base::StringPrintf(