Convert AutoBlocker static class to KeyedService.

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 b38ee5edd010edea28f76564337c263b2f75c207..ee45f0d7cd038a5bc8cabc5c200569753c6385f0 100644
--- a/chrome/browser/permissions/permission_decision_auto_blocker.cc
+++ b/chrome/browser/permissions/permission_decision_auto_blocker.cc
@@ -12,11 +12,17 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#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_util.h"
+#include "chrome/browser/profiles/incognito_helpers.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/chrome_features.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/safe_browsing_db/database_manager.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/browser/web_contents.h"
@@ -36,6 +42,8 @@ int g_blacklist_embargo_days = 7;
 // permission due to repeated dismissals.
 int g_dismissal_embargo_days = 7;
 
+// TODO(meredithl): Migrate to a new and more fitting type, once metrics have
+// been gathered, and deprecate CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT.
 std::unique_ptr<base::DictionaryValue> GetOriginDict(
     HostContentSettingsMap* settings,
     const GURL& origin_url) {
@@ -102,6 +110,8 @@ int GetActionCount(const GURL& url,
 
 }  // namespace
 
+// PermissionDecisionAutoBlocker -----------------------------------------------
+
 // static
 const char PermissionDecisionAutoBlocker::kPromptDismissCountKey[] =
     "dismiss_count";
@@ -118,12 +128,26 @@ const char PermissionDecisionAutoBlocker::kPermissionBlacklistEmbargoKey[] =
 const char PermissionDecisionAutoBlocker::kPermissionDismissalEmbargoKey[] =
     "dismissal_embargo_days";
 
-// static
+// Maximum time in milliseconds to wait for safe browsing service to check a
+// url for blacklisting. After this amount of time, the check will be aborted
+// and the url will be treated as not blacklisted.

[kcarattini, 2017/01/23 02:52:41]

"not blacklisted" -> "safe" (just to keep consistent language with
SafeBrowsing).

[meredithl, 2017/01/24 01:20:46]

Done.

+// TODO(meredithl): Revisit this once UMA metrics have data about request time.
+const int kCheckUrlTimeoutMs = 2000;
+
+PermissionDecisionAutoBlocker::PermissionDecisionAutoBlocker(Profile* profile)
+    : profile_(profile), safe_browsing_timeout_(kCheckUrlTimeoutMs) {}
+
+PermissionDecisionAutoBlocker::~PermissionDecisionAutoBlocker() {}
+
+PermissionDecisionAutoBlocker* PermissionDecisionAutoBlocker::GetForProfile(
+    Profile* profile) {
+  return PermissionDecisionAutoBlockerFactory::GetForProfile(profile);
+}
+
 void PermissionDecisionAutoBlocker::RemoveCountsByUrl(
-    Profile* profile,
     base::Callback<bool(const GURL& url)> filter) {
   HostContentSettingsMap* map =
-      HostContentSettingsMapFactory::GetForProfile(profile);
+      HostContentSettingsMapFactory::GetForProfile(profile_);
 
   std::unique_ptr<ContentSettingsForOneType> settings(
       new ContentSettingsForOneType);
@@ -141,62 +165,40 @@ void PermissionDecisionAutoBlocker::RemoveCountsByUrl(
   }
 }
 
-// static
 int PermissionDecisionAutoBlocker::GetDismissCount(
     const GURL& url,
-    content::PermissionType permission,
-    Profile* profile) {
-  return GetActionCount(url, permission, kPromptDismissCountKey, profile);
+    content::PermissionType permission) {
+  return GetActionCount(url, permission, kPromptDismissCountKey, profile_);
 }
 
-// static
 int PermissionDecisionAutoBlocker::GetIgnoreCount(
     const GURL& url,
-    content::PermissionType permission,
-    Profile* profile) {
-  return GetActionCount(url, permission, kPromptIgnoreCountKey, profile);
+    content::PermissionType permission) {
+  return GetActionCount(url, permission, kPromptIgnoreCountKey, profile_);
 }
 
-// static
 bool PermissionDecisionAutoBlocker::RecordDismissAndEmbargo(
     const GURL& url,
     content::PermissionType permission,
-    Profile* profile,
     base::Time current_time) {
   int current_dismissal_count = RecordActionInWebsiteSettings(
-      url, permission, kPromptDismissCountKey, profile);
+      url, permission, kPromptDismissCountKey, profile_);
+
   if (base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften) &&
       current_dismissal_count >= g_prompt_dismissals_before_block) {
-    HostContentSettingsMap* map =
-        HostContentSettingsMapFactory::GetForProfile(profile);
-    PlaceUnderEmbargo(permission, url, map, current_time,
-                      kPermissionDismissalEmbargoKey);
+    PlaceUnderEmbargo(permission, url,
+                      HostContentSettingsMapFactory::GetForProfile(profile_),
+                      current_time, kPermissionDismissalEmbargoKey);
     return true;
   }
   return false;
 }
 
