Fix an extension api bug where clearing content settings cleared the

settings for unrelated content types.

The extensions api had a bug where calling clear on an instance of
content settings in turn called a function that did not take content
type as a parameter and cleared all content settings for that extension.
This adds a function that clears the settings set by an extension for a
particular content type.

Bug: 700404
Change-Id: I29d519a80006a892fa27be50cf54042247032b38
Reviewed-on: https://chromium-review.googlesource.com/1179620Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Melissa Galonsky <mgalonsky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591528}

chrome/browser/extensions/api/content_settings/content_settings_api.cc

@@ -99,7 +99,8 @@ ContentSettingsContentSettingClearFunction::Run() {
 
   scoped_refptr<ContentSettingsStore> store =
       ContentSettingsService::Get(browser_context())->content_settings_store();
-  store->ClearContentSettingsForExtension(extension_id(), scope);
+  store->ClearContentSettingsForExtensionAndContentType(extension_id(), scope,
+                                                        content_type);
 
   return RespondNow(NoArguments());
 }

chrome/browser/extensions/api/content_settings/content_settings_apitest.cc

@@ -326,6 +326,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionContentSettingsApiTest,
   EXPECT_TRUE(RunExtensionSubtest(kExtensionPath, "test.html")) << message_;
 }
 
+// Tests if an extension clearing content settings for one content type leaves
+// the others unchanged.
+IN_PROC_BROWSER_TEST_F(ExtensionContentSettingsApiTest, ClearProperlyGranular) {
+  const char kExtensionPath[] = "content_settings/clearproperlygranular";
+  EXPECT_TRUE(RunExtensionSubtest(kExtensionPath, "test.html")) << message_;
+}
+
 // Tests if changing permissions in incognito mode keeps the previous state of
 // regular mode.
 IN_PROC_BROWSER_TEST_F(ExtensionContentSettingsApiTest, IncognitoIsolation) {

chrome/browser/extensions/api/content_settings/content_settings_store.cc

@@ -235,11 +235,7 @@ void ContentSettingsStore::ClearContentSettingsForExtension(
   {
     base::AutoLock lock(lock_);
     OriginIdentifierValueMap* map = GetValueMap(ext_id, scope);
-    if (!map) {
-      // Fail gracefully in Release builds.
-      NOTREACHED();
-      return;
-    }
+    DCHECK(map);
     notify = !map->empty();
     map->clear();
   }
@@ -248,6 +244,33 @@ void ContentSettingsStore::ClearContentSettingsForExtension(
   }
 }
 
+void ContentSettingsStore::ClearContentSettingsForExtensionAndContentType(
+    const std::string& ext_id,
+    ExtensionPrefsScope scope,
+    ContentSettingsType content_type) {
+  bool notify = false;
+  {
+    base::AutoLock lock(lock_);
+    OriginIdentifierValueMap* map = GetValueMap(ext_id, scope);
+    DCHECK(map);
+
+    // Get all of the resource identifiers for this |content_type|.
+    std::set<ResourceIdentifier> resource_identifiers;
+    for (const auto& entry : *map) {
+      if (entry.first.content_type == content_type)
+        resource_identifiers.insert(entry.first.resource_identifier);
+    }
+
+    notify = !resource_identifiers.empty();
+
+    for (const ResourceIdentifier& resource_identifier : resource_identifiers)
+      map->DeleteValues(content_type, resource_identifier);
+  }
+  if (notify) {
+    NotifyOfContentSettingChanged(ext_id, scope != kExtensionPrefsScopeRegular);
+  }
+}
+
 std::unique_ptr<base::ListValue> ContentSettingsStore::GetSettingsForExtension(
     const std::string& extension_id,
     ExtensionPrefsScope scope) const {

chrome/browser/extensions/api/content_settings/content_settings_store.h

@@ -77,6 +77,13 @@ class ContentSettingsStore
   void ClearContentSettingsForExtension(const std::string& ext_id,
                                         ExtensionPrefsScope scope);
 
+  // Clears all contents settings set by the extension |ext_id| for the
+  // content type |content_type|.
+  void ClearContentSettingsForExtensionAndContentType(
+      const std::string& ext_id,
+      ExtensionPrefsScope scope,
+      ContentSettingsType content_type);
+
   // Serializes all content settings set by the extension with ID |extension_id|
   // and returns them as a ListValue. The caller takes ownership of the returned
   // value.

chrome/test/data/extensions/api_test/content_settings/clearproperlygranular/manifest.json

+{
+  "name" : "Content Settings API Test Extension",
+  "version" : "0.1",
+  "manifest_version": 2,
+  "description" : "Sets and checks content settings, ensuring clear works properly.",
+  "permissions": [ "contentSettings" ]
+}

chrome/test/data/extensions/api_test/content_settings/clearproperlygranular/test.html

+<!--
+ * Copyright 2018 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.
+-->
+<script src="test.js"></script>
\ No newline at end of file

chrome/test/data/extensions/api_test/content_settings/clearproperlygranular/test.js

+// Copyright 2018 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.
+
+// Content settings API test
+// Run with browser_tests --gtest_filter=ExtensionApiTest.ClearProperlyGranular
+
+var cs = chrome.contentSettings;
+
+function expect(expected, message) {
+  return chrome.test.callbackPass(function(value) {
+    chrome.test.assertEq(expected, value, message);
+  });
+}
+
+chrome.test.runTests([
+  function setAndCheckContentSettings() {
+    // Set settings for the camera and microphone to block.
+    cs['camera'].set({
+      'primaryPattern': '<all_urls>',
+      'secondaryPattern': '<all_urls>',
+      'setting': 'block'
+    });
+
+    cs['microphone'].set({
+      'primaryPattern': '<all_urls>',
+      'secondaryPattern': '<all_urls>',
+      'setting': 'block'
+    });
+
+    // Clearing the camera settings should leave the microphone settings
+    // unchanged.
+    cs['camera'].clear({});
+    var microphoneMessage = 'The microphone setting should be "block", but ' +
+      'was reset.';
+    cs['microphone'].get({
+      'primaryUrl': 'http://www.example.com',
+      'secondaryUrl': 'http://www.example.com'
+    }, expect({'setting': 'block'}, microphoneMessage));
+
+    var cameraMessage = 'The camera setting was reset and should be "ask"';
+    cs['camera'].get({
+      'primaryUrl': 'http://www.example.com',
+      'secondaryUrl': 'http://www.example.com'
+    }, expect({'setting': 'ask'}, cameraMessage));
+
+    // Clear microphone and ensure that its setting updates properly.
+    cs['microphone'].clear({});
+    microphoneMessage = 'The microphone setting was reset and should be "ask"';
+    cs['microphone'].get({
+      'primaryUrl': 'http://www.example.com',
+      'secondaryUrl': 'http://www.example.com'
+    }, expect({'setting': 'ask'}, microphoneMessage));
+  },
+]);