Permissions: Clear embargo if user changes an embargoed permission's setting.

chrome/browser/permissions/permission_decision_auto_blocker.cc

Index: chrome/browser/permissions/permission_decision_auto_blocker.cc
diff --git a/chrome/browser/permissions/permission_decision_auto_blocker.cc b/chrome/browser/permissions/permission_decision_auto_blocker.cc
index a41d4ba59e1b39117fbbc976e00d49bc2ba6dabd..aaf71e7cf3900305e4b209e1a052f68677cd1aff 100644
--- a/chrome/browser/permissions/permission_decision_auto_blocker.cc
+++ b/chrome/browser/permissions/permission_decision_auto_blocker.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/permissions/permission_blacklist_client.h"
+#include "chrome/browser/permissions/permission_manager.h"
 #include "chrome/browser/permissions/permission_util.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
@@ -304,6 +305,47 @@ int PermissionDecisionAutoBlocker::RecordIgnore(
                                        profile_);
 }
 
+void PermissionDecisionAutoBlocker::RemoveEmbargoByUrl(
+    const GURL& origin_url,
+    const GURL& embedder_url,
+    ContentSettingsType permission) {
+  if (!PermissionUtil::IsPermission(permission))
+    return;
+
+  // Don't proceed if |permission| was not under embargo for |origin_url|.
+  PermissionResult permission_result =
+      PermissionManager::Get(profile_)->GetPermissionStatus(

[dominickn, 2017/04/05 08:50:19]

Instead of calling PermissionManager (and thus requiring an embedder URL), you
can just call:

PermissionResult result = GetEmbargoResult(origin_url, permission);

(that's what PermissionManager eventually calls into to check embargo)

Then you can remove the embedder URL from everywhere, since embargo doesn't care
about it (and I'm pretty sure it's actually empty for all permissions now). You
also don't need the cyclic header include back to permission_manager.h

[Patti Lor, 2017/04/06 02:14:02]

Done, thanks Dom :)

+          permission, origin_url, embedder_url);
+  switch (permission_result.source) {

[dominickn, 2017/04/05 08:50:19]

I think it's clearer to just do

if (result.source != PermissionStatusSource::MULTIPLE_DISMISSALS ||
    result.source != PermissionStatusSource::SAFE_BROWSING_BLACKLIST) {
  return;
}

[Patti Lor, 2017/04/06 02:14:02]

Done.

+    case PermissionStatusSource::MULTIPLE_DISMISSALS:
+    case PermissionStatusSource::SAFE_BROWSING_BLACKLIST:
+      break;
+    default:
+      return;
+  }
+
+  HostContentSettingsMap* map =
+      HostContentSettingsMapFactory::GetForProfile(profile_);
+  std::unique_ptr<base::DictionaryValue> dict = GetOriginDict(map, origin_url);
+  base::DictionaryValue* permission_dict = GetOrCreatePermissionDict(
+      dict.get(), PermissionUtil::GetPermissionString(permission));
+
+  // Deleting non-existent entries will return a false value. Since it should be
+  // impossible for a permission to have been embargoed for two different
+  // reasons at the same time, check that exactly one deletion was successful.
+  const bool dismissal_key_deleted =
+      permission_dict->RemoveWithoutPathExpansion(
+          kPermissionDismissalEmbargoKey, nullptr);
+  const bool blacklist_key_deleted =
+      permission_dict->RemoveWithoutPathExpansion(
+          kPermissionBlacklistEmbargoKey, nullptr);
+  DCHECK(dismissal_key_deleted != blacklist_key_deleted);
+
+  map->SetWebsiteSettingDefaultScope(
+      origin_url, GURL(), CONTENT_SETTINGS_TYPE_PERMISSION_AUTOBLOCKER_DATA,
+      std::string(), std::move(dict));
+}
+
 void PermissionDecisionAutoBlocker::RemoveCountsByUrl(
     base::Callback<bool(const GURL& url)> filter) {
   HostContentSettingsMap* map =