-// static
 int PermissionDecisionAutoBlocker::RecordIgnore(
     const GURL& url,
-    content::PermissionType permission,
-    Profile* profile) {
+    content::PermissionType permission) {
   return RecordActionInWebsiteSettings(url, permission, kPromptIgnoreCountKey,
-                                       profile);
-}
-
-// static
-bool PermissionDecisionAutoBlocker::ShouldChangeDismissalToBlock(
-    const GURL& url,
-    content::PermissionType permission,
-    Profile* profile) {
-  int current_dismissal_count =
-      RecordDismissAndEmbargo(url, permission, profile, base::Time::Now());
-
-  if (!base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften))
-    return false;
-
-  return current_dismissal_count >= g_prompt_dismissals_before_block;
+                                       profile_);
 }
 
 // static
@@ -228,30 +230,32 @@ void PermissionDecisionAutoBlocker::UpdateFromVariations() {
   }
 }
 
-// static
-// TODO(meredithl): Have PermissionDecisionAutoBlocker handle the database
-// manager, rather than passing it in.
 void PermissionDecisionAutoBlocker::UpdateEmbargoedStatus(
-    scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
     content::PermissionType permission,
     const GURL& request_origin,
     content::WebContents* web_contents,
-    int timeout,
-    Profile* profile,
     base::Time current_time,
     base::Callback<void(bool)> callback) {
   // Check if origin is currently under embargo for the requested permission.
-  if (IsUnderEmbargo(permission, profile, request_origin, current_time)) {
+  if (IsUnderEmbargo(permission, request_origin, current_time)) {
     callback.Run(true /* permission_blocked */);
     return;
   }
 
+  if (!db_manager_) {
+    safe_browsing::SafeBrowsingService* sb_service =
+        g_browser_process->safe_browsing_service();
+    if (sb_service)
+      db_manager_ = sb_service->database_manager();
+  }

[raymes, 2017/01/23 06:14:23]

nit: Is it worth just putting this in the constructor now? I don't think there
is any benefit to having it here.

[meredithl, 2017/01/24 01:20:46]

Done.

+
   if (base::FeatureList::IsEnabled(features::kPermissionsBlacklist) &&
-      db_manager) {
+      db_manager_) {
     PermissionBlacklistClient::CheckSafeBrowsingBlacklist(
-        db_manager, permission, request_origin, web_contents, timeout,
+        db_manager_, permission, request_origin, web_contents,
+        safe_browsing_timeout_,
         base::Bind(&PermissionDecisionAutoBlocker::CheckSafeBrowsingResult,
-                   permission, profile, request_origin, current_time,
+                   permission, profile_, request_origin, current_time,
                    callback));
     return;
   }
@@ -259,14 +263,12 @@ void PermissionDecisionAutoBlocker::UpdateEmbargoedStatus(
   callback.Run(false /* permission blocked */);
 }
 
-// static
 bool PermissionDecisionAutoBlocker::IsUnderEmbargo(
     content::PermissionType permission,
-    Profile* profile,
     const GURL& request_origin,
     base::Time current_time) {
   HostContentSettingsMap* map =
-      HostContentSettingsMapFactory::GetForProfile(profile);
+      HostContentSettingsMapFactory::GetForProfile(profile_);
   std::unique_ptr<base::DictionaryValue> dict =
       GetOriginDict(map, request_origin);
   base::DictionaryValue* permission_dict = GetOrCreatePermissionDict(
@@ -293,24 +295,17 @@ bool PermissionDecisionAutoBlocker::IsUnderEmbargo(
       is_under_dismiss_embargo = true;
     }
   }
-  // If either embargoes is still in effect, return true.
+
+  // If either embargo is still in effect, return true.
   return is_under_dismiss_embargo || is_under_blacklist_embargo;
 }
 
-void PermissionDecisionAutoBlocker::PlaceUnderEmbargo(
-    content::PermissionType permission,
-    const GURL& request_origin,
-    HostContentSettingsMap* map,
-    base::Time current_time,
-    const char* key) {
-  std::unique_ptr<base::DictionaryValue> dict =
-      GetOriginDict(map, request_origin);
-  base::DictionaryValue* permission_dict = GetOrCreatePermissionDict(
-      dict.get(), PermissionUtil::GetPermissionString(permission));
-  permission_dict->SetDouble(key, current_time.ToInternalValue());
-  map->SetWebsiteSettingDefaultScope(
-      request_origin, GURL(), CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,
-      std::string(), std::move(dict));
+void PermissionDecisionAutoBlocker::
+    SetSafeBrowsingDatabaseManagerAndTimeoutForTesting(
+        scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
+        int timeout) {
+  db_manager_ = db_manager;
+  safe_browsing_timeout_ = timeout;
 }
 
 // static
@@ -330,3 +325,54 @@ void PermissionDecisionAutoBlocker::CheckSafeBrowsingResult(
   }
   callback.Run(should_be_embargoed /* permission blocked */);
 }
+
+// static
+void PermissionDecisionAutoBlocker::PlaceUnderEmbargo(
+    content::PermissionType permission,
+    const GURL& request_origin,
+    HostContentSettingsMap* map,
+    base::Time current_time,
+    const char* key) {
+  std::unique_ptr<base::DictionaryValue> dict =
+      GetOriginDict(map, request_origin);
+  base::DictionaryValue* permission_dict = GetOrCreatePermissionDict(
+      dict.get(), PermissionUtil::GetPermissionString(permission));
+  permission_dict->SetDouble(key, current_time.ToInternalValue());
+  map->SetWebsiteSettingDefaultScope(
+      request_origin, GURL(), CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,
+      std::string(), std::move(dict));
+}
+
+// PermissionDecisionAutoBlockerFactory ----------------------------------------
+
+// static
+PermissionDecisionAutoBlocker*
+PermissionDecisionAutoBlockerFactory::GetForProfile(Profile* profile) {
+  return static_cast<PermissionDecisionAutoBlocker*>(
+      GetInstance()->GetServiceForBrowserContext(profile, true));
+}
+
+// static
+PermissionDecisionAutoBlockerFactory*
+PermissionDecisionAutoBlockerFactory::GetInstance() {
+  return base::Singleton<PermissionDecisionAutoBlockerFactory>::get();
+}
+
+PermissionDecisionAutoBlockerFactory::PermissionDecisionAutoBlockerFactory()
+    : BrowserContextKeyedServiceFactory(
+          "PermissionDecisionAutoBlocker",
+          BrowserContextDependencyManager::GetInstance()) {}
+
+PermissionDecisionAutoBlockerFactory::~PermissionDecisionAutoBlockerFactory() {}
+
+KeyedService* PermissionDecisionAutoBlockerFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  Profile* profile = static_cast<Profile*>(context);
+  return new PermissionDecisionAutoBlocker(profile);
+}
+
+content::BrowserContext*
+PermissionDecisionAutoBlockerFactory::GetBrowserContextToUse(
+    content::BrowserContext* context) const {
+  return chrome::GetBrowserContextOwnInstanceInIncognito(context);
+}