Implement embargo in PermissionDecisionAutoBlocker

chrome/browser/permissions/permission_context_base.cc

 // 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.
 
 #include "chrome/browser/permissions/permission_context_base.h"
 
 #include <stddef.h>
 
 #include <string>
 #include <utility>
 
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/time/time.h"

[dominickn, 2017/01/11 07:52:13]

This include isn't used

[meredithl, 2017/01/11 23:22:28]

Done.

 #include "build/build_config.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_decision_auto_blocker.h"
 #include "chrome/browser/permissions/permission_request.h"
 #include "chrome/browser/permissions/permission_request_id.h"
 #include "chrome/browser/permissions/permission_request_impl.h"
 #include "chrome/browser/permissions/permission_request_manager.h"
 #include "chrome/browser/permissions/permission_uma_util.h"
 #include "chrome/browser/permissions/permission_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/browser/website_settings_registry.h"
 #include "components/prefs/pref_service.h"
+#include "components/safe_browsing_db/database_manager.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/origin_util.h"
 #include "url/gurl.h"
 
 #if defined(OS_ANDROID)
 #include "chrome/browser/permissions/permission_queue_controller.h"
 #endif
@@ skipping 29 matching lines @@
 PermissionContextBase::~PermissionContextBase() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 }
 
 void PermissionContextBase::RequestPermission(
     content::WebContents* web_contents,
     const PermissionRequestID& id,
     const GURL& requesting_frame,
     bool user_gesture,
     const BrowserPermissionCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

[dominickn, 2017/01/11 07:52:13]

Nit: probably can leave this new line.

[meredithl, 2017/01/11 23:22:28]

Done.

-
   // First check if this permission has been disabled.
   if (IsPermissionKillSwitchOn()) {
     // Log to the developer console.
     web_contents->GetMainFrame()->AddMessageToConsole(
         content::CONSOLE_MESSAGE_LEVEL_LOG,
         base::StringPrintf(
             "%s permission has been blocked.",
             PermissionUtil::GetPermissionString(permission_type_).c_str()));
     // The kill switch is enabled for this permission; Block all requests.
     callback.Run(CONTENT_SETTING_BLOCK);
     return;
   }
 
   GURL requesting_origin = requesting_frame.GetOrigin();
   GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin();

[dominickn, 2017/01/11 07:52:13]

Nit: probably can leave this new line.

[meredithl, 2017/01/11 23:22:28]

Done.

-
   if (!requesting_origin.is_valid() || !embedding_origin.is_valid()) {
     std::string type_name =
         content_settings::WebsiteSettingsRegistry::GetInstance()
             ->Get(content_settings_type_)
             ->name();
 
     DVLOG(1) << "Attempt to use " << type_name
              << " from an invalid URL: " << requesting_origin << ","
              << embedding_origin << " (" << type_name
              << " is not supported in popups)";
     NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
                         false /* persist */, CONTENT_SETTING_BLOCK);
     return;
   }
 
-  if (base::FeatureList::IsEnabled(features::kPermissionsBlacklist)) {
-    if (!db_manager_) {
-      safe_browsing::SafeBrowsingService* sb_service =
-          g_browser_process->safe_browsing_service();
-      if (sb_service)
-        db_manager_ = sb_service->database_manager();
-    }
+  // Check the content setting first before autoblocking.

[dominickn, 2017/01/11 07:52:13]

Remove "before autoblocking" - it's a bit misleading.

Instead, write it out in more detail:

1. Synchronously check the content setting to see if the user has made a
decision, or if the origin is under embargo. If so, respect that decision.

2. Asynchronously check whether the origin should be blocked from making a
request. It may be on the Safe Browsing API blacklist, or it may have been
dismissed too many times in a row. If the origin is allowed to request, that
request will be made in ContinueRequestPermission

[meredithl, 2017/01/11 23:22:28]

Done.

+  ContentSetting content_setting =
+      GetPermissionStatus(requesting_origin, embedding_origin);
+  if (content_setting == CONTENT_SETTING_ALLOW) {
+    HostContentSettingsMapFactory::GetForProfile(profile_)->UpdateLastUsage(
+        requesting_origin, embedding_origin, content_settings_type_);
+  }
 
-    // The client contacts Safe Browsing, and runs the callback with the result.
-    PermissionBlacklistClient::CheckSafeBrowsingBlacklist(
-        db_manager_, permission_type_, requesting_origin, web_contents,
-        safe_browsing_timeout_,
-        base::Bind(&PermissionContextBase::ContinueRequestPermission,
-                   weak_factory_.GetWeakPtr(), web_contents, id,
-                   requesting_origin, embedding_origin, user_gesture,
-                   callback));
-  } else {
-    // TODO(meredithl): Add UMA metrics here.
-    ContinueRequestPermission(web_contents, id, requesting_origin,
-                              embedding_origin, user_gesture, callback,
-                              false /* permission blocked */);
+  if (content_setting == CONTENT_SETTING_ALLOW ||
+      content_setting == CONTENT_SETTING_BLOCK) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, content_setting);
+    return;
   }
