Implement origin specific Permissions Blacklisting.

chrome/browser/permissions/permission_context_base.h

Index: chrome/browser/permissions/permission_context_base.h
diff --git a/chrome/browser/permissions/permission_context_base.h b/chrome/browser/permissions/permission_context_base.h
index f556165e1334adbf2f4f866fe7c3528a137b98cc..236046b82aed6caf5a76f9ec0f00ac4fe8ceb917 100644
--- a/chrome/browser/permissions/permission_context_base.h
+++ b/chrome/browser/permissions/permission_context_base.h
@@ -28,6 +28,10 @@ namespace content {
 class WebContents;
 }
 
+namespace safe_browsing {
+class SafeBrowsingDatabaseManager;
+}
+
 using BrowserPermissionCallback = base::Callback<void(ContentSetting)>;
 
 // This base class contains common operations for granting permissions.
@@ -80,7 +84,21 @@ class PermissionContextBase : public KeyedService {
                                  bool user_gesture,
                                  const BrowserPermissionCallback& callback);
 
-  // Returns whether the permission has been granted, denied...
+  // Called when the requesting origin and permission have been checked by Safe
+  // Browsing. |permission_blocked| determines whether to auto-block the
+  // permission request without prompting the user for a decision.
+  void CheckPermissionsBlacklistResult(

[raymes, 2016/12/19 00:07:41]

nit: should this be private?

[meredithl, 2016/12/20 07:38:27]

Done.

+      content::WebContents* web_contents,
+      const PermissionRequestID& id,
+      const GURL& requesting_origin,
+      const GURL& embedding_origin,
+      bool user_gesture,
+      const BrowserPermissionCallback& callback,
+      bool permission_blocked);
+
+  // Returns whether the permission has been granted, denied etc. This method
+  // does not take into account the Safe Browsing blacklist, as it is executed
+  // synchronously.

[raymes, 2016/12/19 00:07:41]

nit: I think the fact that this doesn't handle the blacklist check isn't the
intended behavior so we should mark it as such, e.g.  having this as a TODO
instead TODO(...): Ensure that this result accurately reflects whether the
origin is blacklisted for this permission

[meredithl, 2016/12/20 07:38:27]

Done.

[raymes, 2016/12/20 23:58:56]

I don't see this updated :(

[meredithl, 2016/12/29 06:23:35]

Haha, git add -p blues. Fixed!

   virtual ContentSetting GetPermissionStatus(
       const GURL& requesting_origin,
       const GURL& embedding_origin) const;
@@ -155,6 +173,10 @@ class PermissionContextBase : public KeyedService {
     return content_settings_type_;
   }
 
+  // Virtual to allow for mocking in tests.
+  virtual scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
+  GetSafeBrowsingDatabaseManager();

[raymes, 2016/12/19 00:07:41]

nit: virtuals have several downsides, e.g. the temptation for them to be
overridden by subclasses. I think the SetXForTest pattern tends to be a better
approach and it also makes it really clear that the function is only used in
testing.
e.g. in this case we would have the SafeBrowsingDatbaseManager as a member here
which gets lazily initialized when it's needed and we could have
SetSafeBrowsingDatabaseManagerForTest as a private member function which sets up
the variable in tests.

[meredithl, 2016/12/20 07:38:27]

Done.

+
  private:
   friend class PermissionContextBaseTests;