Make the app banner content settings pref be lossy.

BUG=491031

Review URL: https://codereview.chromium.org/1154703004

Cr-Commit-Position: refs/heads/master@{#332152}

9 files changed, 47 insertions, 12 deletions

diff --git a/chrome/browser/banners/app_banner_settings_helper.cc b/chrome/browser/banners/app_banner_settings_helper.cc
index 02d442d..19e7110 100644
--- a/chrome/browser/banners/app_banner_settings_helper.cc
+++ b/chrome/browser/banners/app_banner_settings_helper.cc
@@ -91,6 +91,7 @@ void AppBannerSettingsHelper::ClearHistoryForURLs(
     settings->SetWebsiteSetting(pattern, ContentSettingsPattern::Wildcard(),
                                 CONTENT_SETTINGS_TYPE_APP_BANNER, std::string(),
                                 nullptr);
+    settings->FlushLossyWebsiteSettings();
   }
 }
 
@@ -168,6 +169,14 @@ void AppBannerSettingsHelper::RecordBannerEvent(
   settings->SetWebsiteSetting(pattern, ContentSettingsPattern::Wildcard(),
                               CONTENT_SETTINGS_TYPE_APP_BANNER, std::string(),
                               origin_dict.release());
+
+  // App banner content settings are lossy, meaning they will not cause the
+  // prefs to become dirty. This is fine for most events, as if they are lost it
+  // just means the user will have to engage a little bit more. However the
+  // DID_ADD_TO_HOMESCREEN event should always be recorded to prevent
+  // spamminess.
+  if (event == APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN)
+    settings->FlushLossyWebsiteSettings();
 }
 
 bool AppBannerSettingsHelper::ShouldShowBanner(
diff --git a/components/content_settings/core/browser/content_settings_default_provider.cc b/components/content_settings/core/browser/content_settings_default_provider.cc
index d85a28e..d6012f3 100644
--- a/components/content_settings/core/browser/content_settings_default_provider.cc
+++ b/components/content_settings/core/browser/content_settings_default_provider.cc
@@ -123,12 +123,8 @@ void DefaultProvider::RegisterProfilePrefs(
   // after two stable releases.
   for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
     ContentSettingsType type = static_cast<ContentSettingsType>(i);
-    if (IsContentSettingsTypeSyncable(type)) {
-      registry->RegisterIntegerPref(GetPrefName(type), GetDefaultValue(type),
-          user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
-    } else {
-      registry->RegisterIntegerPref(GetPrefName(type), GetDefaultValue(type));
-    }
+    registry->RegisterIntegerPref(GetPrefName(type), GetDefaultValue(type),
+                                  PrefRegistrationFlagsForType(type));
   }
 
   // Whether the deprecated dictionary preference has already been migrated
diff --git a/components/content_settings/core/browser/content_settings_pref_provider.cc b/components/content_settings/core/browser/content_settings_pref_provider.cc
index 938eaa1..cd83900 100644
--- a/components/content_settings/core/browser/content_settings_pref_provider.cc
+++ b/components/content_settings/core/browser/content_settings_pref_provider.cc
@@ -121,12 +121,9 @@ void PrefProvider::RegisterProfilePrefs(
                                 false);
 
   for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
-    if (IsContentSettingsTypeSyncable(ContentSettingsType(i))) {
-      registry->RegisterDictionaryPref(kContentSettingsExceptionsPrefs[i],
-          user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
-    } else {
-      registry->RegisterDictionaryPref(kContentSettingsExceptionsPrefs[i]);
-    }
+    registry->RegisterDictionaryPref(
+        kContentSettingsExceptionsPrefs[i],
+        PrefRegistrationFlagsForType(ContentSettingsType(i)));
   }
 }
 
diff --git a/components/content_settings/core/browser/content_settings_utils.cc b/components/content_settings/core/browser/content_settings_utils.cc
index a51f917..eed02d9 100644
--- a/components/content_settings/core/browser/content_settings_utils.cc
+++ b/components/content_settings/core/browser/content_settings_utils.cc
@@ -10,12 +10,14 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
+#include "base/prefs/pref_registry.h"
 #include "base/strings/string_split.h"
 #include "base/values.h"
 #include "components/content_settings/core/browser/content_settings_provider.h"
 #include "components/content_settings/core/browser/content_settings_rule.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
+#include "components/pref_registry/pref_registry_syncable.h"
 #include "url/gurl.h"
 
 namespace {
@@ -227,4 +229,16 @@ void GetRendererContentSettingRules(const HostContentSettingsMap* map,
       &(rules->script_rules));
 }
 
+uint32 PrefRegistrationFlagsForType(ContentSettingsType content_type) {
+  uint32 flags = PrefRegistry::NO_REGISTRATION_FLAGS;
+
+  if (IsContentSettingsTypeSyncable(content_type))
+    flags |= user_prefs::PrefRegistrySyncable::SYNCABLE_PREF;
+
+  if (IsContentSettingsTypeLossy(content_type))
+    flags |= PrefRegistry::LOSSY_PREF;
+
+  return flags;
+}
+
 }  // namespace content_settings