+
+  if (!db_manager_) {
+    safe_browsing::SafeBrowsingService* sb_service =
+        g_browser_process->safe_browsing_service();
+    if (sb_service)
+      db_manager_ = sb_service->database_manager();
+  }
+
+  PermissionDecisionAutoBlocker::ShouldAutomaticallyBlock(

[dominickn, 2017/01/11 07:52:13]

db_manager_ could still be null here (e.g. if sb_service is null). I guess
ShouldAutomaticallyBlock does null-check it before using it, and your TODO
covers moving this from here into the auto blocker (i.e. changing all the tests)

[meredithl, 2017/01/11 23:22:28]

For now should I move this inside an if statement that does a null check?

[dominickn, 2017/01/12 00:22:33]

It should be fine given that this will move soon (and the consumer does a null
check)

+      db_manager_, permission_type_, requesting_origin, web_contents,
+      safe_browsing_timeout_, profile_, base::Time::Now(),
+      base::Bind(&PermissionContextBase::ContinueRequestPermission,
+                 weak_factory_.GetWeakPtr(), web_contents, id,
+                 requesting_origin, embedding_origin, user_gesture, callback));
 }
 
 void PermissionContextBase::ContinueRequestPermission(
     content::WebContents* web_contents,
     const PermissionRequestID& id,
     const GURL& requesting_origin,
     const GURL& embedding_origin,
     bool user_gesture,
     const BrowserPermissionCallback& callback,
     bool permission_blocked) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (permission_blocked) {
     // TODO(meredithl): Add UMA metrics here.
     web_contents->GetMainFrame()->AddMessageToConsole(
         content::CONSOLE_MESSAGE_LEVEL_LOG,
         base::StringPrintf(
             "%s permission has been auto-blocked.",
             PermissionUtil::GetPermissionString(permission_type_).c_str()));
     // Permission has been blacklisted, block the request.
     // TODO(meredithl): Consider setting the content setting and persisting
     // the decision to block.
     callback.Run(CONTENT_SETTING_BLOCK);
     return;
   }
+
   // Site is not blacklisted by Safe Browsing for the requested permission.
-  ContentSetting content_setting =
-      GetPermissionStatus(requesting_origin, embedding_origin);
-  if (content_setting == CONTENT_SETTING_ALLOW) {
-    HostContentSettingsMapFactory::GetForProfile(profile_)->UpdateLastUsage(
-        requesting_origin, embedding_origin, content_settings_type_);
-  }
-
-  if (content_setting == CONTENT_SETTING_ALLOW ||
-      content_setting == CONTENT_SETTING_BLOCK) {
-    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
-                        false /* persist */, content_setting);
-    return;
-  }
-
   PermissionUmaUtil::PermissionRequested(permission_type_, requesting_origin,
                                          embedding_origin, profile_);
 
   DecidePermission(web_contents, id, requesting_origin, embedding_origin,
                    user_gesture, callback);
 }
 
 ContentSetting PermissionContextBase::GetPermissionStatus(
     const GURL& requesting_origin,
     const GURL& embedding_origin) const {
   // If the permission has been disabled through Finch, block all requests.
   if (IsPermissionKillSwitchOn())
     return CONTENT_SETTING_BLOCK;
 
   if (IsRestrictedToSecureOrigins() &&
       !content::IsOriginSecure(requesting_origin)) {
     return CONTENT_SETTING_BLOCK;
   }
 
-  return GetPermissionStatusInternal(requesting_origin, embedding_origin);
+  ContentSetting content_setting =
+      GetPermissionStatusInternal(requesting_origin, embedding_origin);
+  if (content_setting == CONTENT_SETTING_ASK) {

[dominickn, 2017/01/11 07:52:13]

Nit: combine the  two if statements using &&

[meredithl, 2017/01/11 23:22:28]

Done.

+    if (PermissionDecisionAutoBlocker::IsUnderEmbargo(
+            permission_type_, profile_, requesting_origin, base::Time::Now())) {
+      return CONTENT_SETTING_BLOCK;
+    }
+  }
+  return content_setting;
 }
 
 void PermissionContextBase::ResetPermission(
     const GURL& requesting_origin,
     const GURL& embedding_origin) {
   HostContentSettingsMapFactory::GetForProfile(profile_)
       ->SetContentSettingDefaultScope(requesting_origin, embedding_origin,
                                       content_settings_type_, std::string(),
                                       CONTENT_SETTING_DEFAULT);
 }
@@ skipping 178 matching lines @@
                                       content_settings_type_, std::string(),
                                       content_setting);
 }
 
 void PermissionContextBase::SetSafeBrowsingDatabaseManagerAndTimeoutForTest(
     scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
     int timeout) {
   db_manager_ = db_manager;
   safe_browsing_timeout_ = timeout;
 }