Fix default value of ExtensionManagement::IndividualSettings

This CL fixes a bug in commit b2454381 "Add new ExtensionManagement preference". The default value of IndividualSettings are supposed to be |default_settings_| instead of fallback value for each settings.
This bug was not exposed in unit tests since IndividualSettings contains only installation mode settings now.

BUG=177351

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

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

chrome/browser/extensions/extension_management.cc

@@ -45,8 +45,6 @@ bool ParseIndividualSettings(
     const base::DictionaryValue* dict,
     Scope scope,
     ExtensionManagement::IndividualSettings* settings) {
-  settings->Reset();
-
   std::string installation_mode;
   if (dict->GetStringWithoutPathExpansion(schema_constants::kInstallationMode,
                                           &installation_mode)) {
@@ -91,6 +89,13 @@ bool ParseIndividualSettings(
 
 }  // namespace
 
+ExtensionManagement::IndividualSettings::IndividualSettings() {
+  Reset();
+}
+
+ExtensionManagement::IndividualSettings::~IndividualSettings() {
+}
+
 void ExtensionManagement::IndividualSettings::Reset() {
   installation_mode = ExtensionManagement::INSTALLATION_ALLOWED;
   update_url.clear();
@@ -334,20 +339,18 @@ void ExtensionManagement::Refresh() {
         LOG(WARNING) << kMalformedPreferenceWarning;
         continue;
       }
-      if (StartsWithASCII(
-              iter.key(), schema_constants::kUpdateUrlPrefix, true))
+      if (StartsWithASCII(iter.key(), schema_constants::kUpdateUrlPrefix, true))
         continue;
       const std::string& extension_id = iter.key();
       if (!crx_file::id_util::IdIsValid(extension_id)) {
         LOG(WARNING) << kMalformedPreferenceWarning;
         continue;
       }
-      IndividualSettings by_id;
-      if (ParseIndividualSettings(subdict, SCOPE_INDIVIDUAL, &by_id)) {
-        *AccessById(extension_id) = by_id;
-      } else {
+      IndividualSettings* by_id = AccessById(extension_id);
+      if (!ParseIndividualSettings(subdict, SCOPE_INDIVIDUAL, by_id)) {
+        settings_by_id_.erase(settings_by_id_.find(extension_id));
         LOG(WARNING) << "Malformed Extension Management settings for "
-                     << iter.key() << ".";
+                     << extension_id << ".";
       }
     }
   }

chrome/browser/extensions/extension_management.h

@@ -64,6 +64,9 @@ class ExtensionManagement : public KeyedService {
   // by an ID, a group of extensions with specific |update_url| or all
   // extensions at once.
   struct IndividualSettings {
+    IndividualSettings();
+    ~IndividualSettings();
+
     void Reset();
 
     // Extension installation mode. Setting this to INSTALLATION_FORCED or
@@ -72,6 +75,8 @@ class ExtensionManagement : public KeyedService {
     // be specified, containing the update URL for this extension.
     // Note that |update_url| will be ignored for INSTALLATION_ALLOWED and
     // INSTALLATION_BLOCKED installation mode.
+    // These settings will override the default settings, and unspecified
+    // settings will take value from default settings.
     InstallationMode installation_mode;
     std::string update_url;
   };

chrome/browser/extensions/extension_management_test_util.cc

@@ -27,6 +27,18 @@ ExtensionManagementPrefUpdaterBase::ExtensionManagementPrefUpdaterBase() {
 ExtensionManagementPrefUpdaterBase::~ExtensionManagementPrefUpdaterBase() {
 }
 
+void ExtensionManagementPrefUpdaterBase::UnsetPerExtensionSettings(
+    const ExtensionId& id) {
+  DCHECK(crx_file::id_util::IdIsValid(id));
+  pref_->RemoveWithoutPathExpansion(id, NULL);
+}
+
+void ExtensionManagementPrefUpdaterBase::ClearPerExtensionSettings(
+    const ExtensionId& id) {
+  DCHECK(crx_file::id_util::IdIsValid(id));
+  pref_->SetWithoutPathExpansion(id, new base::DictionaryValue());
+}
+
 void ExtensionManagementPrefUpdaterBase::SetBlacklistedByDefault(bool value) {
   pref_->SetString(make_path(schema::kWildcard, schema::kInstallationMode),
                    value ? schema::kBlocked : schema::kAllowed);

chrome/browser/extensions/extension_management_test_util.h

@@ -22,6 +22,10 @@ class ExtensionManagementPrefUpdaterBase {
   ExtensionManagementPrefUpdaterBase();
   virtual ~ExtensionManagementPrefUpdaterBase();
 
+  // Helper functions for per extension settings.
+  void UnsetPerExtensionSettings(const ExtensionId& id);
+  void ClearPerExtensionSettings(const ExtensionId& id);
+
   // Helper functions for 'installation_mode' manipulation.
   void SetBlacklistedByDefault(bool value);
   void ClearInstallationModesForIndividualExtensions();

chrome/browser/extensions/extension_management_unittest.cc

@@ -423,6 +423,8 @@ TEST_F(ExtensionManagementServiceTest, NewInstallBlacklist) {
     PrefUpdater updater(pref_service_.get());
     updater.SetBlacklistedByDefault(false);  // Allowed by default.
     updater.SetIndividualExtensionInstallationAllowed(kTargetExtension, false);
+    updater.ClearPerExtensionSettings(kTargetExtension2);
+    updater.ClearPerExtensionSettings(kOtherExtension);
   }
   EXPECT_FALSE(extension_management_->BlacklistedByDefault());
   EXPECT_EQ(extension_management_->ReadById(kTargetExtension).installation_mode,
@@ -459,6 +461,8 @@ TEST_F(ExtensionManagementServiceTest, NewInstallWhitelist) {
     PrefUpdater updater(pref_service_.get());
     updater.SetBlacklistedByDefault(true);  // Disallowed by default.
     updater.SetIndividualExtensionInstallationAllowed(kTargetExtension, true);
+    updater.ClearPerExtensionSettings(kTargetExtension2);
+    updater.ClearPerExtensionSettings(kOtherExtension);
   }
   EXPECT_TRUE(extension_management_->BlacklistedByDefault());
   EXPECT_EQ(extension_management_->ReadById(kTargetExtension).installation_mode,
@@ -500,6 +504,7 @@ TEST_F(ExtensionManagementServiceTest, NewInstallForcelist) {
     PrefUpdater updater(pref_service_.get());
     updater.SetIndividualExtensionAutoInstalled(
         kTargetExtension, kExampleUpdateUrl, true);
+    updater.ClearPerExtensionSettings(kOtherExtension);
   }
   EXPECT_EQ(extension_management_->ReadById(kTargetExtension).installation_mode,
             ExtensionManagement::INSTALLATION_FORCED);