diff --git a/components/content_settings/core/browser/content_settings_utils.h b/components/content_settings/core/browser/content_settings_utils.h
index bba1de5..8db0cbd 100644
--- a/components/content_settings/core/browser/content_settings_utils.h
+++ b/components/content_settings/core/browser/content_settings_utils.h
@@ -76,6 +76,10 @@ base::Value* GetContentSettingValueAndPatterns(
 void GetRendererContentSettingRules(const HostContentSettingsMap* map,
                                     RendererContentSettingRules* rules);
 
+// Get the flags to use when registering the preference to store |content_type|
+// settings.
+uint32 PrefRegistrationFlagsForType(ContentSettingsType content_type);
+
 }  // namespace content_settings
 
 #endif  // COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_CONTENT_SETTINGS_UTILS_H_
diff --git a/components/content_settings/core/browser/host_content_settings_map.cc b/components/content_settings/core/browser/host_content_settings_map.cc
index 0294e3a..9b906cf 100644
--- a/components/content_settings/core/browser/host_content_settings_map.cc
+++ b/components/content_settings/core/browser/host_content_settings_map.cc
@@ -430,6 +430,10 @@ void HostContentSettingsMap::RemoveObserver(
   observers_.RemoveObserver(observer);
 }
 
+void HostContentSettingsMap::FlushLossyWebsiteSettings() {
+  prefs_->SchedulePendingLossyWrites();
+}
+
 void HostContentSettingsMap::SetPrefClockForTesting(
     scoped_ptr<base::Clock> clock) {
   UsedContentSettingsProviders();
@@ -470,6 +474,7 @@ void HostContentSettingsMap::ClearSettingsForOneType(
        ++provider) {
     provider->second->ClearAllContentSettingsRules(content_type);
   }
+  FlushLossyWebsiteSettings();
 }
 
 bool HostContentSettingsMap::IsValueAllowedForType(
diff --git a/components/content_settings/core/browser/host_content_settings_map.h b/components/content_settings/core/browser/host_content_settings_map.h
index 61e7305..9566883 100644
--- a/components/content_settings/core/browser/host_content_settings_map.h
+++ b/components/content_settings/core/browser/host_content_settings_map.h
@@ -286,6 +286,9 @@ class HostContentSettingsMap
   void AddObserver(content_settings::Observer* observer);
   void RemoveObserver(content_settings::Observer* observer);
 
+  // Schedules any pending lossy website settings to be written to disk.
+  void FlushLossyWebsiteSettings();
+
   // Passes ownership of |clock|.
   void SetPrefClockForTesting(scoped_ptr<base::Clock> clock);
 
diff --git a/components/content_settings/core/common/content_settings.cc b/components/content_settings/core/common/content_settings.cc
index 491f973..91dd091 100644
--- a/components/content_settings/core/common/content_settings.cc
+++ b/components/content_settings/core/common/content_settings.cc
@@ -115,6 +115,10 @@ bool IsContentSettingsTypeSyncable(ContentSettingsType content_setting) {
   return false;
 }
 
+bool IsContentSettingsTypeLossy(ContentSettingsType content_setting) {
+  return content_setting == CONTENT_SETTINGS_TYPE_APP_BANNER;
+}
+
 ContentSettingPatternSource::ContentSettingPatternSource(
     const ContentSettingsPattern& primary_pattern,
     const ContentSettingsPattern& secondary_pattern,
diff --git a/components/content_settings/core/common/content_settings.h b/components/content_settings/core/common/content_settings.h
index c7264476..48f16e5 100644
--- a/components/content_settings/core/common/content_settings.h
+++ b/components/content_settings/core/common/content_settings.h
@@ -37,6 +37,9 @@ ContentSettingsTypeHistogram ContentSettingTypeToHistogramValue(
 // Whether this content setting should be synced.
 bool IsContentSettingsTypeSyncable(ContentSettingsType content_setting);
 
+// Whether this content setting can tolerate data being lost.
+bool IsContentSettingsTypeLossy(ContentSettingsType content_setting);
+
 struct ContentSettingPatternSource {
   ContentSettingPatternSource(const ContentSettingsPattern& primary_pattern,
                               const ContentSettingsPattern& secondary_patttern,