Remove last usage functions from HostContentSettingsMap and clean up prefs

chrome/browser/content_settings/content_settings_pref_provider_unittest.cc

Index: chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
diff --git a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
index 01ed8497530169b3396ae5e69c94bfa51297ad70..80ffc7cf9901e088987678ec9114e80ca22d4728 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
+++ b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
@@ -11,7 +11,6 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
-#include "base/test/simple_test_clock.h"
 #include "base/threading/platform_thread.h"
 #include "base/values.h"
 #include "chrome/browser/content_settings/content_settings_mock_observer.h"
@@ -135,7 +134,7 @@ TEST_F(PrefProviderTest, Observer) {
 }
 
 // Tests that fullscreen and mouselock content settings are cleared.
-TEST_F(PrefProviderTest, DiscardObsoletePreferences) {
+TEST_F(PrefProviderTest, DiscardObsoleteFullscreenAndMouselockPreferences) {
   static const char kFullscreenPrefPath[] =
       "profile.content_settings.exceptions.fullscreen";
 #if !defined(OS_ANDROID)
@@ -179,6 +178,58 @@ TEST_F(PrefProviderTest, DiscardObsoletePreferences) {
                                          std::string(), false));
 }
 
+// Tests that last usage content settings are cleared.
+TEST_F(PrefProviderTest, DiscardObsoleteLastUsagePreferences) {
+  static const char kGeolocationPrefPath[] =
+      "profile.content_settings.exceptions.geolocation";

[raymes, 2017/02/16 11:01:38]

nit: You should be able to get this from
ContentSettingsRegistry::GetInstance()->Get(type)->website_settings_info()->pref_name();

To be honest it's not really simpler, but it saves redefining the constant.

[Timothy Loh, 2017/02/17 06:54:38]

Done.

+  static const char kMicPrefPath[] =
+      "profile.content_settings.exceptions.media_stream_mic";
+  static const char kObsoleteLastUsed[] = "last_used";

[raymes, 2017/02/16 11:01:38]

nit: I think static is unneeded here and below

[Timothy Loh, 2017/02/17 06:54:38]

Done.

+
+  TestingProfile profile;
+  PrefService* prefs = profile.GetPrefs();
+
+  // Content settings prefs are structured as follows:
+  // "media_stream_mic": {
+  //   "https://example.com:443,*": {
+  //     "last_used": 1486968992.758971,
+  //     "setting": 1
+  //   }
+  // }
+  static const char kPattern[] = "https://example.com:443,*";
+
+  auto geolocation_pattern_data = base::MakeUnique<base::DictionaryValue>();
+  geolocation_pattern_data->SetDouble(kObsoleteLastUsed, 1485000000.0);
+  base::DictionaryValue geolocation_pref_data;
+  geolocation_pref_data.SetWithoutPathExpansion(
+      kPattern, std::move(geolocation_pattern_data));
+  prefs->Set(kGeolocationPrefPath, geolocation_pref_data);
+
+  auto mic_pattern_data = base::MakeUnique<base::DictionaryValue>();
+  mic_pattern_data->SetInteger("setting", CONTENT_SETTING_ALLOW);
+  mic_pattern_data->SetDouble(kObsoleteLastUsed, 1480000000.0);
+  base::DictionaryValue mic_pref_data;
+  mic_pref_data.SetWithoutPathExpansion(kPattern, std::move(mic_pattern_data));
+  prefs->Set(kMicPrefPath, mic_pref_data);
+
+  // Instantiate a new PrefProvider here, because we want to test the
+  // constructor's behavior after setting the above.
+  PrefProvider provider(prefs, false);
+  provider.ShutdownOnUIThread();
+
+  // Check that last_used data has been deleted.
+  EXPECT_TRUE(prefs->GetDictionary(kGeolocationPrefPath)->empty());
+  auto mic_prefs = prefs->GetDictionary(kMicPrefPath);
+  const base::DictionaryValue* mic_result_pattern_data;
+  ASSERT_TRUE(mic_prefs->GetDictionaryWithoutPathExpansion(
+      kPattern, &mic_result_pattern_data));
+  EXPECT_EQ(mic_result_pattern_data->size(), static_cast<size_t>(1));
+  int mic_result_setting;
+  EXPECT_TRUE(
+      mic_result_pattern_data->GetInteger("setting", &mic_result_setting));
+  EXPECT_EQ(mic_result_setting, CONTENT_SETTING_ALLOW);

[raymes, 2017/02/16 11:01:38]

In this part, could we also query the content setting through the public API of
PrefProvider? That would help us to be sure that the mutated values get reparsed
correctly.

[Timothy Loh, 2017/02/17 06:54:38]

Done.

+}
+
 // Test for regression in which the PrefProvider modified the user pref store
 // of the OTR unintentionally: http://crbug.com/74466.
 TEST_F(PrefProviderTest, Incognito) {
@@ -399,41 +450,6 @@ TEST_F(PrefProviderTest, Deadlock) {
   provider.ShutdownOnUIThread();
 }
 
-TEST_F(PrefProviderTest, LastUsage) {
-  TestingProfile testing_profile;
-  PrefProvider pref_content_settings_provider(testing_profile.GetPrefs(),
-                                              false);
-  base::SimpleTestClock* test_clock = new base::SimpleTestClock;
-  test_clock->SetNow(base::Time::Now());
-
-  pref_content_settings_provider.SetClockForTesting(
-      std::unique_ptr<base::Clock>(test_clock));
-  GURL host("http://example.com/");
-  ContentSettingsPattern pattern =
-      ContentSettingsPattern::FromString("[*.]example.com");
-
-  base::Time no_usage = pref_content_settings_provider.GetLastUsage(
-      pattern, pattern, CONTENT_SETTINGS_TYPE_GEOLOCATION);
-  EXPECT_EQ(no_usage.ToDoubleT(), 0);
-
-  pref_content_settings_provider.UpdateLastUsage(
-      pattern, pattern, CONTENT_SETTINGS_TYPE_GEOLOCATION);
-  base::Time first = pref_content_settings_provider.GetLastUsage(
-      pattern, pattern, CONTENT_SETTINGS_TYPE_GEOLOCATION);
-
-  test_clock->Advance(base::TimeDelta::FromSeconds(10));
-
-  pref_content_settings_provider.UpdateLastUsage(
-      pattern, pattern, CONTENT_SETTINGS_TYPE_GEOLOCATION);
-  base::Time second = pref_content_settings_provider.GetLastUsage(
-      pattern, pattern, CONTENT_SETTINGS_TYPE_GEOLOCATION);
-
-  base::TimeDelta delta = second - first;
-  EXPECT_EQ(delta.InSeconds(), 10);
-
-  pref_content_settings_provider.ShutdownOnUIThread();
-}
-
 TEST_F(PrefProviderTest, IncognitoInheritsValueMap) {
   sync_preferences::TestingPrefServiceSyncable prefs;
   PrefProvider::RegisterProfilePrefs(prefs.registry());