Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(741)

Unified Diff: chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc

Issue 2286743004: Sends switches to the Software Reporter to enable matching data collection. (Closed)
Patch Set: Always clean state on test setup Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/safe_browsing/srt_client_info_win.cc ('k') | chrome/browser/safe_browsing/srt_fetcher_win.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
diff --git a/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc b/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
index e77870481184d9455df698b491d7641195865259..e8100bf555ec1cc4b5a83912a409603936617993 100644
--- a/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
+++ b/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
@@ -4,17 +4,23 @@
#include "chrome/browser/safe_browsing/srt_fetcher_win.h"
+#include <iterator>
#include <memory>
+#include <set>
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/files/file_path.h"
#include "base/run_loop.h"
+#include "base/test/scoped_feature_list.h"
#include "base/test/test_simple_task_runner.h"
#include "base/time/time.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/srt_client_info_win.h"
#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/component_updater/pref_names.h"
#include "components/prefs/pref_service.h"
@@ -24,6 +30,10 @@ namespace safe_browsing {
namespace {
+const char* const kExpectedSwitches[] = {kExtendedSafeBrowsingEnabledSwitch,
+ kChromeVersionSwitch,
+ kChromeChannelSwitch};
+
class SRTFetcherTest : public InProcessBrowserTest,
public SwReporterTestingDelegate {
public:
@@ -33,6 +43,11 @@ class SRTFetcherTest : public InProcessBrowserTest,
SetSwReporterTestingDelegate(this);
}
+ void SetUpOnMainThread() override {
+ InProcessBrowserTest::SetUpOnMainThread();
+ ClearLastTimeSentReport();
+ }
+
void TearDownInProcessBrowserTestFixture() override {
SetSwReporterTestingDelegate(nullptr);
}
@@ -56,13 +71,19 @@ class SRTFetcherTest : public InProcessBrowserTest,
void NotifyReporterDone() override { reporter_done_notified_ = true; }
- void SetDaysSinceLastReport(int days) {
+ // Sets |path| in the local state to a date corresponding to |days| days ago.
+ void SetDateInLocalState(const std::string& path, int days) {
PrefService* local_state = g_browser_process->local_state();
- local_state->SetInt64(prefs::kSwReporterLastTimeTriggered,
+ DCHECK_NE(local_state, nullptr);
+ local_state->SetInt64(path,
(base::Time::Now() - base::TimeDelta::FromDays(days))
.ToInternalValue());
}
+ void SetDaysSinceLastReport(int days) {
+ SetDateInLocalState(prefs::kSwReporterLastTimeTriggered, days);
+ }
+
void ExpectToRunAgain(int days) {
ASSERT_TRUE(task_runner_->HasPendingTask());
EXPECT_LE(task_runner_->NextPendingTaskDelay(),
@@ -71,6 +92,49 @@ class SRTFetcherTest : public InProcessBrowserTest,
base::TimeDelta::FromDays(days) - base::TimeDelta::FromHours(1));
}
+ // Clears local state for last time the software reporter sent logs to |days|
+ // days ago. This prevents potential false positives that could arise from
+ // state not properly cleaned between successive tests.
+ void ClearLastTimeSentReport() {
+ DCHECK_NE(g_browser_process, nullptr);
+ PrefService* local_state = g_browser_process->local_state();
+ DCHECK_NE(local_state, nullptr);
+ local_state->ClearPref(prefs::kSwReporterLastTimeSentReport);
+ }
+
+ // Sets local state for last time the software reporter sent logs to |days|
+ // days ago.
+ void SetLastTimeSentReport(int days) {
+ SetDateInLocalState(prefs::kSwReporterLastTimeSentReport, days);
+ }
+
+ int64_t GetLastTimeSentReport() {
+ const PrefService* local_state = g_browser_process->local_state();
+ DCHECK_NE(local_state, nullptr);
+ DCHECK(local_state->HasPrefPath(prefs::kSwReporterLastTimeSentReport));
+ return local_state->GetInt64(prefs::kSwReporterLastTimeSentReport);
+ }
+
+ void ExpectLastTimeSentReportNotSet() {
+ PrefService* local_state = g_browser_process->local_state();
+ DCHECK_NE(local_state, nullptr);
+ EXPECT_FALSE(
+ local_state->HasPrefPath(prefs::kSwReporterLastTimeSentReport));
+ }
+
+ void ExpectLastReportSentInTheLastHour() {
+ const PrefService* local_state = g_browser_process->local_state();
+ DCHECK_NE(local_state, nullptr);
+ const base::Time now = base::Time::Now();
+ const base::Time last_time_sent_logs = base::Time::FromInternalValue(
+ local_state->GetInt64(prefs::kSwReporterLastTimeSentReport));
+
+ // Checks if the last time sent logs is set as no more than one hour ago,
+ // which should be enough time if the execution does not fail.
+ EXPECT_LT(now - base::TimeDelta::FromHours(1), last_time_sent_logs);
+ EXPECT_LT(last_time_sent_logs, now);
+ }
+
void TestReporterLaunchCycle(int expected_launch_count,
const base::FilePath& expected_launch_path) {
// This test has an unfortunate amount of knowledge of the internals of
@@ -150,6 +214,30 @@ class SRTFetcherTest : public InProcessBrowserTest,
reporter_done_notified_ = false;
}
+ // Expects |reporter_launch_parameters_| to contain exactly the command line
+ // switches specified in |expected_switches|.
+ void ExpectLoggingSwitches(const std::set<std::string>& expected_switches) {
+ const base::CommandLine::SwitchMap& invocation_switches =
+ reporter_launch_parameters_.command_line.GetSwitches();
+ EXPECT_EQ(expected_switches.size(), invocation_switches.size());
+ // Checks if all expected switches are in the invocation switches. It's not
+ // necessary to check if all invocation switches are expected, since we
+ // checked if both sets should have the same size.
+ for (const std::string& expected_switch : expected_switches) {
+ EXPECT_NE(invocation_switches.find(expected_switch),
+ invocation_switches.end());
+ }
+ }
+
+ void EnableSBExtendedReporting() {
+ Browser* browser = chrome::FindLastActive();
+ ASSERT_NE(browser, nullptr);
+ Profile* profile = browser->profile();
+ ASSERT_NE(profile, nullptr);
+ profile->GetPrefs()->SetBoolean(
+ prefs::kSafeBrowsingExtendedReportingEnabled, true);
+ }
+
scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
bool prompt_trigger_called_ = false;
int reporter_launch_count_ = 0;
@@ -306,4 +394,75 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ParameterChange) {
}
}
+IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_FeatureDisabled) {
+ exit_code_to_report_ = kSwReporterNothingFound;
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndDisableFeature(
+ kSwReporterExtendedSafeBrowsingFeature);
+ RunReporter();
+ TestReporterLaunchCycle(1, base::FilePath());
+ ExpectLoggingSwitches({/*expect no switches*/});
+ ExpectLastTimeSentReportNotSet();
+}
+
+IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_NoSBExtendedReporting) {
+ exit_code_to_report_ = kSwReporterNothingFound;
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(
+ kSwReporterExtendedSafeBrowsingFeature);
+ RunReporter();
+ TestReporterLaunchCycle(1, base::FilePath());
+ ExpectLoggingSwitches({/*expect no switches*/});
+ ExpectLastTimeSentReportNotSet();
+}
+
+IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledFirstRun) {
+ exit_code_to_report_ = kSwReporterNothingFound;
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(
+ kSwReporterExtendedSafeBrowsingFeature);
+ EnableSBExtendedReporting();
+ // Note: don't set last time sent logs in the local state.
+ // SBER is enabled and there is no record in the local state of the last time
+ // logs have been sent, so we should send logs in this run.
+ RunReporter();
+ TestReporterLaunchCycle(1, base::FilePath());
+ ExpectLoggingSwitches(std::set<std::string>(std::begin(kExpectedSwitches),
+ std::end(kExpectedSwitches)));
+ ExpectLastReportSentInTheLastHour();
+}
+
+IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledNoRecentLogging) {
+ exit_code_to_report_ = kSwReporterNothingFound;
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(
+ kSwReporterExtendedSafeBrowsingFeature);
+ // SBER is enabled and last time logs were sent was more than
+ // |kDaysBetweenReporterLogsSent| day ago, so we should send logs in this run.
+ EnableSBExtendedReporting();
+ SetLastTimeSentReport(kDaysBetweenReporterLogsSent + 3);
+ RunReporter();
+ TestReporterLaunchCycle(1, base::FilePath());
+ ExpectLoggingSwitches(std::set<std::string>(std::begin(kExpectedSwitches),
+ std::end(kExpectedSwitches)));
+ ExpectLastReportSentInTheLastHour();
+}
+
+IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledRecentlyLogged) {
+ exit_code_to_report_ = kSwReporterNothingFound;
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(
+ kSwReporterExtendedSafeBrowsingFeature);
+ // SBER is enabled, but logs have been sent less than
+ // |kDaysBetweenReporterLogsSent| day ago, so we shouldn't send any logs in
+ // this run.
+ EnableSBExtendedReporting();
+ SetLastTimeSentReport(kDaysBetweenReporterLogsSent - 1);
+ int64_t last_time_sent_logs = GetLastTimeSentReport();
+ RunReporter();
+ TestReporterLaunchCycle(1, base::FilePath());
+ ExpectLoggingSwitches(std::set<std::string>{/*expect no switches*/});
+ EXPECT_EQ(last_time_sent_logs, GetLastTimeSentReport());
+}
+
} // namespace safe_browsing
« no previous file with comments | « chrome/browser/safe_browsing/srt_client_info_win.cc ('k') | chrome/browser/safe_browsing/srt_fetcher_win.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698