From 93ef5ea059a6cd8c60c6de6e8e4ce3d702695623 Mon Sep 17 00:00:00 2001 From: "grt@chromium.org" Date: Fri, 25 Jul 2014 05:45:00 +0000 Subject: [PATCH] More fine-grained pruning in safe browsing incident reporting service. BUG=396398 Review URL: https://codereview.chromium.org/411793004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285491 0039d316-1c4b-4281-b951-d872f2087c98 --- .../prefs/chrome_pref_service_factory.cc | 7 +- chrome/browser/profiles/profile.cc | 3 + .../safe_browsing/incident_handler_util.cc | 26 +++ .../safe_browsing/incident_handler_util.h | 27 +++ .../incident_reporting_service.cc | 188 ++++++++++++++---- .../incident_reporting_service_unittest.cc | 73 ++++++- .../tracked_preference_incident_handlers.cc | 29 +++ .../tracked_preference_incident_handlers.h | 26 +++ ...d_preference_incident_handlers_unittest.cc | 63 ++++++ chrome/chrome_browser.gypi | 4 + chrome/chrome_tests_unit.gypi | 1 + chrome/common/pref_names.cc | 3 + chrome/common/pref_names.h | 1 + 13 files changed, 413 insertions(+), 38 deletions(-) create mode 100644 chrome/browser/safe_browsing/incident_handler_util.cc create mode 100644 chrome/browser/safe_browsing/incident_handler_util.h create mode 100644 chrome/browser/safe_browsing/tracked_preference_incident_handlers.cc create mode 100644 chrome/browser/safe_browsing/tracked_preference_incident_handlers.h create mode 100644 chrome/browser/safe_browsing/tracked_preference_incident_handlers_unittest.cc diff --git a/chrome/browser/prefs/chrome_pref_service_factory.cc b/chrome/browser/prefs/chrome_pref_service_factory.cc index 9a3df11caeb61..c4bc31475bccb 100644 --- a/chrome/browser/prefs/chrome_pref_service_factory.cc +++ b/chrome/browser/prefs/chrome_pref_service_factory.cc @@ -195,10 +195,15 @@ const PrefHashFilter::TrackedPreferenceMetadata kTrackedPrefs[] = { PrefHashFilter::ENFORCE_ON_LOAD, PrefHashFilter::TRACKING_STRATEGY_ATOMIC }, + { + 18, prefs::kSafeBrowsingIncidentsSent, + PrefHashFilter::ENFORCE_ON_LOAD, + PrefHashFilter::TRACKING_STRATEGY_ATOMIC + }, }; // The count of tracked preferences IDs across all platforms. -const size_t kTrackedPrefsReportingIDsCount = 18; +const size_t kTrackedPrefsReportingIDsCount = 19; COMPILE_ASSERT(kTrackedPrefsReportingIDsCount >= arraysize(kTrackedPrefs), need_to_increment_ids_count); diff --git a/chrome/browser/profiles/profile.cc b/chrome/browser/profiles/profile.cc index daa082820030a..fc5cde6eaf12f 100644 --- a/chrome/browser/profiles/profile.cc +++ b/chrome/browser/profiles/profile.cc @@ -118,6 +118,9 @@ void Profile::RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { prefs::kSafeBrowsingIncidentReportSent, false, user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); + registry->RegisterDictionaryPref( + prefs::kSafeBrowsingIncidentsSent, + user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); #if defined(ENABLE_GOOGLE_NOW) registry->RegisterBooleanPref( prefs::kGoogleGeolocationAccessEnabled, diff --git a/chrome/browser/safe_browsing/incident_handler_util.cc b/chrome/browser/safe_browsing/incident_handler_util.cc new file mode 100644 index 0000000000000..e0ef4586efb78 --- /dev/null +++ b/chrome/browser/safe_browsing/incident_handler_util.cc @@ -0,0 +1,26 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/safe_browsing/incident_handler_util.h" + +#include + +#include "base/hash.h" +#include "base/logging.h" +#include "third_party/protobuf/src/google/protobuf/message_lite.h" + +namespace safe_browsing { + +// Computes a simple hash digest over the serialized form of |message|. +// |message| must be in a canonical form. +uint32_t HashMessage(const google::protobuf::MessageLite& message) { + std::string message_string; + if (!message.SerializeToString(&message_string)) { + NOTREACHED(); + return 0; + } + return base::Hash(message_string); +} + +} // namespace safe_browsing diff --git a/chrome/browser/safe_browsing/incident_handler_util.h b/chrome/browser/safe_browsing/incident_handler_util.h new file mode 100644 index 0000000000000..a8a84ad9f019a --- /dev/null +++ b/chrome/browser/safe_browsing/incident_handler_util.h @@ -0,0 +1,27 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_SAFE_BROWSING_INCIDENT_HANDLER_UTIL_H_ +#define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_HANDLER_UTIL_H_ + +#include + +namespace google { +namespace protobuf { + +class MessageLite; + +} // namespace protobuf +} // namespace google + +namespace safe_browsing { + +// Computes a simple hash digest over the serialized form of |message|. +// |message| must be in a canonical form. For example, fields set to their +// default values should be cleared. +uint32_t HashMessage(const google::protobuf::MessageLite& message); + +} // namespace safe_browsing + +#endif // CHROME_BROWSER_SAFE_BROWSING_INCIDENT_HANDLER_UTIL_H_ diff --git a/chrome/browser/safe_browsing/incident_reporting_service.cc b/chrome/browser/safe_browsing/incident_reporting_service.cc index 46cb9f3a35d45..6c8ce1b9f562f 100644 --- a/chrome/browser/safe_browsing/incident_reporting_service.cc +++ b/chrome/browser/safe_browsing/incident_reporting_service.cc @@ -11,9 +11,12 @@ #include "base/metrics/histogram.h" #include "base/prefs/pref_service.h" +#include "base/prefs/scoped_user_pref_update.h" #include "base/process/process_info.h" #include "base/stl_util.h" +#include "base/strings/string_number_conversions.h" #include "base/threading/sequenced_worker_pool.h" +#include "base/values.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/prefs/tracked/tracked_preference_validation_delegate.h" @@ -23,6 +26,7 @@ #include "chrome/browser/safe_browsing/incident_report_uploader_impl.h" #include "chrome/browser/safe_browsing/preference_validation_delegate.h" #include "chrome/browser/safe_browsing/safe_browsing_service.h" +#include "chrome/browser/safe_browsing/tracked_preference_incident_handlers.h" #include "chrome/common/pref_names.h" #include "chrome/common/safe_browsing/csd.pb.h" #include "content/public/browser/browser_thread.h" @@ -33,28 +37,67 @@ namespace safe_browsing { namespace { +// The type of an incident. Used for user metrics and for pruning of +// previously-reported incidents. enum IncidentType { // Start with 1 rather than zero; otherwise there won't be enough buckets for // the histogram. TRACKED_PREFERENCE = 1, + // Values for new incident types go here. NUM_INCIDENT_TYPES }; +// The action taken for an incident; used for user metrics (see +// LogIncidentDataType). enum IncidentDisposition { DROPPED, ACCEPTED, }; +// The state persisted for a specific instance of an incident to enable pruning +// of previously-reported incidents. +struct PersistentIncidentState { + // The type of the incident. + IncidentType type; + + // The key for a specific instance of an incident. + std::string key; + + // A hash digest representing a specific instance of an incident. + uint32_t digest; +}; + +// The amount of time the service will wait to collate incidents. const int64 kDefaultUploadDelayMs = 1000 * 60; // one minute -void LogIncidentDataType( - IncidentDisposition disposition, +// Returns the number of incidents contained in |incident|. The result is +// expected to be 1. Used in DCHECKs. +size_t CountIncidents(const ClientIncidentReport_IncidentData& incident) { + size_t result = 0; + if (incident.has_tracked_preference()) + ++result; + // Add detection for new incident types here. + return result; +} + +// Returns the type of incident contained in |incident_data|. +IncidentType GetIncidentType( const ClientIncidentReport_IncidentData& incident_data) { - IncidentType type = TRACKED_PREFERENCE; + if (incident_data.has_tracked_preference()) + return TRACKED_PREFERENCE; - // Add a switch statement once other types are supported. - DCHECK(incident_data.has_tracked_preference()); + // Add detection for new incident types here. + COMPILE_ASSERT(TRACKED_PREFERENCE + 1 == NUM_INCIDENT_TYPES, + add_support_for_new_types); + NOTREACHED(); + return NUM_INCIDENT_TYPES; +} +// Logs the type of incident in |incident_data| to a user metrics histogram. +void LogIncidentDataType( + IncidentDisposition disposition, + const ClientIncidentReport_IncidentData& incident_data) { + IncidentType type = GetIncidentType(incident_data); if (disposition == ACCEPTED) { UMA_HISTOGRAM_ENUMERATION("SBIRS.Incident", type, NUM_INCIDENT_TYPES); } else { @@ -64,6 +107,56 @@ void LogIncidentDataType( } } +// Computes the persistent state for an incident. +PersistentIncidentState ComputeIncidentState( + const ClientIncidentReport_IncidentData& incident) { + PersistentIncidentState state = {GetIncidentType(incident)}; + switch (state.type) { + case TRACKED_PREFERENCE: + state.key = GetTrackedPreferenceIncidentKey(incident); + state.digest = GetTrackedPreferenceIncidentDigest(incident); + break; + // Add handling for new incident types here. + default: + COMPILE_ASSERT(TRACKED_PREFERENCE + 1 == NUM_INCIDENT_TYPES, + add_support_for_new_types); + NOTREACHED(); + break; + } + return state; +} + +// Returns true if the incident described by |state| has already been reported +// based on the bookkeeping in the |incidents_sent| preference dictionary. +bool IncidentHasBeenReported(const base::DictionaryValue* incidents_sent, + const PersistentIncidentState& state) { + const base::DictionaryValue* type_dict = NULL; + std::string digest_string; + return (incidents_sent && + incidents_sent->GetDictionaryWithoutPathExpansion( + base::IntToString(state.type), &type_dict) && + type_dict->GetStringWithoutPathExpansion(state.key, &digest_string) && + digest_string == base::UintToString(state.digest)); +} + +// Marks the incidents described by |states| as having been reported +// in |incidents_set|. +void MarkIncidentsAsReported(const std::vector& states, + base::DictionaryValue* incidents_sent) { + for (size_t i = 0; i < states.size(); ++i) { + const PersistentIncidentState& data = states[i]; + base::DictionaryValue* type_dict = NULL; + const std::string type_string(base::IntToString(data.type)); + if (!incidents_sent->GetDictionaryWithoutPathExpansion(type_string, + &type_dict)) { + type_dict = new base::DictionaryValue(); + incidents_sent->SetWithoutPathExpansion(type_string, type_dict); + } + type_dict->SetStringWithoutPathExpansion(data.key, + base::UintToString(data.digest)); + } +} + } // namespace struct IncidentReportingService::ProfileContext { @@ -82,6 +175,9 @@ struct IncidentReportingService::ProfileContext { class IncidentReportingService::UploadContext { public: + typedef std::map > + PersistentIncidentStateCollection; + explicit UploadContext(scoped_ptr report); ~UploadContext(); @@ -91,8 +187,8 @@ class IncidentReportingService::UploadContext { // The uploader in use. This is NULL until the CSD killswitch is checked. scoped_ptr uploader; - // The set of profiles from which incidents in |report| originated. - std::vector profiles; + // A mapping of profiles to the data to be persisted upon successful upload. + PersistentIncidentStateCollection profiles_to_state; private: DISALLOW_COPY_AND_ASSIGN(UploadContext); @@ -261,17 +357,9 @@ void IncidentReportingService::OnProfileDestroyed(Profile* profile) { delete it->second; profiles_.erase(it); - // Remove the association with this profile from any pending uploads. - for (size_t i = 0; i < uploads_.size(); ++i) { - UploadContext* upload = uploads_[i]; - std::vector::iterator it = - std::find(upload->profiles.begin(), upload->profiles.end(), profile); - if (it != upload->profiles.end()) { - *it = upload->profiles.back(); - upload->profiles.resize(upload->profiles.size() - 1); - break; - } - } + // Remove the association with this profile from all pending uploads. + for (size_t i = 0; i < uploads_.size(); ++i) + uploads_[i]->profiles_to_state.erase(profile); } void IncidentReportingService::AddIncident( @@ -280,6 +368,7 @@ void IncidentReportingService::AddIncident( DCHECK(thread_checker_.CalledOnValidThread()); // Incidents outside the context of a profile are not supported at the moment. DCHECK(profile); + DCHECK_EQ(1U, CountIncidents(*incident_data)); ProfileContext* context = GetProfileContext(profile); // It is forbidden to call this function with a destroyed profile. @@ -516,10 +605,10 @@ void IncidentReportingService::UploadIfCollectionComplete() { process->set_extended_consent(false); // Collect incidents across all profiles participating in safe browsing. Drop // incidents if the profile stopped participating before collection completed. - // Prune incidents if the profile has already submitted any incidents. - // Associate the participating profiles with the upload. + // Prune previously submitted incidents. + // Associate the profiles and their incident data with the upload. size_t prune_count = 0; - std::vector profiles; + UploadContext::PersistentIncidentStateCollection profiles_to_state; for (ProfileContextCollection::iterator scan = profiles_.begin(); scan != profiles_.end(); ++scan) { @@ -537,21 +626,48 @@ void IncidentReportingService::UploadIfCollectionComplete() { LogIncidentDataType(DROPPED, *context->incidents[i]); } context->incidents.clear(); - } else if (prefs->GetBoolean(prefs::kSafeBrowsingIncidentReportSent)) { - // Prune all incidents. - // TODO(grt): Only prune previously submitted incidents; - // http://crbug.com/383043. - prune_count += context->incidents.size(); + continue; + } + std::vector states; + const base::DictionaryValue* incidents_sent = + prefs->GetDictionary(prefs::kSafeBrowsingIncidentsSent); + // Prep persistent data and prune any incidents already sent. + for (size_t i = 0; i < context->incidents.size(); ++i) { + ClientIncidentReport_IncidentData* incident = context->incidents[i]; + const PersistentIncidentState state = ComputeIncidentState(*incident); + if (IncidentHasBeenReported(incidents_sent, state)) { + ++prune_count; + delete context->incidents[i]; + context->incidents[i] = NULL; + } else { + states.push_back(state); + } + } + if (prefs->GetBoolean(prefs::kSafeBrowsingIncidentReportSent)) { + // Prune all incidents as if they had been reported, migrating to the new + // technique. TODO(grt): remove this branch after it has shipped. + for (size_t i = 0; i < context->incidents.size(); ++i) { + if (context->incidents[i]) + ++prune_count; + } context->incidents.clear(); + prefs->ClearPref(prefs::kSafeBrowsingIncidentReportSent); + DictionaryPrefUpdate pref_update(prefs, + prefs::kSafeBrowsingIncidentsSent); + MarkIncidentsAsReported(states, pref_update.Get()); } else { for (size_t i = 0; i < context->incidents.size(); ++i) { ClientIncidentReport_IncidentData* incident = context->incidents[i]; - LogIncidentDataType(ACCEPTED, *incident); - // Ownership of the incident is passed to the report. - report->mutable_incident()->AddAllocated(incident); + if (incident) { + LogIncidentDataType(ACCEPTED, *incident); + // Ownership of the incident is passed to the report. + report->mutable_incident()->AddAllocated(incident); + } } context->incidents.weak_clear(); - profiles.push_back(scan->first); + std::vector& profile_states = + profiles_to_state[scan->first]; + profile_states.swap(states); } } @@ -573,7 +689,7 @@ void IncidentReportingService::UploadIfCollectionComplete() { return; scoped_ptr context(new UploadContext(report.Pass())); - context->profiles.swap(profiles); + context->profiles_to_state.swap(profiles_to_state); if (!database_manager_) { // No database manager during testing. Take ownership of the context and // continue processing. @@ -628,9 +744,13 @@ void IncidentReportingService::OnKillSwitchResult(UploadContext* context, } void IncidentReportingService::HandleResponse(const UploadContext& context) { - for (size_t i = 0; i < context.profiles.size(); ++i) { - context.profiles[i]->GetPrefs()->SetBoolean( - prefs::kSafeBrowsingIncidentReportSent, true); + for (UploadContext::PersistentIncidentStateCollection::const_iterator scan = + context.profiles_to_state.begin(); + scan != context.profiles_to_state.end(); + ++scan) { + DictionaryPrefUpdate pref_update(scan->first->GetPrefs(), + prefs::kSafeBrowsingIncidentsSent); + MarkIncidentsAsReported(scan->second, pref_update.Get()); } } diff --git a/chrome/browser/safe_browsing/incident_reporting_service_unittest.cc b/chrome/browser/safe_browsing/incident_reporting_service_unittest.cc index a727792c836d4..d3dcc190d1fa7 100644 --- a/chrome/browser/safe_browsing/incident_reporting_service_unittest.cc +++ b/chrome/browser/safe_browsing/incident_reporting_service_unittest.cc @@ -135,6 +135,7 @@ class IncidentReportingServiceTest : public testing::Test { static const int64 kIncidentTimeMsec; static const char kFakeOsName[]; static const char kFakeDownloadToken[]; + static const char kTestTrackedPrefPath[]; IncidentReportingServiceTest() : task_runner_(new base::TestSimpleTaskRunner), @@ -212,7 +213,9 @@ class IncidentReportingServiceTest : public testing::Test { scoped_ptr incident( new safe_browsing::ClientIncidentReport_IncidentData()); incident->set_incident_time_msec(kIncidentTimeMsec); - incident->mutable_tracked_preference(); + safe_browsing::ClientIncidentReport_IncidentData_TrackedPreferenceIncident* + tp_incident = incident->mutable_tracked_preference(); + tp_incident->set_path(kTestTrackedPrefPath); return incident.Pass(); } @@ -230,6 +233,11 @@ class IncidentReportingServiceTest : public testing::Test { ASSERT_TRUE(uploaded_report_->incident(i).has_incident_time_msec()); ASSERT_EQ(kIncidentTimeMsec, uploaded_report_->incident(i).incident_time_msec()); + ASSERT_TRUE(uploaded_report_->incident(i).has_tracked_preference()); + ASSERT_TRUE( + uploaded_report_->incident(i).tracked_preference().has_path()); + ASSERT_EQ(std::string(kTestTrackedPrefPath), + uploaded_report_->incident(i).tracked_preference().path()); } ASSERT_TRUE(uploaded_report_->has_environment()); ASSERT_TRUE(uploaded_report_->environment().has_os()); @@ -440,6 +448,7 @@ base::LazyInstanceRunUntilIdle(); + + // Verify that report upload took place and contained the incident and + // environment data. + ExpectTestIncidentUploaded(1); + + // Add a variation on the incident to the service. + scoped_ptr incident( + MakeTestIncident()); + incident->mutable_tracked_preference()->set_atomic_value("leeches"); + instance_->GetAddIncidentCallback(profile).Run(incident.Pass()); + + // Let all tasks run. + task_runner_->RunUntilIdle(); + + // Verify that an additional report upload took place. + ExpectTestIncidentUploaded(1); +} + // Tests that the same incident added for two different profiles in sequence // results in two uploads. TEST_F(IncidentReportingServiceTest, TwoProfilesTwoUploads) { @@ -613,6 +649,37 @@ TEST_F(IncidentReportingServiceTest, ProfileDestroyedDuringUpload) { // the service while handling the upload response. } +// Tests that no upload takes place if the old pref was present. +TEST_F(IncidentReportingServiceTest, MigrateOldPref) { + Profile* profile = CreateProfile( + "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION); + + // This is a legacy profile. + profile->GetPrefs()->SetBoolean(prefs::kSafeBrowsingIncidentReportSent, true); + + // Add the test incident. + AddTestIncident(profile); + + // Let all tasks run. + task_runner_->RunUntilIdle(); + + // No upload should have taken place. + AssertNoUpload(); + + // The legacy pref should have been cleared. + ASSERT_FALSE( + profile->GetPrefs()->GetBoolean(prefs::kSafeBrowsingIncidentReportSent)); + + // Adding the same incident again should still result in no upload. + AddTestIncident(profile); + + // Let all tasks run. + task_runner_->RunUntilIdle(); + + // No upload should have taken place. + AssertNoUpload(); +} + // Parallel uploads // Shutdown during processing // environment colection taking longer than incident delay timer diff --git a/chrome/browser/safe_browsing/tracked_preference_incident_handlers.cc b/chrome/browser/safe_browsing/tracked_preference_incident_handlers.cc new file mode 100644 index 0000000000000..ed3d2e1fec4af --- /dev/null +++ b/chrome/browser/safe_browsing/tracked_preference_incident_handlers.cc @@ -0,0 +1,29 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/safe_browsing/tracked_preference_incident_handlers.h" + +#include "base/logging.h" +#include "chrome/browser/safe_browsing/incident_handler_util.h" +#include "chrome/common/safe_browsing/csd.pb.h" + +namespace safe_browsing { + +std::string GetTrackedPreferenceIncidentKey( + const ClientIncidentReport_IncidentData& incident_data) { + DCHECK(incident_data.has_tracked_preference()); + DCHECK(incident_data.tracked_preference().has_path()); + return incident_data.tracked_preference().path(); +} + +uint32_t GetTrackedPreferenceIncidentDigest( + const ClientIncidentReport_IncidentData& incident_data) { + DCHECK(incident_data.has_tracked_preference()); + + // Tracked preference incidents are sufficiently canonical (and have no + // default values), so it's safe to serialize the incident as given. + return HashMessage(incident_data.tracked_preference()); +} + +} // namespace safe_browsing diff --git a/chrome/browser/safe_browsing/tracked_preference_incident_handlers.h b/chrome/browser/safe_browsing/tracked_preference_incident_handlers.h new file mode 100644 index 0000000000000..ca2e5f3e49300 --- /dev/null +++ b/chrome/browser/safe_browsing/tracked_preference_incident_handlers.h @@ -0,0 +1,26 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_SAFE_BROWSING_TRACKED_PREFERENCE_INCIDENT_HANDLERS_H_ +#define CHROME_BROWSER_SAFE_BROWSING_TRACKED_PREFERENCE_INCIDENT_HANDLERS_H_ + +#include + +#include + +namespace safe_browsing { + +class ClientIncidentReport_IncidentData; + +// Returns the path of the tracked preference. +std::string GetTrackedPreferenceIncidentKey( + const ClientIncidentReport_IncidentData& incident_data); + +// Returns a digest computed over the tracked preference incident data. +uint32_t GetTrackedPreferenceIncidentDigest( + const ClientIncidentReport_IncidentData& incident_data); + +} // namespace safe_browsing + +#endif // CHROME_BROWSER_SAFE_BROWSING_TRACKED_PREFERENCE_INCIDENT_HANDLERS_H_ diff --git a/chrome/browser/safe_browsing/tracked_preference_incident_handlers_unittest.cc b/chrome/browser/safe_browsing/tracked_preference_incident_handlers_unittest.cc new file mode 100644 index 0000000000000..189ac6d1dd28d --- /dev/null +++ b/chrome/browser/safe_browsing/tracked_preference_incident_handlers_unittest.cc @@ -0,0 +1,63 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/safe_browsing/tracked_preference_incident_handlers.h" + +#include "base/memory/scoped_ptr.h" +#include "chrome/common/safe_browsing/csd.pb.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +scoped_ptr MakeIncident() { + scoped_ptr incident( + new safe_browsing::ClientIncidentReport_IncidentData); + + incident->mutable_tracked_preference()->set_path("foo"); + incident->mutable_tracked_preference()->set_atomic_value("bar"); + incident->mutable_tracked_preference()->set_value_state( + safe_browsing:: + ClientIncidentReport_IncidentData_TrackedPreferenceIncident_ValueState_CLEARED); + return incident.Pass(); +} + +} // namespace + +// Tests that GetKey returns the preference path. +TEST(GetTrackedPreferenceIncidentKey, KeyIsPath) { + safe_browsing::ClientIncidentReport_IncidentData incident; + + incident.mutable_tracked_preference()->set_path("foo"); + ASSERT_EQ(std::string("foo"), + safe_browsing::GetTrackedPreferenceIncidentKey(incident)); +} + +// Tests that GetDigest returns the same value for the same incident. +TEST(GetTrackedPreferenceIncidentDigest, SameIncidentSameDigest) { + scoped_ptr incident( + MakeIncident()); + + uint32_t digest = + safe_browsing::GetTrackedPreferenceIncidentDigest(*incident); + ASSERT_EQ(digest, + safe_browsing::GetTrackedPreferenceIncidentDigest(*MakeIncident())); +} + +// Tests that GetDigest returns the same value for the same incident. +TEST(GetTrackedPreferenceIncidentDigest, DifferentIncidentDifferentDigest) { + scoped_ptr incident( + MakeIncident()); + + uint32_t digest = + safe_browsing::GetTrackedPreferenceIncidentDigest(*incident); + + scoped_ptr incident2( + MakeIncident()); + incident2->mutable_tracked_preference()->set_value_state( + safe_browsing:: + ClientIncidentReport_IncidentData_TrackedPreferenceIncident_ValueState_WEAK_LEGACY); + + ASSERT_NE(digest, + safe_browsing::GetTrackedPreferenceIncidentDigest(*incident2)); +} diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 0390827605ddb..1c422e1681564 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -2672,6 +2672,8 @@ 'browser/safe_browsing/environment_data_collection.h', 'browser/safe_browsing/environment_data_collection_win.cc', 'browser/safe_browsing/environment_data_collection_win.h', + 'browser/safe_browsing/incident_handler_util.cc', + 'browser/safe_browsing/incident_handler_util.h', 'browser/safe_browsing/incident_reporting_service.cc', 'browser/safe_browsing/incident_reporting_service.h', 'browser/safe_browsing/incident_report_uploader.cc', @@ -2700,6 +2702,8 @@ 'browser/safe_browsing/safe_browsing_store.h', 'browser/safe_browsing/sandboxed_zip_analyzer.cc', 'browser/safe_browsing/sandboxed_zip_analyzer.h', + 'browser/safe_browsing/tracked_preference_incident_handlers.cc', + 'browser/safe_browsing/tracked_preference_incident_handlers.h', 'browser/safe_browsing/two_phase_uploader.cc', 'browser/safe_browsing/two_phase_uploader.h', ], diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 818831f21f3f6..1e55a7517be05 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1266,6 +1266,7 @@ 'browser/safe_browsing/safe_browsing_store_file_unittest.cc', 'browser/safe_browsing/safe_browsing_store_unittest.cc', 'browser/safe_browsing/safe_browsing_util_unittest.cc', + 'browser/safe_browsing/tracked_preference_incident_handlers_unittest.cc', 'browser/safe_browsing/two_phase_uploader_unittest.cc', 'browser/search/hotword_service_unittest.cc', 'browser/search/iframe_source_unittest.cc', diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 58975cbd3c1bd..f5137e8707480 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -315,6 +315,9 @@ const char kSafeBrowsingProceedAnywayDisabled[] = const char kSafeBrowsingIncidentReportSent[] = "safebrowsing.incident_report_sent"; +// A dictionary mapping incident types to a dict of incident key:digest pairs. +const char kSafeBrowsingIncidentsSent[] = "safebrowsing.incidents_sent"; + // Enum that specifies whether Incognito mode is: // 0 - Enabled. Default behaviour. Default mode is available on demand. // 1 - Disabled. Used cannot browse pages in Incognito mode. diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 6198ba02ff6b4..4d6b24c355c32 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -138,6 +138,7 @@ extern const char kSafeBrowsingDownloadFeedbackEnabled[]; extern const char kSafeBrowsingReportingEnabled[]; extern const char kSafeBrowsingProceedAnywayDisabled[]; extern const char kSafeBrowsingIncidentReportSent[]; +extern const char kSafeBrowsingIncidentsSent[]; extern const char kIncognitoModeAvailability[]; extern const char kSearchSuggestEnabled[]; #if defined(OS_ANDROID)