Skip to content

Commit

Permalink
[TailoredWarning] Send download discard report on downloads page.
Browse files Browse the repository at this point in the history 
To better measure the impact of the download warnings, send reports
when the user clicks discard on the downloads page. The did_proceed
field is set to false and the report type is set to
DANGEROUS_DOWNLOAD_RECOVERY in this case.

This is gated by the kSafeBrowsingCsbrrNewDownloadTrigger flag.

Bug: 1363368
Change-Id: I4a6d2aa6aa28b541043a12d1ee3d5813afa46685
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3957478
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1067439}
  • Loading branch information
Xinghui Lu authored and Chromium LUCI CQ committed Nov 4, 2022
1 parent 812a5ab commit 29c3ed6
Show file tree
Hide file tree
Showing 2 changed files with 210 additions and 1 deletion.
28 changes: 28 additions & 0 deletions chrome/browser/ui/webui/downloads/downloads_dom_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@
#include "chrome/browser/download/drag_download_item.h"
#include "chrome/browser/platform_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/browser/ui/webui/downloads/downloads.mojom.h"
#include "chrome/browser/ui/webui/fileicon_source.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "components/download/public/common/download_item.h"
#include "components/prefs/pref_service.h"
#include "components/safe_browsing/core/common/features.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/download_manager.h"
Expand Down Expand Up @@ -180,6 +182,32 @@ void DownloadsDOMHandler::SaveDangerousRequiringGesture(const std::string& id) {

void DownloadsDOMHandler::DiscardDangerous(const std::string& id) {
CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_DISCARD_DANGEROUS);
download::DownloadItem* download = GetDownloadByStringId(id);
if (download) {
// If this download is no longer dangerous, is already canceled or
// completed, don't send any report.
// Only sends dangerous download discard report if :
// 1. Download is dangerous, and
// 2. Download is not canceled or completed, and
// 3. Download URL is not empty, and
// 4. User is not in incognito mode, and
// 5. The new CSBRR trigger feature is enabled.
if (download->IsDangerous() && !download->IsDone() &&
!download->GetURL().is_empty() &&
!GetMainNotifierManager()->GetBrowserContext()->IsOffTheRecord() &&
base::FeatureList::IsEnabled(
safe_browsing::kSafeBrowsingCsbrrNewDownloadTrigger)) {
safe_browsing::SafeBrowsingService* sb_service =
g_browser_process->safe_browsing_service();
if (sb_service) {
sb_service->SendDownloadReport(
download,
safe_browsing::ClientSafeBrowsingReportRequest::
DANGEROUS_DOWNLOAD_RECOVERY,
/*did_proceed=*/false, /*show_download_in_folder=*/absl::nullopt);
}
}
}
RemoveDownloadInArgs(id);
}

Expand Down
183 changes: 182 additions & 1 deletion chrome/browser/ui/webui/downloads/downloads_dom_handler_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@
#include <vector>

#include "chrome/browser/download/download_item_model.h"
#include "chrome/browser/safe_browsing/download_protection/download_protection_service.h"
#include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
#include "chrome/browser/ui/webui/downloads/downloads.mojom.h"
#include "chrome/browser/ui/webui/downloads/mock_downloads_page.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "components/download/public/common/mock_download_item.h"
#include "components/safe_browsing/core/common/features.h"
#include "components/safe_browsing/core/common/proto/csd.pb.h"
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/mock_download_manager.h"
#include "content/public/test/test_web_ui.h"
Expand All @@ -22,6 +27,8 @@

