Commit fda98c74 authored by Daniel Rubery's avatar Daniel Rubery Committed by Commit Bot

Remove CheckClientDownloadRequest::ShouldDelayVerdicts

This function was originally added when there was a "silent" mode for
WebProtect uploading. Users would see no change if ShouldDelayVerdicts
returns false. With pre-delivery mode implemented, we should be showing
that UX when the admin does not block delivery.

Fixed: 1016896, 980779
Change-Id: Ie82416f365668af6ec135d7b142d35ad23db07d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1874203
Commit-Queue: Bettina Dea <bdea@chromium.org>
Reviewed-by: default avatarBettina Dea <bdea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709312}
parent 44cf0f78
......@@ -308,16 +308,14 @@ void CheckClientDownloadRequest::MaybeStorePingsForDownload(
result, upload_requested, item_, request_data, response_body);
}
bool CheckClientDownloadRequest::ShouldReturnAsynchronousVerdict(
bool CheckClientDownloadRequest::MaybeReturnAsynchronousVerdict(
DownloadCheckResultReason reason) {
return ShouldUploadBinary(reason) && ShouldDelayVerdicts();
}
if (ShouldUploadBinary(reason)) {
callback_.Run(DownloadCheckResult::ASYNC_SCANNING);
return true;
}
bool CheckClientDownloadRequest::ShouldDelayVerdicts() {
int delay_delivery = g_browser_process->local_state()->GetInteger(
prefs::kDelayDeliveryUntilVerdict);
return (delay_delivery == DELAY_DOWNLOADS ||
delay_delivery == DELAY_UPLOADS_AND_DOWNLOADS);
return false;
}
bool CheckClientDownloadRequest::ShouldUploadBinary(
......
......@@ -59,9 +59,9 @@ class CheckClientDownloadRequest : public CheckClientDownloadRequestBase,
const std::string& request_data,
const std::string& response_body) override;
// Returns true if the CheckClientDownloadRequest should return the
// Returns true if the CheckClientDownloadRequest returned the
// ASYNC_SCANNING result while it does deep scanning.
bool ShouldReturnAsynchronousVerdict(
bool MaybeReturnAsynchronousVerdict(
DownloadCheckResultReason reason) override;
// Uploads the binary for deep scanning if the reason and policies indicate
......@@ -81,11 +81,6 @@ class CheckClientDownloadRequest : public CheckClientDownloadRequestBase,
// consults the SendFilesForMalwareCheck enterprise policy.
bool ShouldUploadForMalwareScan(DownloadCheckResultReason reason);
// Returns true when the downloads UX should prevent access to the file until
// the deep scanning verdict has been received. This consults the
// DelayDeliveryUntilVerdict enterprise policy.
bool ShouldDelayVerdicts();
// Called when deep scanning is complete. Where appropriate, it updates the
// download UX, and sends a real time report about the download.
void OnDeepScanningComplete(BinaryUploadService::Result result,
......
......@@ -222,8 +222,7 @@ void CheckClientDownloadRequestBase::FinishRequest(
UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckDownloadStats", reason,
REASON_MAX);
if (ShouldReturnAsynchronousVerdict(reason)) {
std::move(callback_).Run(DownloadCheckResult::ASYNC_SCANNING);
if (MaybeReturnAsynchronousVerdict(reason)) {
timeout_closure_.Cancel();
} else {
std::move(callback_).Run(result);
......
......@@ -129,8 +129,8 @@ class CheckClientDownloadRequestBase {
const std::string& response_body) = 0;
// Called when finishing the request, to determine whether asynchronous
// scanning is pending.
virtual bool ShouldReturnAsynchronousVerdict(
// scanning is pending. Returns whether an asynchronous verdict was provided.
virtual bool MaybeReturnAsynchronousVerdict(
DownloadCheckResultReason reason) = 0;
// Called after receiving, or failing to receive a response from the server.
......
......@@ -151,7 +151,7 @@ void CheckNativeFileSystemWriteRequest::MaybeStorePingsForDownload(
// TODO(https://crbug.com/996797): Integrate with DownloadFeedbackService.
}
bool CheckNativeFileSystemWriteRequest::ShouldReturnAsynchronousVerdict(
bool CheckNativeFileSystemWriteRequest::MaybeReturnAsynchronousVerdict(
DownloadCheckResultReason reason) {
return false;
}
......
......@@ -49,7 +49,7 @@ class CheckNativeFileSystemWriteRequest
bool upload_requested,
const std::string& request_data,
const std::string& response_body) override;
bool ShouldReturnAsynchronousVerdict(
bool MaybeReturnAsynchronousVerdict(
DownloadCheckResultReason reason) override;
bool ShouldUploadBinary(DownloadCheckResultReason reason) override;
void UploadBinary(DownloadCheckResultReason reason) override;
......
......@@ -2880,7 +2880,7 @@ TEST_F(DownloadProtectionServiceTest,
&item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this), run_loop.QuitClosure()));
run_loop.Run();
EXPECT_TRUE(IsResult(DownloadCheckResult::SAFE));
EXPECT_TRUE(IsResult(DownloadCheckResult::ASYNC_SCANNING));
EXPECT_TRUE(HasClientDownloadRequest());
ClearClientDownloadRequest();
}
......@@ -2929,7 +2929,7 @@ TEST_F(DownloadProtectionServiceTest, LargeFileBlockedByPreference) {
&item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this), run_loop.QuitClosure()));
run_loop.Run();
EXPECT_TRUE(IsResult(DownloadCheckResult::SAFE));
EXPECT_TRUE(IsResult(DownloadCheckResult::ASYNC_SCANNING));
EXPECT_TRUE(HasClientDownloadRequest());
ClearClientDownloadRequest();
}
......
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