Index: components/safe_browsing/password_protection/password_protection_service.cc |
diff --git a/components/safe_browsing/password_protection/password_protection_service.cc b/components/safe_browsing/password_protection/password_protection_service.cc |
index 3aee7ac11413ccb50a6e0993c05af43acbcb0445..403378ed3c1e76b06f01c4efc8abd31cdc1111ee 100644 |
--- a/components/safe_browsing/password_protection/password_protection_service.cc |
+++ b/components/safe_browsing/password_protection/password_protection_service.cc |
@@ -63,6 +63,12 @@ GURL GetHostNameWithHTTPScheme(const GURL& url) { |
} // namespace |
+PasswordProtectionFrame::PasswordProtectionFrame() { |
Nathan Parker
2017/04/17 20:28:02
I wonder if you can delete this default ctor.
Mayb
Jialiu Lin
2017/04/18 00:58:26
Done.
|
+ NOTIMPLEMENTED(); |
+} |
+ |
+PasswordProtectionFrame::~PasswordProtectionFrame() = default; |
+ |
PasswordProtectionService::PasswordProtectionService( |
const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager, |
scoped_refptr<net::URLRequestContextGetter> request_context_getter, |
@@ -80,6 +86,7 @@ PasswordProtectionService::PasswordProtectionService( |
} |
PasswordProtectionService::~PasswordProtectionService() { |
+ tracker_.TryCancelAll(); |
CancelPendingRequests(); |
history_service_observer_.RemoveAll(); |
weak_factory_.InvalidateWeakPtrs(); |
@@ -88,27 +95,21 @@ PasswordProtectionService::~PasswordProtectionService() { |
void PasswordProtectionService::RecordPasswordReuse(const GURL& url) { |
DCHECK_CURRENTLY_ON(BrowserThread::UI); |
DCHECK(database_manager_); |
- if (!url.is_valid()) |
- return; |
- |
- BrowserThread::PostTask( |
- BrowserThread::IO, FROM_HERE, |
- base::Bind( |
- &PasswordProtectionService::CheckCsdWhitelistOnIOThread, GetWeakPtr(), |
- url, |
- base::Bind(&PasswordProtectionService::OnMatchCsdWhiteListResult, |
- base::Unretained(this)))); |
+ bool* match_whitelist = new bool(false); |
Nathan Parker
2017/04/17 20:28:02
bare pointer alert. :-)
Could this be a unique_p
Jialiu Lin
2017/04/18 00:58:26
Similar to my reply in PasswordProtectionRequest c
|
+ tracker_.PostTaskAndReply( |
+ BrowserThread::GetTaskRunnerForThread(BrowserThread::IO).get(), FROM_HERE, |
+ base::Bind(&PasswordProtectionService::CheckCsdWhitelistOnIOThread, |
+ base::Unretained(this), url, match_whitelist), |
+ base::Bind(&PasswordProtectionService::OnMatchCsdWhiteListResult, |
+ base::Unretained(this), base::Owned(match_whitelist))); |
} |
void PasswordProtectionService::CheckCsdWhitelistOnIOThread( |
const GURL& url, |
- const CheckCsdWhitelistCallback& callback) { |
+ bool* check_result) { |
DCHECK_CURRENTLY_ON(BrowserThread::IO); |
- DCHECK(database_manager_); |
- bool check_result = database_manager_->MatchCsdWhitelistUrl(url); |
- DCHECK(callback); |
- BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, |
- base::Bind(callback, check_result)); |
+ if (url.is_valid()) |
Nathan Parker
2017/04/17 20:28:02
So no-standard URLs will default to "whitelisted=f
Jialiu Lin
2017/04/18 00:58:27
Thanks for pointing this out. Changed default to t
|
+ *check_result = database_manager_->MatchCsdWhitelistUrl(url); |
} |
LoginReputationClientResponse::VerdictType |
@@ -201,19 +202,35 @@ void PasswordProtectionService::CacheVerdict( |
void PasswordProtectionService::StartRequest( |
const GURL& main_frame_url, |
- LoginReputationClientRequest::TriggerType type) { |
+ LoginReputationClientRequest::TriggerType type, |
+ std::unique_ptr<PasswordProtectionFrames> pending_password_frames) { |
DCHECK_CURRENTLY_ON(BrowserThread::UI); |
- if (!IsPingingEnabled()) |
- return; |
- std::unique_ptr<PasswordProtectionRequest> request = |
- base::MakeUnique<PasswordProtectionRequest>( |
- main_frame_url, type, IsExtendedReporting(), IsIncognito(), |
- GetWeakPtr(), GetRequestTimeoutInMS()); |
+ scoped_refptr<PasswordProtectionRequest> request( |
+ new PasswordProtectionRequest( |
+ main_frame_url, type, std::move(pending_password_frames), |
+ GetWeakPtr(), database_manager_, GetRequestTimeoutInMS())); |
DCHECK(request); |
request->Start(); |
requests_.insert(std::move(request)); |
} |
+void PasswordProtectionService::MaybeStartLowReputationRequest( |
+ const GURL& main_frame_url, |
+ std::unique_ptr<PasswordProtectionFrames> pending_password_frames) { |
+ DCHECK_CURRENTLY_ON(BrowserThread::UI); |
+ if (!IsPingingEnabled()) |
+ return; |
+ |
+ // Don't start request if |main_frame_url| is invalid or is NOT http or https. |
Nathan Parker
2017/04/17 20:28:02
nit: this comment isn't necessary, since the code
Jialiu Lin
2017/04/18 00:58:27
Done.
|
+ if (!main_frame_url.is_valid() || !main_frame_url.SchemeIsHTTPOrHTTPS()) { |
+ return; |
+ } |
+ |
+ StartRequest(main_frame_url, |
+ LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE, |
+ std::move(pending_password_frames)); |
+} |
+ |
void PasswordProtectionService::RequestFinished( |
PasswordProtectionRequest* request, |
std::unique_ptr<LoginReputationClientResponse> response) { |
@@ -222,7 +239,7 @@ void PasswordProtectionService::RequestFinished( |
DCHECK(request); |
// TODO(jialiul): We don't cache verdict for incognito mode for now. |
// Later we may consider temporarily caching verdict. |
- if (!request->is_incognito() && response) |
+ if (response && !IsIncognito()) |
CacheVerdict(request->main_frame_url(), response.get(), base::Time::Now()); |
// Finished processing this request. Remove it from pending list. |
@@ -246,6 +263,11 @@ void PasswordProtectionService::CancelPendingRequests() { |
DCHECK(requests_.empty()); |
} |
+scoped_refptr<SafeBrowsingDatabaseManager> |
+PasswordProtectionService::database_manager() { |
+ return database_manager_; |
+} |
+ |
GURL PasswordProtectionService::GetPasswordProtectionRequestUrl() { |
GURL url(kPasswordProtectionRequestUrl); |
std::string api_key = google_apis::GetAPIKey(); |
@@ -284,10 +306,10 @@ int PasswordProtectionService::GetRequestTimeoutInMS() { |
} |
void PasswordProtectionService::OnMatchCsdWhiteListResult( |
- bool match_whitelist) { |
+ const bool* match_whitelist) { |
UMA_HISTOGRAM_BOOLEAN( |
"PasswordManager.PasswordReuse.MainFrameMatchCsdWhitelist", |
- match_whitelist); |
+ *match_whitelist); |
} |
void PasswordProtectionService::OnURLsDeleted( |
@@ -400,7 +422,10 @@ void PasswordProtectionService::GeneratePathVariantsWithoutQuery( |
// "foo.com/foo/bar/" -> "/foo/bar/" |
std::string PasswordProtectionService::GetCacheExpressionPath( |
const std::string& cache_expression) { |
- DCHECK(!cache_expression.empty()); |
+ // TODO(jialiul): Change this to a DCHECk when SB server is ready. |
+ if (cache_expression.empty()) |
+ return std::string("/"); |
+ |
std::string out_put(cache_expression); |
// Append a trailing slash if needed. |
if (out_put[out_put.length() - 1] != '/') |