Start checking URLs using PVer4. Verdict not returned to client yet.
BUG=543161
Review-Url: https://codereview.chromium.org/2371043003
Cr-Commit-Position: refs/heads/master@{#421893}
diff --git a/chrome/browser/loader/safe_browsing_resource_throttle.cc b/chrome/browser/loader/safe_browsing_resource_throttle.cc
index b8a7c53..1ea436b8 100644
--- a/chrome/browser/loader/safe_browsing_resource_throttle.cc
+++ b/chrome/browser/loader/safe_browsing_resource_throttle.cc
@@ -15,6 +15,8 @@
#include "chrome/browser/prerender/prerender_contents.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "components/safe_browsing_db/util.h"
+#include "components/safe_browsing_db/v4_feature_list.h"
+#include "components/safe_browsing_db/v4_local_database_manager.h"
#include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_frame_host.h"
@@ -96,7 +98,8 @@
resource_type_(resource_type),
net_log_with_source_(
net::NetLogWithSource::Make(request->net_log().net_log(),
- NetLogSourceType::SAFE_BROWSING)) {}
+ NetLogSourceType::SAFE_BROWSING)),
+ v4_local_database_manager_(sb_service->v4_local_database_manager()) {}
SafeBrowsingResourceThrottle::~SafeBrowsingResourceThrottle() {
if (defer_state_ != DEFERRED_NONE) {
@@ -105,6 +108,10 @@
if (state_ == STATE_CHECKING_URL) {
database_manager_->CancelCheck(this);
+ if (safe_browsing::V4FeatureList::IsParallelCheckEnabled()) {
+ v4_local_database_manager_->CancelCheck(this);
+ }
+
EndNetLogEvent(NetLogEventType::SAFE_BROWSING_CHECKING_URL, "result",
"request_canceled");
}
@@ -373,6 +380,11 @@
UMA_HISTOGRAM_ENUMERATION("SB2.ResourceTypes2.Checked", resource_type_,
content::RESOURCE_TYPE_LAST_TYPE);
+ if (safe_browsing::V4FeatureList::IsParallelCheckEnabled() &&
+ v4_local_database_manager_->CanCheckResourceType(resource_type_)) {
+ v4_local_database_manager_->CheckBrowseUrl(url, this);
+ }
+
if (succeeded_synchronously) {
threat_type_ = safe_browsing::SB_THREAT_TYPE_SAFE;
ui_manager_->LogPauseDelay(base::TimeDelta()); // No delay.
@@ -398,6 +410,9 @@
CHECK_EQ(state_, STATE_CHECKING_URL);
database_manager_->CancelCheck(this);
+ if (safe_browsing::V4FeatureList::IsParallelCheckEnabled()) {
+ v4_local_database_manager_->CancelCheck(this);
+ }
OnCheckBrowseUrlResult(url_being_checked_, safe_browsing::SB_THREAT_TYPE_SAFE,
safe_browsing::ThreatMetadata());
}
diff --git a/chrome/browser/loader/safe_browsing_resource_throttle.h b/chrome/browser/loader/safe_browsing_resource_throttle.h
index 017a709..ba7924f 100644
--- a/chrome/browser/loader/safe_browsing_resource_throttle.h
+++ b/chrome/browser/loader/safe_browsing_resource_throttle.h
@@ -19,6 +19,7 @@
#include "content/public/common/resource_type.h"
#include "net/log/net_log.h"
#include "net/log/net_log_event_type.h"
+#include "url/gurl.h"
class ResourceDispatcherHost;
@@ -26,6 +27,10 @@
class URLRequest;
}
+namespace safe_browsing {
+class V4LocalDatabaseManager;
+}
+
// SafeBrowsingResourceThrottle checks that URLs are "safe" before
// navigating to them. To be considered "safe", a URL must not appear in the
// malware/phishing blacklists (see SafeBrowsingService for details).
@@ -178,6 +183,8 @@
const net::URLRequest* request_;
const content::ResourceType resource_type_;
net::NetLogWithSource net_log_with_source_;
+ scoped_refptr<safe_browsing::V4LocalDatabaseManager>
+ v4_local_database_manager_;
DISALLOW_COPY_AND_ASSIGN(SafeBrowsingResourceThrottle);
};
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 4083537..58ed668 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -400,6 +400,11 @@
return ping_manager_.get();
}
+const scoped_refptr<V4LocalDatabaseManager>&
+SafeBrowsingService::v4_local_database_manager() const {
+ return services_delegate_->v4_local_database_manager();
+}
+
std::unique_ptr<TrackedPreferenceValidationDelegate>
SafeBrowsingService::CreatePreferenceValidationDelegate(
Profile* profile) const {
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index 0ac36a33..4087626 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -131,6 +131,9 @@
SafeBrowsingPingManager* ping_manager() const;
+ const scoped_refptr<V4LocalDatabaseManager>& v4_local_database_manager()
+ const;
+
// Returns a preference validation delegate that adds incidents to the
// incident reporting service for validation failures. Returns NULL if the
// service is not applicable for the given profile.
diff --git a/chrome/browser/safe_browsing/services_delegate.h b/chrome/browser/safe_browsing/services_delegate.h
index 978ab771..2874137a 100644
--- a/chrome/browser/safe_browsing/services_delegate.h
+++ b/chrome/browser/safe_browsing/services_delegate.h
@@ -7,6 +7,7 @@
#include <memory>
+#include "base/memory/ref_counted.h"
#include "chrome/browser/safe_browsing/incident_reporting/delayed_analysis_callback.h"
#include "components/user_prefs/tracked/tracked_preference_validation_delegate.h"
@@ -28,6 +29,7 @@
class ResourceRequestDetector;
struct ResourceRequestInfo;
class SafeBrowsingService;
+class V4LocalDatabaseManager;
struct V4ProtocolConfig;
// Abstraction to help organize code for mobile vs full safe browsing modes.
@@ -66,6 +68,9 @@
virtual ~ServicesDelegate() {}
+ virtual const scoped_refptr<V4LocalDatabaseManager>&
+ v4_local_database_manager() const = 0;
+
// Initializes internal state using the ServicesCreator.
virtual void Initialize() = 0;
diff --git a/chrome/browser/safe_browsing/services_delegate_impl.cc b/chrome/browser/safe_browsing/services_delegate_impl.cc
index cb72d0e..2097437 100644
--- a/chrome/browser/safe_browsing/services_delegate_impl.cc
+++ b/chrome/browser/safe_browsing/services_delegate_impl.cc
@@ -8,7 +8,6 @@
#include "base/command_line.h"
#include "base/memory/ptr_util.h"
-#include "base/metrics/field_trial.h"
#include "base/strings/string_util.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/common/chrome_switches.h"
@@ -17,15 +16,6 @@
namespace safe_browsing {
-#ifdef NDEBUG
-namespace {
-const base::Feature kSafeBrowsingV4LocalDatabaseManagerEnabled {
- "SafeBrowsingV4LocalDatabaseManagerEnabled",
- base::FEATURE_DISABLED_BY_DEFAULT
-};
-} // namespace
-#endif
-
// static
std::unique_ptr<ServicesDelegate> ServicesDelegate::Create(
SafeBrowsingService* safe_browsing_service) {
@@ -64,6 +54,11 @@
#endif // defined(SAFE_BROWSING_CSD)
}
+const scoped_refptr<V4LocalDatabaseManager>&
+ServicesDelegateImpl::v4_local_database_manager() const {
+ return v4_local_database_manager_;
+}
+
void ServicesDelegateImpl::Initialize() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
download_service_.reset(
@@ -82,9 +77,8 @@
? services_creator_->CreateResourceRequestDetector()
: CreateResourceRequestDetector());
- if (IsV4LocalDatabaseManagerEnabled()) {
- v4_local_database_manager_ = CreateV4LocalDatabaseManager();
- }
+ v4_local_database_manager_ =
+ V4LocalDatabaseManager::Create(SafeBrowsingService::GetBaseFilename());
}
void ServicesDelegateImpl::ShutdownServices() {
@@ -181,17 +175,4 @@
}
}
-V4LocalDatabaseManager* ServicesDelegateImpl::CreateV4LocalDatabaseManager() {
- return new V4LocalDatabaseManager(SafeBrowsingService::GetBaseFilename());
-}
-
-bool ServicesDelegateImpl::IsV4LocalDatabaseManagerEnabled() {
-#ifndef NDEBUG
- return true;
-#else
- return base::FeatureList::IsEnabled(
- kSafeBrowsingV4LocalDatabaseManagerEnabled);
-#endif
-}
-
} // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/services_delegate_impl.h b/chrome/browser/safe_browsing/services_delegate_impl.h
index e861d25..a6608f4 100644
--- a/chrome/browser/safe_browsing/services_delegate_impl.h
+++ b/chrome/browser/safe_browsing/services_delegate_impl.h
@@ -28,6 +28,8 @@
private:
// ServicesDelegate:
+ const scoped_refptr<V4LocalDatabaseManager>& v4_local_database_manager()
+ const override;
void Initialize() override;
void InitializeCsdService(
net::URLRequestContextGetter* context_getter) override;
@@ -49,11 +51,6 @@
const V4ProtocolConfig& v4_config) override;
void StopOnIOThread(bool shutdown) override;
- // Is the Pver4 database manager enabled? Controlled by Finch.
- bool IsV4LocalDatabaseManagerEnabled();
-
- V4LocalDatabaseManager* CreateV4LocalDatabaseManager();
-
DownloadProtectionService* CreateDownloadProtectionService();
IncidentReportingService* CreateIncidentReportingService();
ResourceRequestDetector* CreateResourceRequestDetector();
diff --git a/chrome/browser/safe_browsing/services_delegate_stub.cc b/chrome/browser/safe_browsing/services_delegate_stub.cc
index 1abf0a4..72c45ce 100644
--- a/chrome/browser/safe_browsing/services_delegate_stub.cc
+++ b/chrome/browser/safe_browsing/services_delegate_stub.cc
@@ -6,6 +6,7 @@
#include "base/logging.h"
#include "base/memory/ptr_util.h"
+#include "components/safe_browsing_db/v4_local_database_manager.h"
namespace safe_browsing {
@@ -30,6 +31,11 @@
void ServicesDelegateStub::InitializeCsdService(
net::URLRequestContextGetter* context_getter) {}
+const scoped_refptr<V4LocalDatabaseManager>&
+ServicesDelegateStub::v4_local_database_manager() const {
+ return v4_local_database_manager_;
+}
+
void ServicesDelegateStub::Initialize() {}
void ServicesDelegateStub::ShutdownServices() {}
diff --git a/chrome/browser/safe_browsing/services_delegate_stub.h b/chrome/browser/safe_browsing/services_delegate_stub.h
index 07a326db..0ced25b 100644
--- a/chrome/browser/safe_browsing/services_delegate_stub.h
+++ b/chrome/browser/safe_browsing/services_delegate_stub.h
@@ -10,6 +10,8 @@
namespace safe_browsing {
+class V4LocalDatabaseManager;
+
// Dummy ServicesDelegate implementation. Create via ServicesDelegate::Create().
class ServicesDelegateStub : public ServicesDelegate {
public:
@@ -18,6 +20,8 @@
private:
// ServicesDelegate:
+ const scoped_refptr<V4LocalDatabaseManager>& v4_local_database_manager()
+ const override;
void Initialize() override;
void InitializeCsdService(
net::URLRequestContextGetter* context_getter) override;
@@ -39,6 +43,8 @@
const V4ProtocolConfig& v4_config) override;
void StopOnIOThread(bool shutdown) override;
+ scoped_refptr<V4LocalDatabaseManager> v4_local_database_manager_;
+
DISALLOW_COPY_AND_ASSIGN(ServicesDelegateStub);
};
diff --git a/components/safe_browsing_db/BUILD.gn b/components/safe_browsing_db/BUILD.gn
index bfdf563..e14da2e 100644
--- a/components/safe_browsing_db/BUILD.gn
+++ b/components/safe_browsing_db/BUILD.gn
@@ -41,7 +41,6 @@
deps = [
":safe_browsing_db_shared",
":v4_local_database_manager",
- ":v4_update_protocol_manager",
]
}
@@ -185,6 +184,16 @@
]
}
+static_library("v4_feature_list") {
+ sources = [
+ "v4_feature_list.cc",
+ "v4_feature_list.h",
+ ]
+ deps = [
+ "//base",
+ ]
+}
+
static_library("v4_get_hash_protocol_manager") {
sources = [
"v4_get_hash_protocol_manager.cc",
@@ -213,6 +222,7 @@
":hit_report",
":safebrowsing_proto",
":v4_database",
+ ":v4_feature_list",
":v4_get_hash_protocol_manager",
":v4_protocol_manager_util",
":v4_update_protocol_manager",
diff --git a/components/safe_browsing_db/v4_feature_list.cc b/components/safe_browsing_db/v4_feature_list.cc
new file mode 100644
index 0000000..7c0bf09
--- /dev/null
+++ b/components/safe_browsing_db/v4_feature_list.cc
@@ -0,0 +1,42 @@
+// Copyright 2016 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 "base/feature_list.h"
+#include "components/safe_browsing_db/v4_feature_list.h"
+
+namespace safe_browsing {
+
+namespace V4FeatureList {
+
+#ifdef NDEBUG
+namespace {
+const base::Feature kLocalDatabaseManagerEnabled{
+ "SafeBrowsingV4LocalDatabaseManagerEnabled",
+ base::FEATURE_DISABLED_BY_DEFAULT};
+
+const base::Feature kParallelCheckEnabled{"SafeBrowingV4ParallelCheckEnabled",
+ base::FEATURE_DISABLED_BY_DEFAULT};
+} // namespace
+#endif
+
+bool IsLocalDatabaseManagerEnabled() {
+#ifndef NDEBUG
+ return true;
+#else
+ return IsParallelCheckEnabled() ||
+ base::FeatureList::IsEnabled(kLocalDatabaseManagerEnabled);
+#endif
+}
+
+bool IsParallelCheckEnabled() {
+#ifndef NDEBUG
+ return true;
+#else
+ return base::FeatureList::IsEnabled(kParallelCheckEnabled);
+#endif
+}
+
+} // namespace V4FeatureList
+
+} // namespace safe_browsing
diff --git a/components/safe_browsing_db/v4_feature_list.h b/components/safe_browsing_db/v4_feature_list.h
new file mode 100644
index 0000000..b305541
--- /dev/null
+++ b/components/safe_browsing_db/v4_feature_list.h
@@ -0,0 +1,24 @@
+// Copyright 2016 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 COMPONENTS_SAFE_BROWSING_DB_V4_FEATURE_LIST_H_
+#define COMPONENTS_SAFE_BROWSING_DB_V4_FEATURE_LIST_H_
+
+namespace safe_browsing {
+
+// Exposes methods to check whether a particular feature has been enabled
+// through Finch.
+namespace V4FeatureList {
+
+// Is the Pver4 database manager enabled?
+bool IsLocalDatabaseManagerEnabled();
+
+// Is the Pver4 database manager doing resource checks in paralled with PVer3?
+bool IsParallelCheckEnabled();
+
+} // namespace V4FeatureList
+
+} // namespace safe_browsing
+
+#endif // COMPONENTS_SAFE_BROWSING_DB_V4_FEATURE_LIST_H_
diff --git a/components/safe_browsing_db/v4_local_database_manager.cc b/components/safe_browsing_db/v4_local_database_manager.cc
index 9215dc7..2832025 100644
--- a/components/safe_browsing_db/v4_local_database_manager.cc
+++ b/components/safe_browsing_db/v4_local_database_manager.cc
@@ -12,6 +12,8 @@
#include "base/bind_helpers.h"
#include "base/callback.h"
#include "base/memory/ptr_util.h"
+#include "base/memory/ref_counted.h"
+#include "components/safe_browsing_db/v4_feature_list.h"
#include "content/public/browser/browser_thread.h"
using content::BrowserThread;
@@ -64,6 +66,16 @@
V4LocalDatabaseManager::PendingCheck::~PendingCheck() {}
+// static
+scoped_refptr<V4LocalDatabaseManager> V4LocalDatabaseManager::Create(
+ const base::FilePath& base_path) {
+ if (!V4FeatureList::IsLocalDatabaseManagerEnabled()) {
+ return nullptr;
+ }
+
+ return make_scoped_refptr(new V4LocalDatabaseManager(base_path));
+}
+
V4LocalDatabaseManager::V4LocalDatabaseManager(const base::FilePath& base_path)
: base_path_(base_path), enabled_(false), list_infos_(GetListInfos()) {
DCHECK(!base_path_.empty());
diff --git a/components/safe_browsing_db/v4_local_database_manager.h b/components/safe_browsing_db/v4_local_database_manager.h
index f9510c4..c73db1b9 100644
--- a/components/safe_browsing_db/v4_local_database_manager.h
+++ b/components/safe_browsing_db/v4_local_database_manager.h
@@ -28,18 +28,10 @@
// SafeBrowsing service and interfaces with the protocol manager.
class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
public:
- enum class ClientCallbackType {
- // This represents the case when we're trying to determine if a URL is
- // unsafe from the following perspectives: Malware, Phishing, UwS.
- CHECK_BROWSE_URL = 0,
-
- // This should always be the last value.
- CHECK_MAX
- };
-
- // Construct V4LocalDatabaseManager.
- // Must be initialized by calling StartOnIOThread() before using.
- V4LocalDatabaseManager(const base::FilePath& base_path);
+ // Create and return an instance of V4LocalDatabaseManager, if Finch trial
+ // allows it; nullptr otherwise.
+ static scoped_refptr<V4LocalDatabaseManager> Create(
+ const base::FilePath& base_path);
//
// SafeBrowsingDatabaseManager implementation
@@ -74,6 +66,19 @@
//
protected:
+ // Construct V4LocalDatabaseManager.
+ // Must be initialized by calling StartOnIOThread() before using.
+ V4LocalDatabaseManager(const base::FilePath& base_path);
+
+ enum class ClientCallbackType {
+ // This represents the case when we're trying to determine if a URL is
+ // unsafe from the following perspectives: Malware, Phishing, UwS.
+ CHECK_BROWSE_URL = 0,
+
+ // This should always be the last value.
+ CHECK_MAX
+ };
+
// The information we need to process a URL safety reputation request and
// respond to the SafeBrowsing client that asked for it.
// TODO(vakh): In its current form, it only includes information for
