[Hotword] Making enabling/disabling the setting enable/disable the hotword extension.

chrome/browser/extensions/extension_service.cc

 // Copyright (c) 2013 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 "chrome/browser/extensions/extension_service.h"
 
 #include <algorithm>
 #include <iterator>
 #include <set>
 
@@ skipping 1050 matching lines @@
 
 void ExtensionService::DisableUserExtensions(
     const std::vector<std::string>& except_ids) {
   extensions::ManagementPolicy* management_policy =
       system_->management_policy();
   extensions::ExtensionList to_disable;
 
   const ExtensionSet& enabled_set = registry_->enabled_extensions();
   for (ExtensionSet::const_iterator extension = enabled_set.begin();
       extension != enabled_set.end(); ++extension) {
-    if (management_policy->UserMayModifySettings(extension->get(), NULL))
+    if (management_policy->UserMayModifySettings(extension->get(), NULL) &&
+        extension->get()->location() != Manifest::EXTERNAL_COMPONENT)
       to_disable.push_back(*extension);
   }
   const ExtensionSet& terminated_set = registry_->terminated_extensions();
   for (ExtensionSet::const_iterator extension = terminated_set.begin();
       extension != terminated_set.end(); ++extension) {
-    if (management_policy->UserMayModifySettings(extension->get(), NULL))
+    if (management_policy->UserMayModifySettings(extension->get(), NULL) &&
+        extension->get()->location() != Manifest::EXTERNAL_COMPONENT)

[miket_OOO, 2014/03/17 20:10:51]

This is starting to look ugly. What about wrapping this test into a helper
method?

bool ShouldBeDisabled(const Extension* extension) {
  if (...conditions...)
    return true;
  else if (...future conditions that someone else might add...) {
    return true;
  }
  return false;
}

As-is, it's susceptible to copy-paste errors in the future. Also, with boolean
tests that grow large like this, it's not really clear what's going on.

Bonus points if you put the helper method somewhere that others are more likely
to find and use it for other purposes in the future.

[rpetterson, 2014/03/17 21:14:29]

Because this needs to be merged, I'd rather keep the change small at this point.
I've filed a bug (crbug.com/353266) to cover this and will clean up right after
this. Does that seem reasonable?

       to_disable.push_back(*extension);
   }
 
   for (extensions::ExtensionList::const_iterator extension = to_disable.begin();
       extension != to_disable.end(); ++extension) {
     if ((*extension)->was_installed_by_default() &&
         extension_urls::IsWebstoreUpdateUrl(
             extensions::ManifestURL::GetUpdateURL(*extension)))
       continue;
     const std::string& id = (*extension)->id();
@@ skipping 1793 matching lines @@
 void ExtensionService::UnloadAllExtensionsInternal() {
   profile_->GetExtensionSpecialStoragePolicy()->RevokeRightsForAllExtensions();
 
   registry_->ClearAll();
   system_->runtime_data()->ClearAll();
 
   // TODO(erikkay) should there be a notification for this?  We can't use
   // EXTENSION_UNLOADED since that implies that the extension has been disabled
   // or uninstalled.
 }