Commit 0ef31de1 authored by Daniel Rubery's avatar Daniel Rubery Committed by Commit Bot

Update base::Callback usages in chrome/renderer/safe_browsing

Replace a few usages of base::Callback with base::OnceCallback, and
update the corresponding base::Binds with base::BindOnce.

TEST=Triggered a ping twice for the same page, one with and one without
this change. The generated features were the same.

Bug: 933537
Change-Id: Ib9e5454be2e67342868b00af83a58b16cde6c163
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1489423Reviewed-by: default avatarVarun Khaneja <vakh@chromium.org>
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638341}
parent 6abe75ac
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "chrome/renderer/safe_browsing/phishing_classifier.h" #include "chrome/renderer/safe_browsing/phishing_classifier.h"
#include <string> #include <string>
#include <utility>
#include "base/bind.h" #include "base/bind.h"
#include "base/callback.h" #include "base/callback.h"
...@@ -99,9 +100,8 @@ bool PhishingClassifier::is_ready() const { ...@@ -99,9 +100,8 @@ bool PhishingClassifier::is_ready() const {
return scorer_ != NULL; return scorer_ != NULL;
} }
void PhishingClassifier::BeginClassification( void PhishingClassifier::BeginClassification(const base::string16* page_text,
const base::string16* page_text, DoneCallback done_callback) {
const DoneCallback& done_callback) {
DCHECK(is_ready()); DCHECK(is_ready());
// The RenderView should have called CancelPendingClassification() before // The RenderView should have called CancelPendingClassification() before
...@@ -112,7 +112,7 @@ void PhishingClassifier::BeginClassification( ...@@ -112,7 +112,7 @@ void PhishingClassifier::BeginClassification(
CancelPendingClassification(); CancelPendingClassification();
page_text_ = page_text; page_text_ = page_text;
done_callback_ = done_callback; done_callback_ = std::move(done_callback);
// For consistency, we always want to invoke the DoneCallback // For consistency, we always want to invoke the DoneCallback
// asynchronously, rather than directly from this method. To ensure that // asynchronously, rather than directly from this method. To ensure that
...@@ -154,8 +154,8 @@ void PhishingClassifier::BeginFeatureExtraction() { ...@@ -154,8 +154,8 @@ void PhishingClassifier::BeginFeatureExtraction() {
// in several chunks of work and invokes the callback when finished. // in several chunks of work and invokes the callback when finished.
dom_extractor_->ExtractFeatures( dom_extractor_->ExtractFeatures(
frame->GetDocument(), features_.get(), frame->GetDocument(), features_.get(),
base::Bind(&PhishingClassifier::DOMExtractionFinished, base::BindOnce(&PhishingClassifier::DOMExtractionFinished,
base::Unretained(this))); base::Unretained(this)));
} }
void PhishingClassifier::CancelPendingClassification() { void PhishingClassifier::CancelPendingClassification() {
...@@ -174,11 +174,9 @@ void PhishingClassifier::DOMExtractionFinished(bool success) { ...@@ -174,11 +174,9 @@ void PhishingClassifier::DOMExtractionFinished(bool success) {
// Term feature extraction can take awhile, so it runs asynchronously // Term feature extraction can take awhile, so it runs asynchronously
// in several chunks of work and invokes the callback when finished. // in several chunks of work and invokes the callback when finished.
term_extractor_->ExtractFeatures( term_extractor_->ExtractFeatures(
page_text_, page_text_, features_.get(), shingle_hashes_.get(),
features_.get(), base::BindOnce(&PhishingClassifier::TermExtractionFinished,
shingle_hashes_.get(), base::Unretained(this)));
base::Bind(&PhishingClassifier::TermExtractionFinished,
base::Unretained(this)));
} else { } else {
RunFailureCallback(); RunFailureCallback();
} }
...@@ -225,7 +223,7 @@ void PhishingClassifier::CheckNoPendingClassification() { ...@@ -225,7 +223,7 @@ void PhishingClassifier::CheckNoPendingClassification() {
} }
void PhishingClassifier::RunCallback(const ClientPhishingRequest& verdict) { void PhishingClassifier::RunCallback(const ClientPhishingRequest& verdict) {
done_callback_.Run(verdict); std::move(done_callback_).Run(verdict);
Clear(); Clear();
} }
......
...@@ -49,7 +49,7 @@ class PhishingClassifier { ...@@ -49,7 +49,7 @@ class PhishingClassifier {
// is true, the page is considered phishy by the client-side model, // is true, the page is considered phishy by the client-side model,
// and the browser should ping back to get a final verdict. The // and the browser should ping back to get a final verdict. The
// verdict.client_score() is set to kInvalidScore if classification failed. // verdict.client_score() is set to kInvalidScore if classification failed.
typedef base::Callback<void(const ClientPhishingRequest& /* verdict */)> typedef base::OnceCallback<void(const ClientPhishingRequest& /* verdict */)>
DoneCallback; DoneCallback;
static const float kInvalidScore; static const float kInvalidScore;
...@@ -88,7 +88,7 @@ class PhishingClassifier { ...@@ -88,7 +88,7 @@ class PhishingClassifier {
// It is an error to call BeginClassification if the classifier is not yet // It is an error to call BeginClassification if the classifier is not yet
// ready. // ready.
virtual void BeginClassification(const base::string16* page_text, virtual void BeginClassification(const base::string16* page_text,
const DoneCallback& callback); DoneCallback callback);
// Called by the RenderView (on the render thread) when a page is unloading // Called by the RenderView (on the render thread) when a page is unloading
// or the RenderView is being destroyed. This cancels any extraction that // or the RenderView is being destroyed. This cancels any extraction that
......
...@@ -125,8 +125,9 @@ class PhishingClassifierTest : public ChromeRenderViewTest { ...@@ -125,8 +125,9 @@ class PhishingClassifierTest : public ChromeRenderViewTest {
feature_map_.Clear(); feature_map_.Clear();
classifier_->BeginClassification( classifier_->BeginClassification(
page_text, base::Bind(&PhishingClassifierTest::ClassificationFinished, page_text,
base::Unretained(this))); base::BindOnce(&PhishingClassifierTest::ClassificationFinished,
base::Unretained(this)));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
......
...@@ -278,8 +278,8 @@ void PhishingClassifierDelegate::MaybeStartClassification() { ...@@ -278,8 +278,8 @@ void PhishingClassifierDelegate::MaybeStartClassification() {
is_classifying_ = true; is_classifying_ = true;
classifier_->BeginClassification( classifier_->BeginClassification(
&classifier_page_text_, &classifier_page_text_,
base::Bind(&PhishingClassifierDelegate::ClassificationDone, base::BindOnce(&PhishingClassifierDelegate::ClassificationDone,
base::Unretained(this))); base::Unretained(this)));
} }
void PhishingClassifierDelegate::OnDestruct() { void PhishingClassifierDelegate::OnDestruct() {
......
...@@ -41,8 +41,7 @@ class MockPhishingClassifier : public PhishingClassifier { ...@@ -41,8 +41,7 @@ class MockPhishingClassifier : public PhishingClassifier {
~MockPhishingClassifier() override {} ~MockPhishingClassifier() override {}
MOCK_METHOD2(BeginClassification, MOCK_METHOD2(BeginClassification, void(const base::string16*, DoneCallback));
void(const base::string16*, const DoneCallback&));
MOCK_METHOD0(CancelPendingClassification, void()); MOCK_METHOD0(CancelPendingClassification, void());
private: private:
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "chrome/renderer/safe_browsing/phishing_dom_feature_extractor.h" #include "chrome/renderer/safe_browsing/phishing_dom_feature_extractor.h"
#include <utility>
#include "base/bind.h" #include "base/bind.h"
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/location.h" #include "base/location.h"
...@@ -112,10 +114,9 @@ PhishingDOMFeatureExtractor::~PhishingDOMFeatureExtractor() { ...@@ -112,10 +114,9 @@ PhishingDOMFeatureExtractor::~PhishingDOMFeatureExtractor() {
CheckNoPendingExtraction(); CheckNoPendingExtraction();
} }
void PhishingDOMFeatureExtractor::ExtractFeatures( void PhishingDOMFeatureExtractor::ExtractFeatures(blink::WebDocument document,
blink::WebDocument document, FeatureMap* features,
FeatureMap* features, DoneCallback done_callback) {
const DoneCallback& done_callback) {
// The RenderView should have called CancelPendingExtraction() before // The RenderView should have called CancelPendingExtraction() before
// starting a new extraction, so DCHECK this. // starting a new extraction, so DCHECK this.
CheckNoPendingExtraction(); CheckNoPendingExtraction();
...@@ -124,7 +125,7 @@ void PhishingDOMFeatureExtractor::ExtractFeatures( ...@@ -124,7 +125,7 @@ void PhishingDOMFeatureExtractor::ExtractFeatures(
CancelPendingExtraction(); CancelPendingExtraction();
features_ = features; features_ = features;
done_callback_ = done_callback; done_callback_ = std::move(done_callback);
page_feature_state_.reset(new PageFeatureState(clock_->Now())); page_feature_state_.reset(new PageFeatureState(clock_->Now()));
cur_document_ = document; cur_document_ = document;
...@@ -360,7 +361,7 @@ void PhishingDOMFeatureExtractor::RunCallback(bool success) { ...@@ -360,7 +361,7 @@ void PhishingDOMFeatureExtractor::RunCallback(bool success) {
clock_->Now() - page_feature_state_->start_time); clock_->Now() - page_feature_state_->start_time);
DCHECK(!done_callback_.is_null()); DCHECK(!done_callback_.is_null());
done_callback_.Run(success); std::move(done_callback_).Run(success);
Clear(); Clear();
} }
......
...@@ -32,7 +32,7 @@ class PhishingDOMFeatureExtractor { ...@@ -32,7 +32,7 @@ class PhishingDOMFeatureExtractor {
public: public:
// Callback to be run when feature extraction finishes. The callback // Callback to be run when feature extraction finishes. The callback
// argument is true if extraction was successful, false otherwise. // argument is true if extraction was successful, false otherwise.
typedef base::Callback<void(bool)> DoneCallback; typedef base::OnceCallback<void(bool)> DoneCallback;
// Creates a PhishingDOMFeatureExtractor instance. // Creates a PhishingDOMFeatureExtractor instance.
// |clock| is used for timing feature extractor operations, and may be // |clock| is used for timing feature extractor operations, and may be
...@@ -48,7 +48,7 @@ class PhishingDOMFeatureExtractor { ...@@ -48,7 +48,7 @@ class PhishingDOMFeatureExtractor {
// takes ownership of the callback. // takes ownership of the callback.
void ExtractFeatures(blink::WebDocument document, void ExtractFeatures(blink::WebDocument document,
FeatureMap* features, FeatureMap* features,
const DoneCallback& done_callback); DoneCallback done_callback);
// Cancels any pending feature extraction. The DoneCallback will not be run. // Cancels any pending feature extraction. The DoneCallback will not be run.
// Must be called if there is a feature extraction in progress when the page // Must be called if there is a feature extraction in progress when the page
......
...@@ -161,8 +161,8 @@ class PhishingDOMFeatureExtractorTest : public ChromeRenderViewTest { ...@@ -161,8 +161,8 @@ class PhishingDOMFeatureExtractorTest : public ChromeRenderViewTest {
extractor_->ExtractFeatures( extractor_->ExtractFeatures(
GetMainFrame()->GetDocument(), features, GetMainFrame()->GetDocument(), features,
base::Bind(&PhishingDOMFeatureExtractorTest::AnotherExtractionDone, base::BindOnce(&PhishingDOMFeatureExtractorTest::AnotherExtractionDone,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
message_loop_->Run(); message_loop_->Run();
} }
...@@ -174,8 +174,8 @@ class PhishingDOMFeatureExtractorTest : public ChromeRenderViewTest { ...@@ -174,8 +174,8 @@ class PhishingDOMFeatureExtractorTest : public ChromeRenderViewTest {
extractor_->ExtractFeatures( extractor_->ExtractFeatures(
GetMainFrame()->GetDocument(), features, GetMainFrame()->GetDocument(), features,
base::Bind(&PhishingDOMFeatureExtractorTest::AnotherExtractionDone, base::BindOnce(&PhishingDOMFeatureExtractorTest::AnotherExtractionDone,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
message_loop_->Run(); message_loop_->Run();
} }
......
...@@ -111,7 +111,7 @@ void PhishingTermFeatureExtractor::ExtractFeatures( ...@@ -111,7 +111,7 @@ void PhishingTermFeatureExtractor::ExtractFeatures(
const base::string16* page_text, const base::string16* page_text,
FeatureMap* features, FeatureMap* features,
std::set<uint32_t>* shingle_hashes, std::set<uint32_t>* shingle_hashes,
const DoneCallback& done_callback) { DoneCallback done_callback) {
// The RenderView should have called CancelPendingExtraction() before // The RenderView should have called CancelPendingExtraction() before
// starting a new extraction, so DCHECK this. // starting a new extraction, so DCHECK this.
CheckNoPendingExtraction(); CheckNoPendingExtraction();
...@@ -121,8 +121,7 @@ void PhishingTermFeatureExtractor::ExtractFeatures( ...@@ -121,8 +121,7 @@ void PhishingTermFeatureExtractor::ExtractFeatures(
page_text_ = page_text; page_text_ = page_text;
features_ = features; features_ = features;
shingle_hashes_ = shingle_hashes, shingle_hashes_ = shingle_hashes, done_callback_ = std::move(done_callback);
done_callback_ = done_callback;
state_.reset(new ExtractionState(*page_text_, clock_->Now())); state_.reset(new ExtractionState(*page_text_, clock_->Now()));
base::ThreadTaskRunnerHandle::Get()->PostTask( base::ThreadTaskRunnerHandle::Get()->PostTask(
...@@ -282,7 +281,7 @@ void PhishingTermFeatureExtractor::RunCallback(bool success) { ...@@ -282,7 +281,7 @@ void PhishingTermFeatureExtractor::RunCallback(bool success) {
clock_->Now() - state_->start_time); clock_->Now() - state_->start_time);
DCHECK(!done_callback_.is_null()); DCHECK(!done_callback_.is_null());
done_callback_.Run(success); std::move(done_callback_).Run(success);
Clear(); Clear();
} }
......
...@@ -38,7 +38,7 @@ class PhishingTermFeatureExtractor { ...@@ -38,7 +38,7 @@ class PhishingTermFeatureExtractor {
public: public:
// Callback to be run when feature extraction finishes. The callback // Callback to be run when feature extraction finishes. The callback
// argument is true if extraction was successful, false otherwise. // argument is true if extraction was successful, false otherwise.
typedef base::Callback<void(bool)> DoneCallback; typedef base::OnceCallback<void(bool)> DoneCallback;
// Creates a PhishingTermFeatureExtractor which will extract features for // Creates a PhishingTermFeatureExtractor which will extract features for
// all of the terms whose SHA-256 hashes are in |page_term_hashes|. These // all of the terms whose SHA-256 hashes are in |page_term_hashes|. These
...@@ -83,7 +83,7 @@ class PhishingTermFeatureExtractor { ...@@ -83,7 +83,7 @@ class PhishingTermFeatureExtractor {
void ExtractFeatures(const base::string16* page_text, void ExtractFeatures(const base::string16* page_text,
FeatureMap* features, FeatureMap* features,
std::set<uint32_t>* shingle_hashes, std::set<uint32_t>* shingle_hashes,
const DoneCallback& done_callback); DoneCallback done_callback);
// Cancels any pending feature extraction. The DoneCallback will not be run. // Cancels any pending feature extraction. The DoneCallback will not be run.
// Must be called if there is a feature extraction in progress when the page // Must be called if there is a feature extraction in progress when the page
......
...@@ -97,11 +97,9 @@ class PhishingTermFeatureExtractorTest : public ::testing::Test { ...@@ -97,11 +97,9 @@ class PhishingTermFeatureExtractorTest : public ::testing::Test {
std::set<uint32_t>* shingle_hashes) { std::set<uint32_t>* shingle_hashes) {
success_ = false; success_ = false;
extractor_->ExtractFeatures( extractor_->ExtractFeatures(
page_text, page_text, features, shingle_hashes,
features, base::BindOnce(&PhishingTermFeatureExtractorTest::ExtractionDone,
shingle_hashes, base::Unretained(this)));
base::Bind(&PhishingTermFeatureExtractorTest::ExtractionDone,
base::Unretained(this)));
active_run_loop_ = std::make_unique<base::RunLoop>(); active_run_loop_ = std::make_unique<base::RunLoop>();
active_run_loop_->Run(); active_run_loop_->Run();
return success_; return success_;
...@@ -111,11 +109,9 @@ class PhishingTermFeatureExtractorTest : public ::testing::Test { ...@@ -111,11 +109,9 @@ class PhishingTermFeatureExtractorTest : public ::testing::Test {
FeatureMap* features, FeatureMap* features,
std::set<uint32_t>* shingle_hashes) { std::set<uint32_t>* shingle_hashes) {
extractor_->ExtractFeatures( extractor_->ExtractFeatures(
page_text, page_text, features, shingle_hashes,
features, base::BindOnce(&PhishingTermFeatureExtractorTest::ExtractionDone,
shingle_hashes, base::Unretained(this)));
base::Bind(&PhishingTermFeatureExtractorTest::ExtractionDone,
base::Unretained(this)));
base::ThreadTaskRunnerHandle::Get()->PostTask( base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce(&PhishingTermFeatureExtractorTest::QuitExtraction, base::BindOnce(&PhishingTermFeatureExtractorTest::QuitExtraction,
......
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