namespace {

const char kTestDangerousDownloadUrl[] = "http://evildownload.com";

class TestDownloadsDOMHandler : public DownloadsDOMHandler {
public:
TestDownloadsDOMHandler(mojo::PendingRemote<downloads::mojom::Page> page,
Expand All @@ -42,7 +49,7 @@ class TestDownloadsDOMHandler : public DownloadsDOMHandler {
// A fixture to test DownloadsDOMHandler.
class DownloadsDOMHandlerTest : public testing::Test {
public:
DownloadsDOMHandlerTest() {}
DownloadsDOMHandlerTest() = default;

// testing::Test:
void SetUp() override {
Expand Down Expand Up @@ -128,3 +135,177 @@ TEST_F(DownloadsDOMHandlerTest, ClearAll) {
EXPECT_CALL(completed, Remove());
handler.FinalizeRemovals();
}

class DownloadsDOMHandlerWithFakeSafeBrowsingTest
: public DownloadsDOMHandlerTest {
public:
DownloadsDOMHandlerWithFakeSafeBrowsingTest()
: test_safe_browsing_factory_(
new safe_browsing::TestSafeBrowsingServiceFactory()) {
feature_list_.InitAndDisableFeature(
safe_browsing::kSafeBrowsingCsbrrNewDownloadTrigger);
}

void SetUp() override {
browser_process_ = TestingBrowserProcess::GetGlobal();
safe_browsing::SafeBrowsingServiceInterface::RegisterFactory(
test_safe_browsing_factory_.get());
sb_service_ = static_cast<safe_browsing::SafeBrowsingService*>(
safe_browsing::SafeBrowsingService::CreateSafeBrowsingService());
browser_process_->SetSafeBrowsingService(sb_service_.get());
sb_service_->Initialize();
base::RunLoop().RunUntilIdle();

DownloadsDOMHandlerTest::SetUp();
}

void TearDown() override {
browser_process_->safe_browsing_service()->ShutDown();
browser_process_->SetSafeBrowsingService(nullptr);
safe_browsing::SafeBrowsingServiceInterface::RegisterFactory(nullptr);
DownloadsDOMHandlerTest::TearDown();
base::RunLoop().RunUntilIdle();
}

protected:
void SetUpDangerousDownload() {
EXPECT_CALL(dangerous_download_, IsDangerous())
.WillRepeatedly(testing::Return(true));
EXPECT_CALL(dangerous_download_, IsDone())
.WillRepeatedly(testing::Return(false));
EXPECT_CALL(dangerous_download_, GetURL())
.WillRepeatedly(testing::ReturnRef(download_url_));
EXPECT_CALL(*manager(), GetDownload(1))
.WillRepeatedly(testing::Return(&dangerous_download_));
safe_browsing::DownloadProtectionService::SetDownloadProtectionData(
&dangerous_download_, "token",
safe_browsing::ClientDownloadResponse::DANGEROUS,
safe_browsing::ClientDownloadResponse::TailoredVerdict());
}

std::unique_ptr<safe_browsing::TestSafeBrowsingServiceFactory>
test_safe_browsing_factory_;
raw_ptr<TestingBrowserProcess> browser_process_;
scoped_refptr<safe_browsing::SafeBrowsingService> sb_service_;
GURL download_url_ = GURL(kTestDangerousDownloadUrl);
testing::StrictMock<download::MockDownloadItem> dangerous_download_;

private:
base::test::ScopedFeatureList feature_list_;
};

TEST_F(DownloadsDOMHandlerWithFakeSafeBrowsingTest, DiscardDangerous) {
SetUpDangerousDownload();

TestDownloadsDOMHandler handler(page_.BindAndGetRemote(), manager(),
web_ui());

// Dangerous items should be removed on DiscardDangerous.
EXPECT_CALL(dangerous_download_, Remove());
handler.DiscardDangerous("1");

// Verify that dangerous download report is not sent because the feature flag
// is disabled.
EXPECT_TRUE(test_safe_browsing_factory_->test_safe_browsing_service()
->serilized_download_report()
.empty());
}

class DownloadsDOMHandlerWithFakeSafeBrowsingTestNewCsbrrTrigger
: public DownloadsDOMHandlerWithFakeSafeBrowsingTest {
public:
DownloadsDOMHandlerWithFakeSafeBrowsingTestNewCsbrrTrigger() {
feature_list_.InitAndEnableFeature(
safe_browsing::kSafeBrowsingCsbrrNewDownloadTrigger);
}

private:
base::test::ScopedFeatureList feature_list_;
};

TEST_F(DownloadsDOMHandlerWithFakeSafeBrowsingTestNewCsbrrTrigger,
DiscardDangerous) {
SetUpDangerousDownload();

TestDownloadsDOMHandler handler(page_.BindAndGetRemote(), manager(),
web_ui());

// Dangerous items should be removed on DiscardDangerous.
EXPECT_CALL(dangerous_download_, Remove());
handler.DiscardDangerous("1");

// Verify that dangerous download report is sent.
safe_browsing::ClientSafeBrowsingReportRequest expected_report;
std::string expected_serialized_report;
expected_report.set_url(GURL(kTestDangerousDownloadUrl).spec());
expected_report.set_type(safe_browsing::ClientSafeBrowsingReportRequest::
DANGEROUS_DOWNLOAD_RECOVERY);
expected_report.set_did_proceed(false);
expected_report.set_download_verdict(
safe_browsing::ClientDownloadResponse::DANGEROUS);
expected_report.set_token("token");
expected_report.SerializeToString(&expected_serialized_report);
EXPECT_EQ(expected_serialized_report,
test_safe_browsing_factory_->test_safe_browsing_service()
->serilized_download_report());
}

TEST_F(DownloadsDOMHandlerWithFakeSafeBrowsingTestNewCsbrrTrigger,
DiscardDangerous_IsDone) {
SetUpDangerousDownload();
EXPECT_CALL(dangerous_download_, IsDone())
.WillRepeatedly(testing::Return(true));

TestDownloadsDOMHandler handler(page_.BindAndGetRemote(), manager(),
web_ui());

EXPECT_CALL(dangerous_download_, Remove());
handler.DiscardDangerous("1");

// Verify that dangerous download report is not sent because the download is
// already in complete state.
EXPECT_TRUE(test_safe_browsing_factory_->test_safe_browsing_service()
->serilized_download_report()
.empty());
}

TEST_F(DownloadsDOMHandlerWithFakeSafeBrowsingTestNewCsbrrTrigger,
DiscardDangerous_EmptyURL) {
SetUpDangerousDownload();
GURL empty_url = GURL();
EXPECT_CALL(dangerous_download_, GetURL())
.WillRepeatedly(testing::ReturnRef(empty_url));

TestDownloadsDOMHandler handler(page_.BindAndGetRemote(), manager(),
web_ui());

EXPECT_CALL(dangerous_download_, Remove());
handler.DiscardDangerous("1");

// Verify that dangerous download report is not sent because the URL is empty.
EXPECT_TRUE(test_safe_browsing_factory_->test_safe_browsing_service()
->serilized_download_report()
.empty());
}

TEST_F(DownloadsDOMHandlerWithFakeSafeBrowsingTestNewCsbrrTrigger,
DiscardDangerous_Incognito) {
SetUpDangerousDownload();
TestingProfile::Builder otr_profile_builder;
otr_profile_builder.DisallowBrowserWindows();
Profile* incognito_profile = otr_profile_builder.BuildIncognito(profile());
ON_CALL(*manager(), GetBrowserContext())
.WillByDefault(testing::Return(incognito_profile));

TestDownloadsDOMHandler handler(page_.BindAndGetRemote(), manager(),
web_ui());

EXPECT_CALL(dangerous_download_, Remove());
handler.DiscardDangerous("1");

// Verify that dangerous download report is not sent because it's in
// Incognito.
EXPECT_TRUE(test_safe_browsing_factory_->test_safe_browsing_service()
->serilized_download_report()
.empty());
}

0 comments on commit 29c3ed6

Please sign in to comment.