Commit 724a3a74 authored by Alex Turner's avatar Alex Turner Committed by Commit Bot

Migrate chrome/browser/safe_browsing base::Bind -> Once/Repeating (2/2)

base::Bind, base::Callback and base::Closure are deprecated and should
be replaced with the more explicit base::Bind{Once,Repeating},
base::{Once,Repeating}Callback and base::{Once,Repeating}Closure.

This cl finishes the conversion started by crrev.com/c/2493044 and
removes the directory from the legacy exception list in PRESUBMIT.py.

Bug: 1141564
Change-Id: Iaac5f197d740c806ded30ddbaea5c32015a118a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518030Reviewed-by: default avatarDaniel Rubery <drubery@chromium.org>
Reviewed-by: default avatarJochen Eisinger <jochen@chromium.org>
Commit-Queue: Alex Turner <alexmt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824136}
parent 1f391d4b
...@@ -356,7 +356,6 @@ _NOT_CONVERTED_TO_MODERN_BIND_AND_CALLBACK = '|'.join(( ...@@ -356,7 +356,6 @@ _NOT_CONVERTED_TO_MODERN_BIND_AND_CALLBACK = '|'.join((
'^chrome/browser/resource_coordinator/', '^chrome/browser/resource_coordinator/',
'^chrome/browser/resources/chromeos/accessibility/', '^chrome/browser/resources/chromeos/accessibility/',
'^chrome/browser/rlz/chrome_rlz_tracker_delegate.cc', '^chrome/browser/rlz/chrome_rlz_tracker_delegate.cc',
'^chrome/browser/safe_browsing/',
'^chrome/browser/search_engines/', '^chrome/browser/search_engines/',
'^chrome/browser/service_process/', '^chrome/browser/service_process/',
'^chrome/browser/signin/', '^chrome/browser/signin/',
......
...@@ -147,8 +147,8 @@ CertificateReportingService::Reporter::GetQueueForTesting() const { ...@@ -147,8 +147,8 @@ CertificateReportingService::Reporter::GetQueueForTesting() const {
} }
void CertificateReportingService::Reporter:: void CertificateReportingService::Reporter::
SetClosureWhenNoInflightReportsForTesting(const base::Closure& closure) { SetClosureWhenNoInflightReportsForTesting(base::OnceClosure closure) {
no_in_flight_reports_ = closure; no_in_flight_reports_ = std::move(closure);
} }
void CertificateReportingService::Reporter::SendInternal( void CertificateReportingService::Reporter::SendInternal(
...@@ -176,14 +176,14 @@ void CertificateReportingService::Reporter::ErrorCallback( ...@@ -176,14 +176,14 @@ void CertificateReportingService::Reporter::ErrorCallback(
} }
CHECK_GT(inflight_reports_.erase(report_id), 0u); CHECK_GT(inflight_reports_.erase(report_id), 0u);
if (inflight_reports_.empty() && no_in_flight_reports_) if (inflight_reports_.empty() && no_in_flight_reports_)
no_in_flight_reports_.Run(); std::move(no_in_flight_reports_).Run();
} }
void CertificateReportingService::Reporter::SuccessCallback(int report_id) { void CertificateReportingService::Reporter::SuccessCallback(int report_id) {
RecordUMAEvent(ReportOutcome::SUCCESSFUL); RecordUMAEvent(ReportOutcome::SUCCESSFUL);
CHECK_GT(inflight_reports_.erase(report_id), 0u); CHECK_GT(inflight_reports_.erase(report_id), 0u);
if (inflight_reports_.empty() && no_in_flight_reports_) if (inflight_reports_.empty() && no_in_flight_reports_)
no_in_flight_reports_.Run(); std::move(no_in_flight_reports_).Run();
} }
CertificateReportingService::CertificateReportingService( CertificateReportingService::CertificateReportingService(
......
...@@ -138,8 +138,7 @@ class CertificateReportingService : public KeyedService { ...@@ -138,8 +138,7 @@ class CertificateReportingService : public KeyedService {
size_t inflight_report_count_for_testing() const; size_t inflight_report_count_for_testing() const;
BoundedReportList* GetQueueForTesting() const; BoundedReportList* GetQueueForTesting() const;
// Sets a closure that is called when there are no more inflight reports. // Sets a closure that is called when there are no more inflight reports.
void SetClosureWhenNoInflightReportsForTesting( void SetClosureWhenNoInflightReportsForTesting(base::OnceClosure closure);
const base::Closure& closure);
private: private:
void SendInternal(const Report& report); void SendInternal(const Report& report);
...@@ -164,7 +163,7 @@ class CertificateReportingService : public KeyedService { ...@@ -164,7 +163,7 @@ class CertificateReportingService : public KeyedService {
std::map<int, Report> inflight_reports_; std::map<int, Report> inflight_reports_;
base::Closure no_in_flight_reports_; base::OnceClosure no_in_flight_reports_;
base::WeakPtrFactory<Reporter> weak_factory_{this}; base::WeakPtrFactory<Reporter> weak_factory_{this};
...@@ -183,7 +182,7 @@ class CertificateReportingService : public KeyedService { ...@@ -183,7 +182,7 @@ class CertificateReportingService : public KeyedService {
size_t max_queued_report_count, size_t max_queued_report_count,
base::TimeDelta max_report_age, base::TimeDelta max_report_age,
base::Clock* clock, base::Clock* clock,
const base::Callback<void()>& reset_callback); const base::RepeatingClosure& reset_callback);
~CertificateReportingService() override; ~CertificateReportingService() override;
...@@ -222,7 +221,7 @@ class CertificateReportingService : public KeyedService { ...@@ -222,7 +221,7 @@ class CertificateReportingService : public KeyedService {
// Subscription for state changes. When this subscription is notified, it // Subscription for state changes. When this subscription is notified, it
// means SafeBrowsingService is enabled/disabled or one of the preferences // means SafeBrowsingService is enabled/disabled or one of the preferences
// related to it is changed. // related to it is changed.
std::unique_ptr<base::CallbackList<void(void)>::Subscription> std::unique_ptr<base::RepeatingClosureList::Subscription>
safe_browsing_state_subscription_; safe_browsing_state_subscription_;
// Maximum number of reports to be queued for retry. // Maximum number of reports to be queued for retry.
...@@ -236,7 +235,7 @@ class CertificateReportingService : public KeyedService { ...@@ -236,7 +235,7 @@ class CertificateReportingService : public KeyedService {
base::Clock* const clock_; base::Clock* const clock_;
// Called when the service is reset. Used for testing. // Called when the service is reset. Used for testing.
base::Callback<void()> reset_callback_; base::RepeatingClosure reset_callback_;
// Encryption parameters. // Encryption parameters.
uint8_t* server_public_key_; uint8_t* server_public_key_;
......
...@@ -66,7 +66,7 @@ void CertificateReportingServiceFactory::SetMaxQueuedReportCountForTesting( ...@@ -66,7 +66,7 @@ void CertificateReportingServiceFactory::SetMaxQueuedReportCountForTesting(
} }
void CertificateReportingServiceFactory::SetServiceResetCallbackForTesting( void CertificateReportingServiceFactory::SetServiceResetCallbackForTesting(
const base::Callback<void()>& service_reset_callback) { const base::RepeatingClosure& service_reset_callback) {
service_reset_callback_ = service_reset_callback; service_reset_callback_ = service_reset_callback;
} }
......
...@@ -60,7 +60,7 @@ class CertificateReportingServiceFactory ...@@ -60,7 +60,7 @@ class CertificateReportingServiceFactory
base::Clock* clock_; base::Clock* clock_;
base::TimeDelta queued_report_ttl_; base::TimeDelta queued_report_ttl_;
size_t max_queued_report_count_; size_t max_queued_report_count_;
base::Callback<void()> service_reset_callback_; base::RepeatingClosure service_reset_callback_;
scoped_refptr<network::SharedURLLoaderFactory> test_url_loader_factory_; scoped_refptr<network::SharedURLLoaderFactory> test_url_loader_factory_;
DISALLOW_COPY_AND_ASSIGN(CertificateReportingServiceFactory); DISALLOW_COPY_AND_ASSIGN(CertificateReportingServiceFactory);
......
...@@ -523,7 +523,7 @@ int MockChromeCleanerProcess::Run() { ...@@ -523,7 +523,7 @@ int MockChromeCleanerProcess::Run() {
// task to unblock the child process's main thread. // task to unblock the child process's main thread.
auto quit_closure = base::BindOnce( auto quit_closure = base::BindOnce(
[](scoped_refptr<base::SequencedTaskRunner> main_runner, [](scoped_refptr<base::SequencedTaskRunner> main_runner,
base::Closure quit_closure) { base::OnceClosure quit_closure) {
main_runner->PostTask(FROM_HERE, std::move(quit_closure)); main_runner->PostTask(FROM_HERE, std::move(quit_closure));
}, },
base::SequencedTaskRunnerHandle::Get(), run_loop.QuitClosure()); base::SequencedTaskRunnerHandle::Get(), run_loop.QuitClosure());
......
...@@ -36,13 +36,13 @@ using ::testing::StrictMock; ...@@ -36,13 +36,13 @@ using ::testing::StrictMock;
// Callback for CreateProfile() that assigns |profile| to |*out_profile| // Callback for CreateProfile() that assigns |profile| to |*out_profile|
// if the profile creation is successful. // if the profile creation is successful.
void CreateProfileCallback(Profile** out_profile, void CreateProfileCallback(Profile** out_profile,
const base::Closure& closure, base::OnceClosure closure,
Profile* profile, Profile* profile,
Profile::CreateStatus status) { Profile::CreateStatus status) {
DCHECK(out_profile); DCHECK(out_profile);
if (status == Profile::CREATE_STATUS_INITIALIZED) if (status == Profile::CREATE_STATUS_INITIALIZED)
*out_profile = profile; *out_profile = profile;
closure.Run(); std::move(closure).Run();
} }
// Creates a new profile from the UI thread. // Creates a new profile from the UI thread.
......
...@@ -581,8 +581,8 @@ class ChromePasswordProtectionService : public PasswordProtectionService { ...@@ -581,8 +581,8 @@ class ChromePasswordProtectionService : public PasswordProtectionService {
// Subscription for state changes. When this subscription is notified, it // Subscription for state changes. When this subscription is notified, it
// means HashPasswordManager password data list has changed. // means HashPasswordManager password data list has changed.
std::unique_ptr< std::unique_ptr<base::RepeatingCallbackList<void(
base::CallbackList<void(const std::string& username)>::Subscription> const std::string& username)>::Subscription>
hash_password_manager_subscription_; hash_password_manager_subscription_;
// Reference to the current profile's VerdictCacheManager. This is unowned. // Reference to the current profile's VerdictCacheManager. This is unowned.
......
...@@ -460,14 +460,15 @@ void ClientSideDetectionHost::PhishingDetectionDone( ...@@ -460,14 +460,15 @@ void ClientSideDetectionHost::PhishingDetectionDone(
// We only send phishing verdict to the server if the verdict is phishing. // We only send phishing verdict to the server if the verdict is phishing.
if (verdict->is_phishing()) { if (verdict->is_phishing()) {
ClientSideDetectionService::ClientReportPhishingRequestCallback callback = ClientSideDetectionService::ClientReportPhishingRequestCallback callback =
base::Bind(&ClientSideDetectionHost::MaybeShowPhishingWarning, base::BindOnce(&ClientSideDetectionHost::MaybeShowPhishingWarning,
weak_factory_.GetWeakPtr(), weak_factory_.GetWeakPtr(),
/*is_from_cache=*/false); /*is_from_cache=*/false);
Profile* profile = Profile* profile =
Profile::FromBrowserContext(web_contents()->GetBrowserContext()); Profile::FromBrowserContext(web_contents()->GetBrowserContext());
csd_service_->SendClientReportPhishingRequest( csd_service_->SendClientReportPhishingRequest(
std::move(verdict), IsExtendedReportingEnabled(*profile->GetPrefs()), std::move(verdict), IsExtendedReportingEnabled(*profile->GetPrefs()),
IsEnhancedProtectionEnabled(*profile->GetPrefs()), callback); IsEnhancedProtectionEnabled(*profile->GetPrefs()),
std::move(callback));
} }
} }
} }
......
...@@ -32,15 +32,18 @@ class FakeClientSideDetectionService : public ClientSideDetectionService { ...@@ -32,15 +32,18 @@ class FakeClientSideDetectionService : public ClientSideDetectionService {
std::unique_ptr<ClientPhishingRequest> verdict, std::unique_ptr<ClientPhishingRequest> verdict,
bool is_extended_reporting, bool is_extended_reporting,
bool is_enhanced_protection, bool is_enhanced_protection,
const ClientReportPhishingRequestCallback& callback) override { ClientReportPhishingRequestCallback callback) override {
saved_request_ = *verdict; saved_request_ = *verdict;
saved_callback_ = callback; saved_callback_ = std::move(callback);
request_callback_.Run(); request_callback_.Run();
} }
const ClientPhishingRequest& saved_request() { return saved_request_; } const ClientPhishingRequest& saved_request() { return saved_request_; }
const ClientReportPhishingRequestCallback& saved_callback() {
return saved_callback_; bool saved_callback_is_null() { return saved_callback_.is_null(); }
ClientReportPhishingRequestCallback saved_callback() {
return std::move(saved_callback_);
} }
void SetModel(const ClientSideModel& model) { model_ = model; } void SetModel(const ClientSideModel& model) { model_ = model; }
...@@ -123,7 +126,7 @@ IN_PROC_BROWSER_TEST_F(ClientSideDetectionHostBrowserTest, ...@@ -123,7 +126,7 @@ IN_PROC_BROWSER_TEST_F(ClientSideDetectionHostBrowserTest,
run_loop.Run(); run_loop.Run();
ASSERT_FALSE(fake_csd_service.saved_callback().is_null()); ASSERT_FALSE(fake_csd_service.saved_callback_is_null());
EXPECT_EQ(fake_csd_service.saved_request().model_version(), 123); EXPECT_EQ(fake_csd_service.saved_request().model_version(), 123);
ASSERT_EQ(fake_csd_service.saved_request().vision_match_size(), 1); ASSERT_EQ(fake_csd_service.saved_request().vision_match_size(), 1);
...@@ -133,7 +136,7 @@ IN_PROC_BROWSER_TEST_F(ClientSideDetectionHostBrowserTest, ...@@ -133,7 +136,7 @@ IN_PROC_BROWSER_TEST_F(ClientSideDetectionHostBrowserTest,
// Expect an interstitail to be shown // Expect an interstitail to be shown
EXPECT_CALL(*mock_ui_manager, DisplayBlockingPage(_)); EXPECT_CALL(*mock_ui_manager, DisplayBlockingPage(_));
fake_csd_service.saved_callback().Run(page_url, true); std::move(fake_csd_service.saved_callback()).Run(page_url, true);
} }
} // namespace safe_browsing } // namespace safe_browsing
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/synchronization/waitable_event.h" #include "base/synchronization/waitable_event.h"
#include "base/test/gmock_move_support.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/simple_test_tick_clock.h" #include "base/test/simple_test_tick_clock.h"
#include "chrome/browser/safe_browsing/client_side_detection_service.h" #include "chrome/browser/safe_browsing/client_side_detection_service.h"
...@@ -106,7 +107,7 @@ MATCHER(CallbackIsNull, "") { ...@@ -106,7 +107,7 @@ MATCHER(CallbackIsNull, "") {
class MockModelLoader : public ModelLoader { class MockModelLoader : public ModelLoader {
public: public:
MockModelLoader() : ModelLoader(base::Closure(), nullptr, false) {} MockModelLoader() : ModelLoader(base::RepeatingClosure(), nullptr, false) {}
~MockModelLoader() override = default; ~MockModelLoader() override = default;
MOCK_METHOD1(ScheduleFetch, void(int64_t)); MOCK_METHOD1(ScheduleFetch, void(int64_t));
...@@ -125,7 +126,7 @@ class MockClientSideDetectionService : public ClientSideDetectionService { ...@@ -125,7 +126,7 @@ class MockClientSideDetectionService : public ClientSideDetectionService {
void(std::unique_ptr<ClientPhishingRequest>, void(std::unique_ptr<ClientPhishingRequest>,
bool, bool,
bool, bool,
const ClientReportPhishingRequestCallback&)); ClientReportPhishingRequestCallback));
MOCK_CONST_METHOD1(IsPrivateIPAddress, bool(const std::string&)); MOCK_CONST_METHOD1(IsPrivateIPAddress, bool(const std::string&));
MOCK_METHOD2(GetValidCachedResult, bool(const GURL&, bool*)); MOCK_METHOD2(GetValidCachedResult, bool(const GURL&, bool*));
MOCK_METHOD1(IsInCache, bool(const GURL&)); MOCK_METHOD1(IsInCache, bool(const GURL&));
...@@ -392,14 +393,14 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneNotPhishing) { ...@@ -392,14 +393,14 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneNotPhishing) {
EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest( EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest(
PartiallyEqualVerdict(verdict), _, _, _)) PartiallyEqualVerdict(verdict), _, _, _))
.WillOnce(SaveArg<3>(&cb)); .WillOnce(MoveArg<3>(&cb));
PhishingDetectionDone(verdict.SerializeAsString()); PhishingDetectionDone(verdict.SerializeAsString());
EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get())); EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
ASSERT_FALSE(cb.is_null()); ASSERT_FALSE(cb.is_null());
// Make sure DisplayBlockingPage is not going to be called. // Make sure DisplayBlockingPage is not going to be called.
EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_)).Times(0); EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_)).Times(0);
cb.Run(GURL(verdict.url()), false); std::move(cb).Run(GURL(verdict.url()), false);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get())); EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
} }
...@@ -415,14 +416,14 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneDisabled) { ...@@ -415,14 +416,14 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneDisabled) {
EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest( EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest(
PartiallyEqualVerdict(verdict), _, _, _)) PartiallyEqualVerdict(verdict), _, _, _))
.WillOnce(SaveArg<3>(&cb)); .WillOnce(MoveArg<3>(&cb));
PhishingDetectionDone(verdict.SerializeAsString()); PhishingDetectionDone(verdict.SerializeAsString());
EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get())); EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
ASSERT_FALSE(cb.is_null()); ASSERT_FALSE(cb.is_null());
// Make sure DisplayBlockingPage is not going to be called. // Make sure DisplayBlockingPage is not going to be called.
EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_)).Times(0); EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_)).Times(0);
cb.Run(GURL(verdict.url()), false); std::move(cb).Run(GURL(verdict.url()), false);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get())); EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
} }
...@@ -439,7 +440,7 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneShowInterstitial) { ...@@ -439,7 +440,7 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneShowInterstitial) {
EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest( EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest(
PartiallyEqualVerdict(verdict), _, _, _)) PartiallyEqualVerdict(verdict), _, _, _))
.WillOnce(SaveArg<3>(&cb)); .WillOnce(MoveArg<3>(&cb));
PhishingDetectionDone(verdict.SerializeAsString()); PhishingDetectionDone(verdict.SerializeAsString());
EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get())); EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get())); EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
...@@ -448,7 +449,7 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneShowInterstitial) { ...@@ -448,7 +449,7 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneShowInterstitial) {
UnsafeResource resource; UnsafeResource resource;
EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_)) EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_))
.WillOnce(SaveArg<0>(&resource)); .WillOnce(SaveArg<0>(&resource));
cb.Run(phishing_url, true); std::move(cb).Run(phishing_url, true);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get())); EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
...@@ -481,7 +482,7 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneMultiplePings) { ...@@ -481,7 +482,7 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneMultiplePings) {
EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest( EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest(
PartiallyEqualVerdict(verdict), _, _, _)) PartiallyEqualVerdict(verdict), _, _, _))
.WillOnce(SaveArg<3>(&cb)); .WillOnce(MoveArg<3>(&cb));
PhishingDetectionDone(verdict.SerializeAsString()); PhishingDetectionDone(verdict.SerializeAsString());
EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get())); EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get())); EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
...@@ -501,7 +502,7 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneMultiplePings) { ...@@ -501,7 +502,7 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneMultiplePings) {
verdict.set_client_score(0.8f); verdict.set_client_score(0.8f);
EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest( EXPECT_CALL(*csd_service_, SendClientReportPhishingRequest(
PartiallyEqualVerdict(verdict), _, _, _)) PartiallyEqualVerdict(verdict), _, _, _))
.WillOnce(SaveArg<3>(&cb_other)); .WillOnce(MoveArg<3>(&cb_other));
PhishingDetectionDone(verdict.SerializeAsString()); PhishingDetectionDone(verdict.SerializeAsString());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get())); EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
...@@ -514,8 +515,9 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneMultiplePings) { ...@@ -514,8 +515,9 @@ TEST_F(ClientSideDetectionHostTest, PhishingDetectionDoneMultiplePings) {
EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_)) EXPECT_CALL(*ui_manager_.get(), DisplayBlockingPage(_))
.WillOnce(SaveArg<0>(&resource)); .WillOnce(SaveArg<0>(&resource));
cb.Run(phishing_url, true); // Should have no effect. std::move(cb).Run(phishing_url, true); // Should have no effect.
cb_other.Run(other_phishing_url, true); // Should show interstitial. std::move(cb_other).Run(other_phishing_url,
true); // Should show interstitial.
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get())); EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
......
...@@ -159,14 +159,14 @@ void ClientSideDetectionService::SendClientReportPhishingRequest( ...@@ -159,14 +159,14 @@ void ClientSideDetectionService::SendClientReportPhishingRequest(
std::unique_ptr<ClientPhishingRequest> verdict, std::unique_ptr<ClientPhishingRequest> verdict,
bool is_extended_reporting, bool is_extended_reporting,
bool is_enhanced_reporting, bool is_enhanced_reporting,
const ClientReportPhishingRequestCallback& callback) { ClientReportPhishingRequestCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::ThreadTaskRunnerHandle::Get()->PostTask( base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce( base::BindOnce(
&ClientSideDetectionService::StartClientReportPhishingRequest, &ClientSideDetectionService::StartClientReportPhishingRequest,
weak_factory_.GetWeakPtr(), std::move(verdict), is_extended_reporting, weak_factory_.GetWeakPtr(), std::move(verdict), is_extended_reporting,
is_enhanced_reporting, callback)); is_enhanced_reporting, std::move(callback)));
} }
bool ClientSideDetectionService::IsPrivateIPAddress( bool ClientSideDetectionService::IsPrivateIPAddress(
...@@ -218,12 +218,12 @@ void ClientSideDetectionService::StartClientReportPhishingRequest( ...@@ -218,12 +218,12 @@ void ClientSideDetectionService::StartClientReportPhishingRequest(
std::unique_ptr<ClientPhishingRequest> request, std::unique_ptr<ClientPhishingRequest> request,
bool is_extended_reporting, bool is_extended_reporting,
bool is_enhanced_reporting, bool is_enhanced_reporting,
const ClientReportPhishingRequestCallback& callback) { ClientReportPhishingRequestCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (!enabled_) { if (!enabled_) {
if (!callback.is_null()) if (!callback.is_null())
callback.Run(GURL(request->url()), false); std::move(callback).Run(GURL(request->url()), false);
return; return;
} }
...@@ -299,7 +299,7 @@ void ClientSideDetectionService::StartClientReportPhishingRequest( ...@@ -299,7 +299,7 @@ void ClientSideDetectionService::StartClientReportPhishingRequest(
std::unique_ptr<ClientPhishingReportInfo> info(new ClientPhishingReportInfo); std::unique_ptr<ClientPhishingReportInfo> info(new ClientPhishingReportInfo);
auto* loader_ptr = loader.get(); auto* loader_ptr = loader.get();
info->loader = std::move(loader); info->loader = std::move(loader);
info->callback = callback; info->callback = std::move(callback);
info->phishing_url = GURL(request->url()); info->phishing_url = GURL(request->url());
client_phishing_reports_[loader_ptr] = std::move(info); client_phishing_reports_[loader_ptr] = std::move(info);
......
...@@ -52,7 +52,8 @@ class ClientSideDetectionHost; ...@@ -52,7 +52,8 @@ class ClientSideDetectionHost;
class ClientSideDetectionService : public KeyedService { class ClientSideDetectionService : public KeyedService {
public: public:
// void(GURL phishing_url, bool is_phishing). // void(GURL phishing_url, bool is_phishing).
typedef base::Callback<void(GURL, bool)> ClientReportPhishingRequestCallback; typedef base::OnceCallback<void(GURL, bool)>
ClientReportPhishingRequestCallback;
explicit ClientSideDetectionService(Profile* profile); explicit ClientSideDetectionService(Profile* profile);
~ClientSideDetectionService() override; ~ClientSideDetectionService() override;
...@@ -85,7 +86,7 @@ class ClientSideDetectionService : public KeyedService { ...@@ -85,7 +86,7 @@ class ClientSideDetectionService : public KeyedService {
std::unique_ptr<ClientPhishingRequest> verdict, std::unique_ptr<ClientPhishingRequest> verdict,
bool is_extended_reporting, bool is_extended_reporting,
bool is_enhanced_protection, bool is_enhanced_protection,
const ClientReportPhishingRequestCallback& callback); ClientReportPhishingRequestCallback callback);
// Returns true if the given IP address string falls within a private // Returns true if the given IP address string falls within a private
// (unroutable) network block. Pages which are hosted on these IP addresses // (unroutable) network block. Pages which are hosted on these IP addresses
...@@ -170,7 +171,7 @@ class ClientSideDetectionService : public KeyedService { ...@@ -170,7 +171,7 @@ class ClientSideDetectionService : public KeyedService {
std::unique_ptr<ClientPhishingRequest> request, std::unique_ptr<ClientPhishingRequest> request,
bool is_extended_reporting, bool is_extended_reporting,
bool is_enhanced_protection, bool is_enhanced_protection,
const ClientReportPhishingRequestCallback& callback); ClientReportPhishingRequestCallback callback);
// Called by OnURLFetchComplete to handle the server response from // Called by OnURLFetchComplete to handle the server response from
// sending the client-side phishing request. // sending the client-side phishing request.
......
...@@ -29,7 +29,9 @@ namespace { ...@@ -29,7 +29,9 @@ namespace {
class FakeModelLoader : public ModelLoader { class FakeModelLoader : public ModelLoader {
public: public:
explicit FakeModelLoader(std::string model_str) explicit FakeModelLoader(std::string model_str)
: ModelLoader(base::Closure(), nullptr, /*is_extended_reporting=*/false) { : ModelLoader(base::RepeatingClosure(),
nullptr,
/*is_extended_reporting=*/false) {
model_str_ = model_str; model_str_ = model_str;
} }
~FakeModelLoader() override = default; ~FakeModelLoader() override = default;
......
...@@ -50,7 +50,7 @@ namespace { ...@@ -50,7 +50,7 @@ namespace {
class MockModelLoader : public ModelLoader { class MockModelLoader : public ModelLoader {
public: public:
explicit MockModelLoader(const std::string& model_name) explicit MockModelLoader(const std::string& model_name)
: ModelLoader(base::Closure(), nullptr, model_name) {} : ModelLoader(base::RepeatingClosure(), nullptr, model_name) {}
~MockModelLoader() override {} ~MockModelLoader() override {}
MOCK_METHOD1(ScheduleFetch, void(int64_t)); MOCK_METHOD1(ScheduleFetch, void(int64_t));
...@@ -95,8 +95,8 @@ class ClientSideDetectionServiceTest : public testing::Test { ...@@ -95,8 +95,8 @@ class ClientSideDetectionServiceTest : public testing::Test {
base::RunLoop run_loop; base::RunLoop run_loop;
csd_service_->SendClientReportPhishingRequest( csd_service_->SendClientReportPhishingRequest(
std::move(request), is_extended_reporting, is_enhanced_reporting, std::move(request), is_extended_reporting, is_enhanced_reporting,
base::Bind(&ClientSideDetectionServiceTest::SendRequestDone, base::BindOnce(&ClientSideDetectionServiceTest::SendRequestDone,
base::Unretained(this), run_loop.QuitWhenIdleClosure())); base::Unretained(this), run_loop.QuitWhenIdleClosure()));
phishing_url_ = phishing_url; phishing_url_ = phishing_url;
run_loop.Run(); // Waits until callback is called. run_loop.Run(); // Waits until callback is called.
return is_phishing_; return is_phishing_;
......
...@@ -117,7 +117,7 @@ bool ModelLoader::ModelHasValidHashIds(const ClientSideModel& model) { ...@@ -117,7 +117,7 @@ bool ModelLoader::ModelHasValidHashIds(const ClientSideModel& model) {
// Model name and URL are a function of is_extended_reporting and Finch. // Model name and URL are a function of is_extended_reporting and Finch.
ModelLoader::ModelLoader( ModelLoader::ModelLoader(
base::Closure update_renderers_callback, base::RepeatingClosure update_renderers_callback,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
bool is_extended_reporting) bool is_extended_reporting)
: name_(FillInModelName(is_extended_reporting, GetModelNumber())), : name_(FillInModelName(is_extended_reporting, GetModelNumber())),
...@@ -131,7 +131,7 @@ ModelLoader::ModelLoader( ...@@ -131,7 +131,7 @@ ModelLoader::ModelLoader(
// For testing only // For testing only
ModelLoader::ModelLoader( ModelLoader::ModelLoader(
base::Closure update_renderers_callback, base::RepeatingClosure update_renderers_callback,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const std::string& model_name) const std::string& model_name)
: name_(model_name), : name_(model_name),
......
...@@ -64,7 +64,7 @@ class ModelLoader { ...@@ -64,7 +64,7 @@ class ModelLoader {
// Constructs a model loader to fetch a model using |url_loader_factory|. // Constructs a model loader to fetch a model using |url_loader_factory|.
// When ScheduleFetch is called, |update_renderers| will be called on the // When ScheduleFetch is called, |update_renderers| will be called on the
// same sequence if the fetch is successful. // same sequence if the fetch is successful.
ModelLoader(base::Closure update_renderers, ModelLoader(base::RepeatingClosure update_renderers,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
bool is_extended_reporting); bool is_extended_reporting);
virtual ~ModelLoader(); virtual ~ModelLoader();
...@@ -90,7 +90,7 @@ class ModelLoader { ...@@ -90,7 +90,7 @@ class ModelLoader {
protected: protected:
// For testing only. // For testing only.
ModelLoader(base::Closure update_renderers, ModelLoader(base::RepeatingClosure update_renderers,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const std::string& model_name); const std::string& model_name);
...@@ -136,7 +136,7 @@ class ModelLoader { ...@@ -136,7 +136,7 @@ class ModelLoader {
std::unique_ptr<network::SimpleURLLoader> url_loader_; std::unique_ptr<network::SimpleURLLoader> url_loader_;
// Callback to invoke when we've got a new model. CSD will send it around. // Callback to invoke when we've got a new model. CSD will send it around.
base::Closure update_renderers_callback_; base::RepeatingClosure update_renderers_callback_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_; scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
......
...@@ -43,7 +43,7 @@ namespace { ...@@ -43,7 +43,7 @@ namespace {
class MockModelLoader : public ModelLoader { class MockModelLoader : public ModelLoader {
public: public:
MockModelLoader( MockModelLoader(
base::Closure update_renderers_callback, base::RepeatingClosure update_renderers_callback,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const std::string model_name) const std::string model_name)
: ModelLoader(update_renderers_callback, url_loader_factory, model_name) { : ModelLoader(update_renderers_callback, url_loader_factory, model_name) {
...@@ -129,8 +129,8 @@ ACTION_P(InvokeClosure, closure) { ...@@ -129,8 +129,8 @@ ACTION_P(InvokeClosure, closure) {
} }
TEST_F(ModelLoaderTest, FetchModelFromLocalFileTest) { TEST_F(ModelLoaderTest, FetchModelFromLocalFileTest) {
StrictMock<MockModelLoader> loader(base::Closure(), shared_loader_factory(), StrictMock<MockModelLoader> loader(base::RepeatingClosure(),
"top_model.pb"); shared_loader_factory(), "top_model.pb");
SetModelUrl(loader); SetModelUrl(loader);
// The model fetch tries to read from local file but is empty. // The model fetch tries to read from local file but is empty.
...@@ -190,8 +190,8 @@ TEST_F(ModelLoaderTest, FetchModelFromLocalFileTest) { ...@@ -190,8 +190,8 @@ TEST_F(ModelLoaderTest, FetchModelFromLocalFileTest) {
// Test the response to many variations of model responses. // Test the response to many variations of model responses.
TEST_F(ModelLoaderTest, FetchModelTest) { TEST_F(ModelLoaderTest, FetchModelTest) {
StrictMock<MockModelLoader> loader(base::Closure(), shared_loader_factory(), StrictMock<MockModelLoader> loader(base::RepeatingClosure(),
"top_model.pb"); shared_loader_factory(), "top_model.pb");
SetModelUrl(loader); SetModelUrl(loader);
// The model fetch failed. // The model fetch failed.
...@@ -332,7 +332,8 @@ TEST_F(ModelLoaderTest, UpdateRenderersTest) { ...@@ -332,7 +332,8 @@ TEST_F(ModelLoaderTest, UpdateRenderersTest) {
// Test that a one fetch schedules another fetch. // Test that a one fetch schedules another fetch.
TEST_F(ModelLoaderTest, RescheduleFetchTest) { TEST_F(ModelLoaderTest, RescheduleFetchTest) {
StrictMock<MockModelLoader> loader(base::Closure(), nullptr, "top_model.pb"); StrictMock<MockModelLoader> loader(base::RepeatingClosure(), nullptr,
"top_model.pb");
// Zero max_age. Uses default. // Zero max_age. Uses default.
base::TimeDelta max_age; base::TimeDelta max_age;
...@@ -364,7 +365,7 @@ TEST_F(ModelLoaderTest, ModelNamesTest) { ...@@ -364,7 +365,7 @@ TEST_F(ModelLoaderTest, ModelNamesTest) {
// No Finch setup. Should default to 4. // No Finch setup. Should default to 4.
std::unique_ptr<ModelLoader> loader; std::unique_ptr<ModelLoader> loader;
loader.reset(new ModelLoader(base::Closure(), nullptr, loader.reset(new ModelLoader(base::RepeatingClosure(), nullptr,
false /* is_extended_reporting */)); false /* is_extended_reporting */));
EXPECT_EQ(loader->name(), "client_model_v5_variation_4.pb"); EXPECT_EQ(loader->name(), "client_model_v5_variation_4.pb");
EXPECT_EQ(loader->url_.spec(), EXPECT_EQ(loader->url_.spec(),
...@@ -373,12 +374,12 @@ TEST_F(ModelLoaderTest, ModelNamesTest) { ...@@ -373,12 +374,12 @@ TEST_F(ModelLoaderTest, ModelNamesTest) {
// Model 1, no extended reporting. // Model 1, no extended reporting.
SetFinchModelNumber(1); SetFinchModelNumber(1);
loader.reset(new ModelLoader(base::Closure(), nullptr, false)); loader.reset(new ModelLoader(base::RepeatingClosure(), nullptr, false));
EXPECT_EQ(loader->name(), "client_model_v5_variation_1.pb"); EXPECT_EQ(loader->name(), "client_model_v5_variation_1.pb");
// Model 2, with extended reporting. // Model 2, with extended reporting.
SetFinchModelNumber(2); SetFinchModelNumber(2);
loader.reset(new ModelLoader(base::Closure(), nullptr, true)); loader.reset(new ModelLoader(base::RepeatingClosure(), nullptr, true));
EXPECT_EQ(loader->name(), "client_model_v5_ext_variation_2.pb"); EXPECT_EQ(loader->name(), "client_model_v5_ext_variation_2.pb");
} }
......
...@@ -116,7 +116,7 @@ class DownloadItemCreatedObserver : public DownloadManager::Observer { ...@@ -116,7 +116,7 @@ class DownloadItemCreatedObserver : public DownloadManager::Observer {
base::RunLoop run_loop; base::RunLoop run_loop;
quit_waiting_callback_ = run_loop.QuitClosure(); quit_waiting_callback_ = run_loop.QuitClosure();
run_loop.Run(); run_loop.Run();
quit_waiting_callback_ = base::Closure(); quit_waiting_callback_ = base::OnceClosure();
} }
*items_seen = items_seen_; *items_seen = items_seen_;
...@@ -131,17 +131,17 @@ class DownloadItemCreatedObserver : public DownloadManager::Observer { ...@@ -131,17 +131,17 @@ class DownloadItemCreatedObserver : public DownloadManager::Observer {
items_seen_.push_back(item); items_seen_.push_back(item);
if (!quit_waiting_callback_.is_null()) if (!quit_waiting_callback_.is_null())
quit_waiting_callback_.Run(); std::move(quit_waiting_callback_).Run();
} }
void ManagerGoingDown(DownloadManager* manager) override { void ManagerGoingDown(DownloadManager* manager) override {
manager_->RemoveObserver(this); manager_->RemoveObserver(this);
manager_ = nullptr; manager_ = nullptr;
if (!quit_waiting_callback_.is_null()) if (!quit_waiting_callback_.is_null())
quit_waiting_callback_.Run(); std::move(quit_waiting_callback_).Run();
} }
base::Closure quit_waiting_callback_; base::OnceClosure quit_waiting_callback_;
DownloadManager* manager_; DownloadManager* manager_;
std::vector<DownloadItem*> items_seen_; std::vector<DownloadItem*> items_seen_;
......
...@@ -171,14 +171,13 @@ class SafeBrowsingService : public SafeBrowsingServiceInterface, ...@@ -171,14 +171,13 @@ class SafeBrowsingService : public SafeBrowsingServiceInterface,
void AddDownloadManager(content::DownloadManager* download_manager); void AddDownloadManager(content::DownloadManager* download_manager);
// Type for subscriptions to SafeBrowsing service state. // Type for subscriptions to SafeBrowsing service state.
typedef base::CallbackList<void(void)>::Subscription StateSubscription; typedef base::RepeatingClosureList::Subscription StateSubscription;
typedef base::CallbackList<void(void)>::Subscription ShutdownSubscription;
// Adds a listener for when SafeBrowsing preferences might have changed. // Adds a listener for when SafeBrowsing preferences might have changed.
// To get the current state, the callback should call enabled_by_prefs(). // To get the current state, the callback should call enabled_by_prefs().
// Should only be called on the UI thread. // Should only be called on the UI thread.
virtual std::unique_ptr<StateSubscription> RegisterStateCallback( virtual std::unique_ptr<StateSubscription> RegisterStateCallback(
const base::Callback<void(void)>& callback); const base::RepeatingClosure& callback);
// Sends serialized download report to backend. // Sends serialized download report to backend.
virtual void SendSerializedDownloadReport(Profile* profile, virtual void SendSerializedDownloadReport(Profile* profile,
......
...@@ -50,7 +50,7 @@ TestSafeBrowsingService::GetTestUrlLoaderFactory() { ...@@ -50,7 +50,7 @@ TestSafeBrowsingService::GetTestUrlLoaderFactory() {
std::unique_ptr<SafeBrowsingService::StateSubscription> std::unique_ptr<SafeBrowsingService::StateSubscription>
TestSafeBrowsingService::RegisterStateCallback( TestSafeBrowsingService::RegisterStateCallback(
const base::Callback<void(void)>& callback) { const base::RepeatingClosure& callback) {
// This override is required since TestSafeBrowsingService can be destroyed // This override is required since TestSafeBrowsingService can be destroyed
// before CertificateReportingService, which causes a crash due to the // before CertificateReportingService, which causes a crash due to the
// leftover callback at destruction time. // leftover callback at destruction time.
......
...@@ -70,7 +70,7 @@ class TestSafeBrowsingService : public SafeBrowsingService, ...@@ -70,7 +70,7 @@ class TestSafeBrowsingService : public SafeBrowsingService,
void SetUseTestUrlLoaderFactory(bool use_test_url_loader_factory); void SetUseTestUrlLoaderFactory(bool use_test_url_loader_factory);
std::unique_ptr<SafeBrowsingService::StateSubscription> RegisterStateCallback( std::unique_ptr<SafeBrowsingService::StateSubscription> RegisterStateCallback(
const base::Callback<void(void)>& callback) override; const base::RepeatingClosure& callback) override;
network::TestURLLoaderFactory* GetTestUrlLoaderFactory(); network::TestURLLoaderFactory* GetTestUrlLoaderFactory();
protected: protected:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment