Remove support for installing enhanced bookmarks extension as an external component extension

The enhanced bookmarks extension is being distributed from the Web Store
as a user-installable extension. There's no longer a need to support
installing it as an external component extension, so remove the
associated support code.

BUG=503838

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

Cr-Commit-Position: refs/heads/master@{#338900}

7 files changed, 29 insertions, 105 deletions

diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 7a56094..375605c 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -2141,8 +2141,7 @@ void GetSanitizedEnabledFlags(
   *result = flags_storage->GetFlags();
 }
 
-bool SkipConditionalExperiment(const Experiment& experiment,
-                               FlagsStorage* flags_storage) {
+bool SkipConditionalExperiment(const Experiment& experiment) {
   chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
 
 #if defined(OS_ANDROID)
@@ -2335,7 +2334,7 @@ void GetFlagsExperimentsData(FlagsStorage* flags_storage,
 
   for (size_t i = 0; i < num_experiments; ++i) {
     const Experiment& experiment = experiments[i];
-    if (SkipConditionalExperiment(experiment, flags_storage))
+    if (SkipConditionalExperiment(experiment))
       continue;
 
     base::DictionaryValue* data = new base::DictionaryValue();
diff --git a/chrome/browser/bookmarks/enhanced_bookmarks_features.cc b/chrome/browser/bookmarks/enhanced_bookmarks_features.cc
index 475b517..bf826f4 100644
--- a/chrome/browser/bookmarks/enhanced_bookmarks_features.cc
+++ b/chrome/browser/bookmarks/enhanced_bookmarks_features.cc
@@ -4,71 +4,47 @@
 
 #include "chrome/browser/bookmarks/enhanced_bookmarks_features.h"
 
+#include <string>
+
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
+#include "build/build_config.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/variations/variations_associated_data.h"
 
-#if !defined(OS_ANDROID) && !defined(OS_IOS)
-#include "extensions/common/features/feature.h"
-#include "extensions/common/features/feature_provider.h"
-#endif  // !defined(OS_ANDROID) && !defined(OS_IOS)
-
 namespace {
 
 const char kFieldTrialName[] = "EnhancedBookmarks";
 
-#if !defined(OS_ANDROID)
-
-bool GetBookmarksExperimentExtensionID(std::string* extension_id) {
-  *extension_id = variations::GetVariationParamValue(
-      kFieldTrialName, "id");
-  if (extension_id->empty())
-    return false;
-
-#if defined(OS_IOS)
-  return true;
-#else
-  const extensions::FeatureProvider* feature_provider =
-      extensions::FeatureProvider::GetPermissionFeatures();
-  extensions::Feature* feature = feature_provider->GetFeature("metricsPrivate");
-  return feature && feature->IsIdInWhitelist(*extension_id);
-#endif  // defined(OS_IOS)
-}
-
-#endif  // !defined(OS_ANDROID)
-
 }  // namespace
 
 bool IsEnhancedBookmarksEnabled() {
-  std::string extension_id;
-  return IsEnhancedBookmarksEnabled(&extension_id);
-}
-
-bool IsEnhancedBookmarksEnabled(std::string* extension_id) {
-  // kEnhancedBookmarksExperiment flag could have values "", "1" and "0".
-  // "0" - user opted out. "1" is only possible on mobile as desktop needs a
-  // extension id that would not be available by just using the flag.
-
-#if defined(OS_ANDROID) || defined(OS_IOS)
-  // Tests use command line flag to force enhanced bookmark to be on.
-  bool opt_in = base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-                    switches::kEnhancedBookmarksExperiment) == "1";
-  if (opt_in)
+  // Enhanced bookmarks is not used on desktop, so it shouldn't be calling this
+  // function.
+#if !defined(OS_IOS) && !defined(OS_ANDROID)
+  NOTREACHED();
+#endif  // !defined(OS_IOS) || !defined(OS_ANDROID)
+
+  // kEnhancedBookmarksExperiment flag could have values "", "1" and "0".  "" -
+  // default, "0" - user opted out, "1" - user opted in.  Tests also use the
+  // command line flag to force enhanced bookmark to be on.
+  std::string switch_value =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kEnhancedBookmarksExperiment);
+  if (switch_value == "1")
     return true;
-#endif  // defined(OS_ANDROID) || defined(OS_IOS)
-
-  bool opt_out = base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-                     switches::kEnhancedBookmarksExperiment) == "0";
+  if (switch_value == "0")
+    return false;
 
-  if (opt_out)
+#if defined(OS_IOS)
+  // Check that the "id" param is present. This is a legacy of the desktop
+  // implementation providing the extension id via param. This probably should
+  // be replaced with code that checks the experiment name instead.
+  if (variations::GetVariationParamValue(kFieldTrialName, "id").empty())
     return false;
+#endif  // defined(OS_IOS)
 
-#if defined(OS_ANDROID)
   return true;
-#else
-  return GetBookmarksExperimentExtensionID(extension_id);
-#endif  // defined(OS_ANDROID)
 }
 
 bool IsEnableDomDistillerSet() {
diff --git a/chrome/browser/bookmarks/enhanced_bookmarks_features.h b/chrome/browser/bookmarks/enhanced_bookmarks_features.h
index 0bb4a2a..3786f59 100644
--- a/chrome/browser/bookmarks/enhanced_bookmarks_features.h
+++ b/chrome/browser/bookmarks/enhanced_bookmarks_features.h
@@ -5,21 +5,9 @@
 #ifndef CHROME_BROWSER_BOOKMARKS_ENHANCED_BOOKMARKS_FEATURES_H_
 #define CHROME_BROWSER_BOOKMARKS_ENHANCED_BOOKMARKS_FEATURES_H_
 
-#include <string>
-
-#include "build/build_config.h"
-
-class PrefService;
-
 // Returns true if enhanced bookmarks is enabled.
 bool IsEnhancedBookmarksEnabled();
 
-// Returns true and sets |extension_id| if enhanced bookmarks experiment is
-// enabled. Returns false if no bookmark experiment or extension id is empty.
-// Besides, this method takes commandline flags into account before trying
-// to get an extension_id.
-bool IsEnhancedBookmarksEnabled(std::string* extension_id);
-
 // Returns true when flag enable-dom-distiller is set or enabled from Finch.
 bool IsEnableDomDistillerSet();
 
diff --git a/chrome/browser/extensions/component_extensions_whitelist/whitelist.cc b/chrome/browser/extensions/component_extensions_whitelist/whitelist.cc
index aea5d76..bb72d80 100644
--- a/chrome/browser/extensions/component_extensions_whitelist/whitelist.cc
+++ b/chrome/browser/extensions/component_extensions_whitelist/whitelist.cc
@@ -6,7 +6,6 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
-#include "chrome/browser/bookmarks/enhanced_bookmarks_features.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "extensions/common/constants.h"
 #include "grit/browser_resources.h"
@@ -44,11 +43,6 @@ bool IsComponentExtensionWhitelisted(const std::string& extension_id) {
       return true;
   }
 
-  std::string bookmark_extension_id;
-  if (IsEnhancedBookmarksEnabled(&bookmark_extension_id) &&
-      bookmark_extension_id == extension_id)
-    return true;
-
 #if defined(ENABLE_APP_LIST) && defined(OS_CHROMEOS)
   std::string google_now_extension_id;
   if (GetGoogleNowExtensionId(&google_now_extension_id) &&
diff --git a/chrome/browser/extensions/external_component_loader.cc b/chrome/browser/extensions/external_component_loader.cc
index 27858d9..8bace51 100644
--- a/chrome/browser/extensions/external_component_loader.cc
+++ b/chrome/browser/extensions/external_component_loader.cc
@@ -5,8 +5,6 @@
 #include "chrome/browser/extensions/external_component_loader.h"
 
 #include "base/command_line.h"
-#include "base/strings/string_number_conversions.h"
-#include "chrome/browser/bookmarks/enhanced_bookmarks_features.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/component_extensions_whitelist/whitelist.h"
 #include "chrome/browser/search/hotword_service.h"
@@ -15,9 +13,8 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "components/signin/core/browser/signin_manager.h"
-#include "extensions/common/extension.h"
 #include "extensions/common/extension_urls.h"
-#include "extensions/common/features/simple_feature.h"
+#include "extensions/common/manifest.h"
 
 #if defined(OS_CHROMEOS)
 #include "base/command_line.h"
@@ -36,20 +33,6 @@ ExternalComponentLoader::ExternalComponentLoader(Profile* profile)
 
 ExternalComponentLoader::~ExternalComponentLoader() {}
 
-// static
-bool ExternalComponentLoader::IsModifiable(const Extension* extension) {
-  if (extension->location() == Manifest::EXTERNAL_COMPONENT) {
-    static const char* const kEnhancedExtensions[] = {
-        "D5736E4B5CF695CB93A2FB57E4FDC6E5AFAB6FE2",  // http://crbug.com/312900
-        "D57DE394F36DC1C3220E7604C575D29C51A6C495",  // http://crbug.com/319444
-        "3F65507A3B39259B38C8173C6FFA3D12DF64CCE9"   // http://crbug.com/371562
-    };
-    return SimpleFeature::IsIdInArray(
-        extension->id(), kEnhancedExtensions, arraysize(kEnhancedExtensions));
-  }
-  return false;
-}
-
 void ExternalComponentLoader::StartLoading() {
   prefs_.reset(new base::DictionaryValue());
   AddExternalExtension(extension_misc::kInAppPaymentsSupportAppId);
@@ -57,12 +40,6 @@ void ExternalComponentLoader::StartLoading() {
   if (HotwordServiceFactory::IsHotwordAllowed(profile_))
     AddExternalExtension(extension_misc::kHotwordSharedModuleId);
 
-  {
-    std::string extension_id;
-    if (IsEnhancedBookmarksEnabled(&extension_id))
-      AddExternalExtension(extension_id);
-  }
-
 #if defined(OS_CHROMEOS)
   {
     base::CommandLine* const command_line =
diff --git a/chrome/browser/extensions/external_component_loader.h b/chrome/browser/extensions/external_component_loader.h
index 4b5d52b..3d75a53 100644
--- a/chrome/browser/extensions/external_component_loader.h
+++ b/chrome/browser/extensions/external_component_loader.h
@@ -13,8 +13,6 @@
 
 namespace extensions {
 
-class Extension;
-
 // A specialization of the ExternalLoader that loads a hard-coded list of
 // external extensions, that should be considered components of chrome (but
 // unlike Component extensions, these extensions are installed from the webstore
@@ -25,9 +23,6 @@ class ExternalComponentLoader : public ExternalLoader {
  public:
   explicit ExternalComponentLoader(Profile* profile);
 
-  // True if |extension| should be modifiable by the user.
-  static bool IsModifiable(const extensions::Extension* extension);
-
  protected:
   void StartLoading() override;
 
diff --git a/chrome/browser/extensions/standard_management_policy_provider.cc b/chrome/browser/extensions/standard_management_policy_provider.cc
index 379778b..1a0542f 100644
--- a/chrome/browser/extensions/standard_management_policy_provider.cc
+++ b/chrome/browser/extensions/standard_management_policy_provider.cc
@@ -10,7 +10,6 @@
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_management.h"
-#include "chrome/browser/extensions/external_component_loader.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest.h"
 #include "grit/extensions_strings.h"
@@ -125,17 +124,13 @@ bool StandardManagementPolicyProvider::UserMayLoad(
 bool StandardManagementPolicyProvider::UserMayModifySettings(
     const Extension* extension,
     base::string16* error) const {
-  return AdminPolicyIsModifiable(extension, error) ||
-         (extension->location() == extensions::Manifest::EXTERNAL_COMPONENT &&
-          ExternalComponentLoader::IsModifiable(extension));
+  return AdminPolicyIsModifiable(extension, error);
 }
 
 bool StandardManagementPolicyProvider::MustRemainEnabled(
     const Extension* extension,
     base::string16* error) const {
-  return !AdminPolicyIsModifiable(extension, error) ||
-         (extension->location() == extensions::Manifest::EXTERNAL_COMPONENT &&
-          ExternalComponentLoader::IsModifiable(extension));
+  return !AdminPolicyIsModifiable(extension, error);
 }
 
 bool StandardManagementPolicyProvider::MustRemainDisabled(