Refactor blacklist client into own .h/.cc.
The size of the Safe Browsing client class was getting to be too large
to be contained entirely within PermissionContextBase, so it has been
moved to its own header and implementation.
Test coverage is supplied in permission_context_base_unitttest.cc, but
there will be follow up unit tests for this class.
BUG=561867
Review-Url: https://codereview.chromium.org/2626853002
Cr-Commit-Position: refs/heads/master@{#442805}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 56e55e4..ee3e743d 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -826,6 +826,8 @@
"permissions/chooser_context_base.h",
"permissions/delegation_tracker.cc",
"permissions/delegation_tracker.h",
+ "permissions/permission_blacklist_client.cc",
+ "permissions/permission_blacklist_client.h",
"permissions/permission_context_base.cc",
"permissions/permission_context_base.h",
"permissions/permission_decision_auto_blocker.cc",
diff --git a/chrome/browser/permissions/permission_blacklist_client.cc b/chrome/browser/permissions/permission_blacklist_client.cc
new file mode 100644
index 0000000..18f57460
--- /dev/null
+++ b/chrome/browser/permissions/permission_blacklist_client.cc
@@ -0,0 +1,102 @@
+// Copyright 2017 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_blacklist_client.h"
+
+#include <set>
+#include <string>
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/timer/timer.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
+#include "url/gurl.h"
+
+// static
+void PermissionBlacklistClient::CheckSafeBrowsingBlacklist(
+ scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
+ content::PermissionType permission_type,
+ const GURL& request_origin,
+ content::WebContents* web_contents,
+ int timeout,
+ base::Callback<void(bool)> callback) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+ new PermissionBlacklistClient(db_manager, permission_type, request_origin,
+ web_contents, timeout, callback);
+}
+
+PermissionBlacklistClient::PermissionBlacklistClient(
+ scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
+ content::PermissionType permission_type,
+ const GURL& request_origin,
+ content::WebContents* web_contents,
+ int timeout,
+ base::Callback<void(bool)> callback)
+ : content::WebContentsObserver(web_contents),
+ db_manager_(db_manager),
+ permission_type_(permission_type),
+ callback_(callback),
+ timeout_(timeout),
+ is_active_(true) {
+ // Balanced by a call to Release() in OnCheckApiBlacklistUrlResult().
+ AddRef();
+ content::BrowserThread::PostTask(
+ content::BrowserThread::IO, FROM_HERE,
+ base::Bind(&PermissionBlacklistClient::StartCheck, this, request_origin));
+}
+
+PermissionBlacklistClient::~PermissionBlacklistClient() {}
+
+void PermissionBlacklistClient::StartCheck(const GURL& request_origin) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+ // Start the timer to interrupt into the client callback method with an
+ // empty response if Safe Browsing times out.
+ safe_browsing::ThreatMetadata empty_metadata;
+ timer_ = base::MakeUnique<base::OneShotTimer>();
+ timer_->Start(
+ FROM_HERE, base::TimeDelta::FromMilliseconds(timeout_),
+ base::Bind(&PermissionBlacklistClient::OnCheckApiBlacklistUrlResult, this,
+ request_origin, empty_metadata));
+ db_manager_->CheckApiBlacklistUrl(request_origin, this);
+}
+
+void PermissionBlacklistClient::OnCheckApiBlacklistUrlResult(
+ const GURL& url,
+ const safe_browsing::ThreatMetadata& metadata) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+ if (timer_->IsRunning())
+ timer_->Stop();
+ else
+ db_manager_->CancelApiCheck(this);
+ timer_.reset(nullptr);
+
+ // TODO(meredithl): Convert the strings returned from Safe Browsing to the
+ // ones used by PermissionUtil for comparison.
+ bool permission_blocked =
+ metadata.api_permissions.find(PermissionUtil::GetPermissionString(
+ permission_type_)) != metadata.api_permissions.end();
+
+ content::BrowserThread::PostTask(
+ content::BrowserThread::UI, FROM_HERE,
+ base::Bind(&PermissionBlacklistClient::EvaluateBlacklistResultOnUiThread,
+ this, permission_blocked));
+}
+
+void PermissionBlacklistClient::EvaluateBlacklistResultOnUiThread(
+ bool permission_blocked) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+ if (is_active_)
+ callback_.Run(permission_blocked);
+ Release();
+}
+
+void PermissionBlacklistClient::WebContentsDestroyed() {
+ is_active_ = false;
+ Observe(nullptr);
+}
diff --git a/chrome/browser/permissions/permission_blacklist_client.h b/chrome/browser/permissions/permission_blacklist_client.h
new file mode 100644
index 0000000..2d1666e0
--- /dev/null
+++ b/chrome/browser/permissions/permission_blacklist_client.h
@@ -0,0 +1,87 @@
+// Copyright 2017 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_BLACKLIST_CLIENT_H_
+#define CHROME_BROWSER_PERMISSIONS_PERMISSION_BLACKLIST_CLIENT_H_
+
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "chrome/browser/permissions/permission_util.h"
+#include "components/safe_browsing_db/database_manager.h"
+#include "content/public/browser/permission_type.h"
+#include "content/public/browser/web_contents_observer.h"
+
+class GURL;
+
+namespace content {
+class WebContents;
+}
+
+namespace base {
+class OneShotTimer;
+}
+
+// The client used when checking whether a permission has been blacklisted by
+// Safe Browsing. The check is done asynchronously as no state can be stored in
+// PermissionContextBase (since additional permission requests may be made).
+// This class must be created and destroyed on the UI thread.
+class PermissionBlacklistClient
+ : public safe_browsing::SafeBrowsingDatabaseManager::Client,
+ public base::RefCountedThreadSafe<PermissionBlacklistClient>,
+ public content::WebContentsObserver {
+ public:
+ static void CheckSafeBrowsingBlacklist(
+ scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
+ content::PermissionType permission_type,
+ const GURL& request_origin,
+ content::WebContents* web_contents,
+ int timeout,
+ base::Callback<void(bool)> callback);
+
+ private:
+ friend class base::RefCountedThreadSafe<PermissionBlacklistClient>;
+
+ PermissionBlacklistClient(
+ scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
+ content::PermissionType permission_type,
+ const GURL& request_origin,
+ content::WebContents* web_contents,
+ int timeout,
+ base::Callback<void(bool)> callback);
+
+ ~PermissionBlacklistClient() override;
+
+ void StartCheck(const GURL& request_origin);
+
+ // SafeBrowsingDatabaseManager::Client implementation.
+ void OnCheckApiBlacklistUrlResult(
+ const GURL& url,
+ const safe_browsing::ThreatMetadata& metadata) override;
+
+ void EvaluateBlacklistResultOnUiThread(bool permission_blocked);
+
+ // WebContentsObserver implementation. Sets a flag so that when the database
+ // manager returns with a result, it won't attempt to run the callback with a
+ // deleted WebContents.
+ void WebContentsDestroyed() override;
+
+ scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager_;
+ content::PermissionType permission_type_;
+
+ // PermissionContextBase callback to run on the UI thread.
+ base::Callback<void(bool)> callback_;
+
+ // Timer to abort the Safe Browsing check if it takes too long. Created and
+ // used on the IO Thread.
+ std::unique_ptr<base::OneShotTimer> timer_;
+ int timeout_;
+
+ // True if |callback_| should be invoked, if web_contents() is destroyed, this
+ // is set to false.
+ bool is_active_;
+
+ DISALLOW_COPY_AND_ASSIGN(PermissionBlacklistClient);
+};
+
+#endif // CHROME_BROWSER_PERMISSIONS_PERMISSION_BLACKLIST_CLIENT_H_
diff --git a/chrome/browser/permissions/permission_context_base.cc b/chrome/browser/permissions/permission_context_base.cc
index 8911b967..753134c3 100644
--- a/chrome/browser/permissions/permission_context_base.cc
+++ b/chrome/browser/permissions/permission_context_base.cc
@@ -6,7 +6,6 @@
#include <stddef.h>
-#include <set>
#include <string>
#include <utility>
@@ -14,10 +13,10 @@
#include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "base/strings/stringprintf.h"
-#include "base/timer/timer.h"
#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"
@@ -32,12 +31,10 @@
#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/browser/web_contents_observer.h"
#include "content/public/common/origin_util.h"
#include "url/gurl.h"
@@ -57,128 +54,6 @@
// TODO(meredithl): Revisit this once UMA metrics have data about request time.
const int kCheckUrlTimeoutMs = 2000;
-// The client used when checking whether a permission has been blacklisted by
-// Safe Browsing. The check is done asynchronously as no state can be stored in
-// PermissionContextBase (since additional permission requests may be made).
-// This class must be created and destroyed on the UI thread.
-class PermissionsBlacklistingClient
- : public safe_browsing::SafeBrowsingDatabaseManager::Client,
- public base::RefCountedThreadSafe<PermissionsBlacklistingClient>,
- public content::WebContentsObserver {
- public:
- static void CheckSafeBrowsingBlacklist(
- scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
- content::PermissionType permission_type,
- const GURL& request_origin,
- content::WebContents* web_contents,
- int timeout,
- base::Callback<void(bool)> callback) {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
- new PermissionsBlacklistingClient(db_manager, permission_type,
- request_origin, web_contents, timeout,
- callback);
- }
-
- private:
- friend class base::RefCountedThreadSafe<PermissionsBlacklistingClient>;
-
- PermissionsBlacklistingClient(
- scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
- content::PermissionType permission_type,
- const GURL& request_origin,
- content::WebContents* web_contents,
- int timeout,
- base::Callback<void(bool)> callback)
- : content::WebContentsObserver(web_contents),
- db_manager_(db_manager),
- permission_type_(permission_type),
- callback_(callback),
- timeout_(timeout),
- is_active_(true) {
- // Balanced by a call to Release() in OnCheckApiBlacklistUrlResult().
- AddRef();
- content::BrowserThread::PostTask(
- content::BrowserThread::IO, FROM_HERE,
- base::Bind(&PermissionsBlacklistingClient::StartCheck, this,
- request_origin));
- }
-
- ~PermissionsBlacklistingClient() override {}
-
- void StartCheck(const GURL& request_origin) {
- DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-
- // Start the timer to interrupt into the client callback method with an
- // empty response if Safe Browsing times out.
- safe_browsing::ThreatMetadata empty_metadata;
- timer_ = base::MakeUnique<base::OneShotTimer>();
- timer_->Start(
- FROM_HERE, base::TimeDelta::FromMilliseconds(timeout_),
- base::Bind(&PermissionsBlacklistingClient::OnCheckApiBlacklistUrlResult,
- this, request_origin, empty_metadata));
- db_manager_->CheckApiBlacklistUrl(request_origin, this);
- }
-
- // SafeBrowsingDatabaseManager::Client implementation.
- void OnCheckApiBlacklistUrlResult(
- const GURL& url,
- const safe_browsing::ThreatMetadata& metadata) override {
- DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-
- if (timer_->IsRunning())
- timer_->Stop();
- else
- db_manager_->CancelApiCheck(this);
- timer_.reset(nullptr);
-
- // TODO(meredithl): Convert the strings returned from Safe Browsing to the
- // ones used by PermissionUtil for comparison.
- bool permission_blocked =
- metadata.api_permissions.find(PermissionUtil::GetPermissionString(
- permission_type_)) != metadata.api_permissions.end();
-
- content::BrowserThread::PostTask(
- content::BrowserThread::UI, FROM_HERE,
- base::Bind(
- &PermissionsBlacklistingClient::EvaluateBlacklistResultOnUiThread,
- this, permission_blocked));
- }
-
- void EvaluateBlacklistResultOnUiThread(bool permission_blocked) {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
- if (is_active_)
- callback_.Run(permission_blocked);
- Release();
- }
-
- // WebContentsObserver implementation. Sets the flag so that when the database
- // manager returns with a result, it won't attempt to run the callback with a
- // deleted WebContents.
- void WebContentsDestroyed() override {
- is_active_ = false;
- Observe(nullptr);
- }
-
- scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager_;
- content::PermissionType permission_type_;
-
- // PermissionContextBase callback to run on the UI thread.
- base::Callback<void(bool)> callback_;
-
- // Timer to abort the Safe Browsing check if it takes too long. Created and
- // used on the IO Thread.
- std::unique_ptr<base::OneShotTimer> timer_;
- int timeout_;
-
- // True if |callback_| should be invoked, if web_contents() is destroyed, this
- // is set to false.
- bool is_active_;
-
- DISALLOW_COPY_AND_ASSIGN(PermissionsBlacklistingClient);
-};
-
PermissionContextBase::PermissionContextBase(
Profile* profile,
const content::PermissionType permission_type,
@@ -247,7 +122,7 @@
}
// The client contacts Safe Browsing, and runs the callback with the result.
- PermissionsBlacklistingClient::CheckSafeBrowsingBlacklist(
+ PermissionBlacklistClient::CheckSafeBrowsingBlacklist(
db_manager_, permission_type_, requesting_origin, web_contents,
safe_browsing_timeout_,
base::Bind(&PermissionContextBase::ContinueRequestPermission,
