Clean up pref state in the wake of removing the omnibox watcher.

chrome/browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc

Index: chrome/browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc
diff --git a/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc b/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc
index 092a8f73271396cd65435fca7718c2578ae807d9..f224a32b1b15a8e134d76c9f69ece735b71735f5 100644
--- a/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc
+++ b/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc
@@ -188,16 +188,20 @@ class IncidentReportingServiceTest : public testing::Test {
 
   // Creates and returns a profile (owned by the profile manager) with or
   // without safe browsing enabled. An incident will be created within
-  // PreProfileAdd if requested.
+  // PreProfileAdd if requested. |incidents_sent|, if provided, will be set
+  // in the profile's preference.
   TestingProfile* CreateProfile(const std::string& profile_name,
                                 SafeBrowsingDisposition safe_browsing_opt_in,
-                                OnProfileAdditionAction on_addition_action) {
+                                OnProfileAdditionAction on_addition_action,
+                                scoped_ptr<base::Value> incidents_sent) {
     // Create prefs for the profile with safe browsing enabled or not.
     scoped_ptr<TestingPrefServiceSyncable> prefs(
         new TestingPrefServiceSyncable);
     chrome::RegisterUserProfilePrefs(prefs->registry());
     prefs->SetBoolean(prefs::kSafeBrowsingEnabled,
                       safe_browsing_opt_in == SAFE_BROWSING_OPT_IN);
+    if (incidents_sent)
+      prefs->Set(prefs::kSafeBrowsingIncidentsSent, *incidents_sent);
 
     // Remember whether or not to create an incident.
     profile_properties_[profile_name].on_addition_action = on_addition_action;
@@ -477,7 +481,8 @@ const char IncidentReportingServiceTest::kTestTrackedPrefPath[] = "some_pref";
 // is on is uploaded.
 TEST_F(IncidentReportingServiceTest, AddIncident) {
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -503,7 +508,8 @@ TEST_F(IncidentReportingServiceTest, AddIncident) {
 // Tests that multiple incidents are coalesced into the same report.
 TEST_F(IncidentReportingServiceTest, CoalesceIncidents) {
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_TWO_INCIDENTS);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_TWO_INCIDENTS,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -531,7 +537,8 @@ TEST_F(IncidentReportingServiceTest, CoalesceIncidents) {
 TEST_F(IncidentReportingServiceTest, NoSafeBrowsing) {
   // Create the profile, thereby causing the test to begin.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_OUT, ON_PROFILE_ADDITION_ADD_INCIDENT);
+      "profile1", SAFE_BROWSING_OPT_OUT, ON_PROFILE_ADDITION_ADD_INCIDENT,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -550,7 +557,8 @@ TEST_F(IncidentReportingServiceTest, NoDownloadNoUpload) {
 
   // Create the profile, thereby causing the test to begin.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -573,7 +581,8 @@ TEST_F(IncidentReportingServiceTest, NoProfilesNoUpload) {
 
   // Create the profile, thereby causing the test to begin.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -594,7 +603,8 @@ TEST_F(IncidentReportingServiceTest, NoProfilesNoUpload) {
 TEST_F(IncidentReportingServiceTest, OneIncidentOneUpload) {
   // Create the profile, thereby causing the test to begin.
   Profile* profile = CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -621,7 +631,8 @@ TEST_F(IncidentReportingServiceTest, OneIncidentOneUpload) {
 TEST_F(IncidentReportingServiceTest, TwoIncidentsTwoUploads) {
   // Create the profile, thereby causing the test to begin.
   Profile* profile = CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -648,7 +659,8 @@ TEST_F(IncidentReportingServiceTest, TwoIncidentsTwoUploads) {
 TEST_F(IncidentReportingServiceTest, TwoProfilesTwoUploads) {
   // Create the profile, thereby causing the test to begin.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -659,7 +671,8 @@ TEST_F(IncidentReportingServiceTest, TwoProfilesTwoUploads) {
 
   // Create a second profile with its own incident on addition.
   CreateProfile(
-      "profile2", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT);
+      "profile2", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -676,7 +689,8 @@ TEST_F(IncidentReportingServiceTest, TwoProfilesTwoUploads) {
 TEST_F(IncidentReportingServiceTest, ProfileDestroyedDuringUpload) {
   // Create a profile for which an incident will be added.
   Profile* profile = CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_ADD_INCIDENT,
+      nullptr);
 
   // Hook up a callback to run when the upload is started that will post a task
   // to delete the profile. This task will run before the upload finishes.
@@ -717,7 +731,7 @@ TEST_F(IncidentReportingServiceTest, ProcessWideNoProfileNoUpload) {
 TEST_F(IncidentReportingServiceTest, ProcessWideOneUpload) {
   // Add a profile that participates in safe browsing.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION, nullptr);
 
   // Add the test incident.
   AddTestIncident(NULL);
@@ -746,7 +760,7 @@ TEST_F(IncidentReportingServiceTest, ProcessWideOneUpload) {
 TEST_F(IncidentReportingServiceTest, ProcessWideTwoUploads) {
   // Add a profile that participates in safe browsing.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION, nullptr);
 
   // Add the test incident.
   safe_browsing::AddIncidentCallback add_incident(
@@ -786,7 +800,7 @@ TEST_F(IncidentReportingServiceTest, ProcessWideOneUploadAfterProfile) {
 
   // Add a profile that participates in safe browsing.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION, nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -813,7 +827,7 @@ TEST_F(IncidentReportingServiceTest, NoCollectionWithoutIncident) {
 
   // Add a profile that participates in safe browsing.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION, nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -842,7 +856,7 @@ TEST_F(IncidentReportingServiceTest, AnalysisAfterProfile) {
 
   // Add a profile that participates in safe browsing.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION, nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -859,7 +873,8 @@ TEST_F(IncidentReportingServiceTest, AnalysisAfterProfile) {
 TEST_F(IncidentReportingServiceTest, AnalysisWhenRegisteredWithProfile) {
   // Add a profile that participates in safe browsing.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION,
+      nullptr);
 
   // Register a callback.
   RegisterAnalysis(ON_DELAYED_ANALYSIS_NO_ACTION);
@@ -882,7 +897,8 @@ TEST_F(IncidentReportingServiceTest, DelayedAnalysisNoProfileNoUpload) {
 
   // Add a profile that does not participate in safe browsing.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_OUT, ON_PROFILE_ADDITION_NO_ACTION);
+      "profile1", SAFE_BROWSING_OPT_OUT, ON_PROFILE_ADDITION_NO_ACTION,
+      nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -905,7 +921,7 @@ TEST_F(IncidentReportingServiceTest, DelayedAnalysisOneUpload) {
 
   // Add a profile that participates in safe browsing.
   CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION, nullptr);
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -939,7 +955,7 @@ TEST_F(IncidentReportingServiceTest, NoDownloadNoWaiting) {
 
   // Add a profile that participates in safe browsing.
   Profile* profile = CreateProfile(
-      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION);
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION, nullptr);
 
   // Add an incident.
   AddTestIncident(profile);
@@ -957,6 +973,33 @@ TEST_F(IncidentReportingServiceTest, NoDownloadNoWaiting) {
   ASSERT_FALSE(instance_->IsProcessingReport());
 }
 
+// Test that a profile's prune state is properly cleaned upon load.
+TEST_F(IncidentReportingServiceTest, CleanLegacyPruneState) {
+  // Set up a prune state dict with data to be cleared (and not).
+  scoped_ptr<base::DictionaryValue> incidents_sent(new base::DictionaryValue());
+  base::DictionaryValue* type_dict = new base::DictionaryValue();
+  type_dict->SetStringWithoutPathExpansion("foo", "47");
+  incidents_sent->SetWithoutPathExpansion("4", type_dict);  // omnibox

[robertshield, 2015/01/27 14:12:02]

s/"4"/IntToString(OMNIBOX_INTERACTION)/ here and below?

[grt (UTC plus 2), 2015/01/27 18:33:06]

Done.

+  type_dict = new base::DictionaryValue();
+  type_dict->SetStringWithoutPathExpansion("bar", "43");
+  incidents_sent->SetWithoutPathExpansion("1", type_dict);  // preference
+
+  // Add a profile.
+  Profile* profile = CreateProfile(
+      "profile1", SAFE_BROWSING_OPT_IN, ON_PROFILE_ADDITION_NO_ACTION,
+      incidents_sent.Pass());
+
+  // Let all tasks run.
+  task_runner_->RunUntilIdle();
+
+  const base::DictionaryValue* new_state =
+      profile->GetPrefs()->GetDictionary(prefs::kSafeBrowsingIncidentsSent);
+  // The legacy value must be gone.
+  ASSERT_FALSE(new_state->HasKey("4"));
+  // But other data must be untouched.
+  ASSERT_TRUE(new_state->HasKey("1"));
+}
+
 // Parallel uploads
 // Shutdown during processing
 // environment colection taking longer than incident delay timer