Implement origin specific Permissions Blacklisting.

chrome/browser/permissions/permission_context_base.h

 // Copyright 2014 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.
 
 #ifndef CHROME_BROWSER_PERMISSIONS_PERMISSION_CONTEXT_BASE_H_
 #define CHROME_BROWSER_PERMISSIONS_PERMISSION_CONTEXT_BASE_H_
 
 #include <memory>
 
 #include "base/callback_forward.h"
@@ skipping 10 matching lines @@
 class PermissionQueueController;
 #endif
 class GURL;
 class PermissionRequestID;
 class Profile;
 
 namespace content {
 class WebContents;
 }
 
+namespace safe_browsing {
+class SafeBrowsingDatabaseManager;
+}
+
 using BrowserPermissionCallback = base::Callback<void(ContentSetting)>;
 
 // This base class contains common operations for granting permissions.
 // It offers the following functionality:
 //   - Creates a permission request when needed.
 //   - If accepted/denied the permission is saved in content settings for
 //     future uses (for the domain that requested it).
 //   - If dismissed the permission is not saved but it's considered denied for
 //     this one request
 //   - In any case the BrowserPermissionCallback is executed once a decision
@@ skipping 32 matching lines @@
 
   // The renderer is requesting permission to push messages.
   // When the answer to a permission request has been determined, |callback|
   // should be called with the result.
   virtual void RequestPermission(content::WebContents* web_contents,
                                  const PermissionRequestID& id,
                                  const GURL& requesting_frame,
                                  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;
 
   // Resets the permission to its default value.
   virtual void ResetPermission(const GURL& requesting_origin,
                                const GURL& embedding_origin);
 
   // Withdraw an existing permission request, no op if the permission request
   // was already cancelled by some other means.
@@ skipping 54 matching lines @@
                                     ContentSetting content_setting);
 
   // Whether the permission should be restricted to secure origins.
   virtual bool IsRestrictedToSecureOrigins() const = 0;
 
   content::PermissionType permission_type() const { return permission_type_; }
   ContentSettingsType content_settings_type() const {
     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;
 
   // Called when a request is no longer used so it can be cleaned up.
   void CleanUpRequest(const PermissionRequestID& id);
 
   Profile* profile_;
   const content::PermissionType permission_type_;
   const ContentSettingsType content_settings_type_;
 #if defined(OS_ANDROID)
   std::unique_ptr<PermissionQueueController> permission_queue_controller_;
 #endif
   base::ScopedPtrHashMap<std::string, std::unique_ptr<PermissionRequest>>
       pending_requests_;
 
   // Must be the last member, to ensure that it will be
   // destroyed first, which will invalidate weak pointers
   base::WeakPtrFactory<PermissionContextBase> weak_factory_;
 };
 
 #endif  // CHROME_BROWSER_PERMISSIONS_PERMISSION_CONTEXT_BASE_H_