Limit scope of settings API configuration and proxy permission

extensions/browser/extension_pref_value_map.cc

Index: extensions/browser/extension_pref_value_map.cc
diff --git a/extensions/browser/extension_pref_value_map.cc b/extensions/browser/extension_pref_value_map.cc
index 5cba7e510a2afac21c3827a9ae09a97369ae5457..0797b2416e3c03e7eaf55985e88a0fae77f2bbc3 100644
--- a/extensions/browser/extension_pref_value_map.cc
+++ b/extensions/browser/extension_pref_value_map.cc
@@ -15,6 +15,8 @@ struct ExtensionPrefValueMap::ExtensionEntry {
   base::Time install_time;
   // Whether extension is enabled in the profile.
   bool enabled;
+  // Whether the extension has access to the incognito profile.
+  bool incognito_enabled;
   // Extension controlled preferences for the regular profile.
   PrefValueMap regular_profile_preferences;
   // Extension controlled preferences that should *only* apply to the regular
@@ -74,6 +76,9 @@ bool ExtensionPrefValueMap::CanExtensionControlPref(
     return false;
   }
 
+  if (incognito && !ext->second->incognito_enabled)
+    return false;
+
   ExtensionEntryMap::const_iterator winner =
       GetEffectivePrefValueController(pref_key, incognito, NULL);
   if (winner == entries_.end())
@@ -115,7 +120,8 @@ bool ExtensionPrefValueMap::DoesExtensionControlPref(
 
 void ExtensionPrefValueMap::RegisterExtension(const std::string& ext_id,
                                               const base::Time& install_time,
-                                              bool is_enabled) {
+                                              bool is_enabled,
+                                              bool is_incognito_enabled) {
   if (entries_.find(ext_id) == entries_.end()) {
     entries_[ext_id] = new ExtensionEntry;
 
@@ -124,6 +130,7 @@ void ExtensionPrefValueMap::RegisterExtension(const std::string& ext_id,
   }
 
   entries_[ext_id]->enabled = is_enabled;
+  entries_[ext_id]->incognito_enabled = is_incognito_enabled;
 }
 
 void ExtensionPrefValueMap::UnregisterExtension(const std::string& ext_id) {
@@ -154,6 +161,22 @@ void ExtensionPrefValueMap::SetExtensionState(const std::string& ext_id,
   NotifyPrefValueChanged(keys);
 }
 
+void ExtensionPrefValueMap::SetExtensionIncognitoState(
+    const std::string& ext_id,
+    bool is_incognito_enabled) {
+  ExtensionEntryMap::const_iterator i = entries_.find(ext_id);
+  // This may happen when sync sets the extension state for an
+  // extension that is not installed.
+  if (i == entries_.end())
+    return;
+  if (i->second->incognito_enabled == is_incognito_enabled)
+    return;
+  std::set<std::string> keys;  // keys set by this extension
+  GetExtensionControlledKeys(*(i->second), &keys);
+  i->second->incognito_enabled = is_incognito_enabled;
+  NotifyPrefValueChanged(keys);

[not at google - send to devlin, 2014/03/05 17:14:06]

it would be kinda nice to factor this common logic sensibly with
SetExtensionState, but that may be more than it's worth

[battre, 2014/03/10 13:50:11]

I see two different ways of achieving this:

1)

void ExtensionPrefValueMap::SetExtensionStateHelper(ExtensionEntry* extension,
                                                    bool* state,
                                                    bool new_state) {
  if (*state == new_state)
    return;
  std::set<std::string> keys;  // keys set by this extension
  GetExtensionControlledKeys(*extension, &keys);
  *state = new_state;
  NotifyPrefValueChanged(keys);
}

void ExtensionPrefValueMap::SetExtensionState(const std::string& ext_id,
                                              bool is_enabled) {
  ExtensionEntryMap::const_iterator i = entries_.find(ext_id);
  // This may happen when sync sets the extension state for an
  // extension that is not installed.
  if (i == entries_.end())
    return;
  SetExtensionStateHelper(i->second, &i->second->enabled, is_enabled);
}

void ExtensionPrefValueMap::SetExtensionIncognitoState(
    const std::string& ext_id,
    bool is_incognito_enabled) {
  ExtensionEntryMap::const_iterator i = entries_.find(ext_id);
  // This may happen when sync sets the extension state for an
  // extension that is not installed.
  if (i == entries_.end())
    return;
  SetExtensionStateHelper(
      i->second, &i->second->incognito_enabled, is_incognito_enabled);
}


2)

void ExtensionPrefValueMap::SetExtensionStateHelper(const std::string& ext_id,
                                                    bool incognito,
                                                    bool new_state) {
  ExtensionEntryMap::const_iterator i = entries_.find(ext_id);
  // This may happen when sync sets the extension state for an
  // extension that is not installed.
  if (i == entries_.end())
    return;
  bool old_state =
      incognito ? i->second->incognito_enabled : i->second->enabled;
  if (old_state == new_state)
    return;
  std::set<std::string> keys;  // keys set by this extension
  GetExtensionControlledKeys(*(i->second), &keys);
  if (incognito)
    i->second->incognito_enabled = new_state;
  else
    i->second->enabled = new_state;
  NotifyPrefValueChanged(keys);
}

void ExtensionPrefValueMap::SetExtensionState(const std::string& ext_id,
                                              bool is_enabled) {
  SetExtensionStateHelper(ext_id, false, is_enabled);
}

void ExtensionPrefValueMap::SetExtensionIncognitoState(
    const std::string& ext_id,
    bool is_incognito_enabled) {
  SetExtensionStateHelper(ext_id, true, is_incognito_enabled);
}

My feeling is that neither is significantly better. WDYT?

[Yoyo Zhou, 2014/03/10 22:33:25]

Yes, I don't see either of these as an improvement.

At core, the logic is
1. find extension
2. apply change and see if it's different, return if not
3. notify changes for keys

2. would be easiest as a lambda, but C++ isn't the right language for it. I
think it's okay as is.

+}
+
 PrefValueMap* ExtensionPrefValueMap::GetExtensionPrefValueMap(
     const std::string& ext_id,
     ExtensionPrefsScope scope) {
@@ -231,6 +254,7 @@ const base::Value* ExtensionPrefValueMap::GetEffectivePrefValue(
 
   // First search for incognito session only preferences.
   if (incognito) {
+    DCHECK(winner->second->incognito_enabled);
     const PrefValueMap* prefs = GetExtensionPrefValueMap(
         ext_id, extensions::kExtensionPrefsScopeIncognitoSessionOnly);
     prefs->GetValue(key, &value);
@@ -274,11 +298,14 @@ ExtensionPrefValueMap::GetEffectivePrefValueController(
     const std::string& ext_id = i->first;
     const base::Time& install_time = i->second->install_time;
     const bool enabled = i->second->enabled;
+    const bool incognito_enabled = i->second->incognito_enabled;
 
     if (!enabled)
       continue;
     if (install_time < winners_install_time)
       continue;
+    if (incognito && !incognito_enabled)
+      continue;
 
     const base::Value* value = NULL;
     const PrefValueMap* prefs = GetExtensionPrefValueMap(