Ensure content settings aren't persisted in the guest profile

components/content_settings/core/browser/host_content_settings_map.cc

Index: components/content_settings/core/browser/host_content_settings_map.cc
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 35100454fee7ba4ca79122721f164677a47a97a2..9c050f715b5f47bbce53b42785bafb2d60842519 100644
--- a/components/content_settings/core/browser/host_content_settings_map.cc
+++ b/components/content_settings/core/browser/host_content_settings_map.cc
@@ -94,22 +94,29 @@ scoped_ptr<base::Value> CoerceSettingInheritedToIncognito(
 }  // namespace
 
 HostContentSettingsMap::HostContentSettingsMap(PrefService* prefs,
-                                               bool incognito)
+                                               bool is_incognito_profile,
+                                               bool is_guest_profile)
     :
 #ifndef NDEBUG
       used_from_thread_id_(base::PlatformThread::CurrentId()),
 #endif
       prefs_(prefs),
-      is_off_the_record_(incognito) {
+      is_off_the_record_(is_incognito_profile || is_guest_profile) {
+  DCHECK(!(is_incognito_profile && is_guest_profile));
   content_settings::ObservableProvider* policy_provider =
       new content_settings::PolicyProvider(prefs_);
   policy_provider->AddObserver(this);
   content_settings_providers_[POLICY_PROVIDER] = policy_provider;
 
-  content_settings::ObservableProvider* pref_provider =
+  content_settings::PrefProvider* pref_provider =
       new content_settings::PrefProvider(prefs_, is_off_the_record_);
   pref_provider->AddObserver(this);
   content_settings_providers_[PREF_PROVIDER] = pref_provider;
+  // This ensures that content settings are cleared for the guest profile. This
+  // wouldn't be needed except that we used to allow settings to be stored for
+  // the guest profile and so we need to ensure those get cleared.
+  if (is_guest_profile)
+    pref_provider->ClearPrefs();
 
   content_settings::ObservableProvider* default_provider =
       new content_settings::DefaultProvider(prefs_, is_off_the_record_);
@@ -307,15 +314,15 @@ void HostContentSettingsMap::SetWebsiteSettingCustomScope(
          resource_identifier.empty());
   UsedContentSettingsProviders();
 
-  base::Value* val = value.release();
   for (auto& provider_pair : content_settings_providers_) {
-    if (provider_pair.second->SetWebsiteSetting(primary_pattern,
-                                                secondary_pattern, content_type,
-                                                resource_identifier, val)) {
+    if (provider_pair.second->SetWebsiteSetting(
+            primary_pattern, secondary_pattern, content_type,
+            resource_identifier, value.get())) {
+      // If succesful then ownership is passed to the provider.
+      ignore_result(value.release());

[msramek, 2016/01/20 11:01:46]

I think that the NOTREACHED() was not just supposed to check a leaked pointer,
but also to guarantee that at least one provider will accept the setting. I'd
prefer to keep it.

[raymes, 2016/01/20 22:55:42]

I had to remove it though because in the case of the default setting for the
guest case, none of the providers handle it.

Perhaps we should return true in DefaultProvider::SetWebsiteSetting? I've
changed the code to do that instead and keep the NOTREACHED. 

[msramek, 2016/01/21 10:37:30]

Interesting. So HostContentSettingsMap::SetWebsiteSetting(wildcard, wildcard)
would also crash if called in incognito. Which, of course, wasn't a problem,
since it wasn't possible to set default settings in the UI in incognito.

I agree with this solution. DefaultProvider returns true to acknowledge that it
knows how to handle the value (even if doesn't actually do anything).

In fact, I would say that this logic isn't really needed anymore. All providers
except Pref and Default always return false (since they read their settings from
elsewhere). And for these two, we know that Pref will always accept non-default
settings and Default will always accept default settings. (Just an observation,
I'm not asking you to fix this :) )

       return;
     }
   }
-  NOTREACHED();
 }
 
 void HostContentSettingsMap::SetNarrowestContentSetting(