Site Settings Desktop: Support adding exceptions for incognito mode.

chrome/browser/ui/webui/site_settings_helper.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/site_settings_helper.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/values.h"
 #include "chrome/browser/permissions/chooser_context_base.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/usb/usb_chooser_context_factory.h"
 #include "chrome/common/pref_names.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/prefs/pref_service.h"
 
 namespace site_settings {
 
 const char kSetting[] = "setting";
 const char kOrigin[] = "origin";
 const char kPolicyProviderId[] = "policy";
 const char kSource[] = "source";
+const char kIncognito[] = "incognito";
 const char kEmbeddingOrigin[] = "embeddingOrigin";
 const char kPreferencesSource[] = "preference";
 const char kObject[] = "object";
 const char kObjectName[] = "objectName";
 
 const char kGroupTypeUsb[] = "usb-devices";
 
 ChooserContextBase* GetUsbChooserContext(Profile* profile) {
   return reinterpret_cast<ChooserContextBase*>(
       UsbChooserContextFactory::GetForProfile(profile));
@@ skipping 41 matching lines @@
   NOTREACHED() << type << " is not a recognized content settings type.";
   return std::string();
 }
 
 // Create a DictionaryValue* that will act as a data source for a single row
 // in a HostContentSettingsMap-controlled exceptions table (e.g., cookies).
 std::unique_ptr<base::DictionaryValue> GetExceptionForPage(
     const ContentSettingsPattern& pattern,
     const ContentSettingsPattern& secondary_pattern,
     const ContentSetting& setting,
-    const std::string& provider_name) {
+    const std::string& provider_name,
+    bool incognito) {
   base::DictionaryValue* exception = new base::DictionaryValue();
   exception->SetString(kOrigin, pattern.ToString());
   exception->SetString(kEmbeddingOrigin,
                        secondary_pattern == ContentSettingsPattern::Wildcard() ?
                            std::string() :
                            secondary_pattern.ToString());
 
   std::string setting_string =
       content_settings::ContentSettingToString(setting);
   DCHECK(!setting_string.empty());
 
   exception->SetString(kSetting, setting_string);
   exception->SetString(kSource, provider_name);
+  exception->SetBoolean(kIncognito, incognito);
   return base::WrapUnique(exception);
 }
 
 void GetExceptionsFromHostContentSettingsMap(const HostContentSettingsMap* map,
                                              ContentSettingsType type,
                                              content::WebUI* web_ui,
+                                             bool incognito,
                                              base::ListValue* exceptions) {
   ContentSettingsForOneType entries;
   map->GetSettingsForOneType(type, std::string(), &entries);
   // Group settings by primary_pattern.
   AllPatternsSettings all_patterns_settings;
   for (ContentSettingsForOneType::iterator i = entries.begin();
        i != entries.end(); ++i) {
     // Don't add default settings.
     if (i->primary_pattern == ContentSettingsPattern::Wildcard() &&
         i->secondary_pattern == ContentSettingsPattern::Wildcard() &&
@@ skipping 37 matching lines @@
     const std::string& source = i->first.second;
     auto& this_provider_exceptions = all_provider_exceptions
         [HostContentSettingsMap::GetProviderTypeFromSource(source)];
 
     // Add the "parent" entry for the non-embedded setting.
     ContentSetting parent_setting =
         parent == one_settings.end() ? CONTENT_SETTING_DEFAULT : parent->second;
     const ContentSettingsPattern& secondary_pattern =
         parent == one_settings.end() ? primary_pattern : parent->first;
     this_provider_exceptions.push_back(GetExceptionForPage(
-        primary_pattern, secondary_pattern, parent_setting, source));
+        primary_pattern, secondary_pattern, parent_setting, source, incognito));
 
     // Add the "children" for any embedded settings.
     for (OnePatternSettings::const_iterator j = one_settings.begin();
          j != one_settings.end(); ++j) {
       // Skip the non-embedded setting which we already added above.
       if (j == parent)
         continue;
 
       ContentSetting content_setting = j->second;
       this_provider_exceptions.push_back(GetExceptionForPage(
-          primary_pattern, j->first, content_setting, source));
+          primary_pattern, j->first, content_setting, source, incognito));
     }
   }
 
   // For camera and microphone, we do not have policy exceptions, but we do have
   // the policy-set allowed URLs, which should be displayed in the same manner.
   if (type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC ||
       type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA) {
     auto& policy_exceptions = all_provider_exceptions
         [HostContentSettingsMap::GetProviderTypeFromSource(kPolicyProviderId)];
     DCHECK(policy_exceptions.empty());
-    GetPolicyAllowedUrls(type, &policy_exceptions, web_ui);
+    GetPolicyAllowedUrls(type, &policy_exceptions, web_ui, incognito);
   }
 
   for (auto& one_provider_exceptions : all_provider_exceptions) {
     for (auto& exception : one_provider_exceptions)
       exceptions->Append(std::move(exception));
   }
 }
 
 void GetPolicyAllowedUrls(
     ContentSettingsType type,
     std::vector<std::unique_ptr<base::DictionaryValue>>* exceptions,
-    content::WebUI* web_ui) {
+    content::WebUI* web_ui,
+    bool incognito) {
   DCHECK(type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC ||
          type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
 
   PrefService* prefs = Profile::FromWebUI(web_ui)->GetPrefs();
   const base::ListValue* policy_urls = prefs->GetList(
       type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC
           ? prefs::kAudioCaptureAllowedUrls
           : prefs::kVideoCaptureAllowedUrls);
 
   // Convert the URLs to |ContentSettingsPattern|s. Ignore any invalid ones.
@@ skipping 12 matching lines @@
   }
 
   // The patterns are shown in the UI in a reverse order defined by
   // |ContentSettingsPattern::operator<|.
   std::sort(
       patterns.begin(), patterns.end(), std::greater<ContentSettingsPattern>());
 
   for (const ContentSettingsPattern& pattern : patterns) {
     exceptions->push_back(GetExceptionForPage(pattern, ContentSettingsPattern(),
                                               CONTENT_SETTING_ALLOW,
-                                              kPolicyProviderId));
+                                              kPolicyProviderId, incognito));
   }
 }
 
 const ChooserTypeNameEntry* ChooserTypeFromGroupName(const std::string& name) {
   for (const auto& chooser_type : kChooserTypeGroupNames) {
     if (chooser_type.name == name)
       return &chooser_type;
   }
   return nullptr;
 }
 
 // Create a DictionaryValue* that will act as a data source for a single row
 // in a chooser permission exceptions table.
 std::unique_ptr<base::DictionaryValue> GetChooserExceptionForPage(
     const GURL& requesting_origin,
     const GURL& embedding_origin,
     const std::string& provider_name,
+    bool incognito,
     const std::string& name,
     const base::DictionaryValue* object) {
   std::unique_ptr<base::DictionaryValue> exception(new base::DictionaryValue());
 
   std::string setting_string =
       content_settings::ContentSettingToString(CONTENT_SETTING_DEFAULT);
   DCHECK(!setting_string.empty());
 
   exception->SetString(site_settings::kSetting, setting_string);
   exception->SetString(site_settings::kOrigin, requesting_origin.spec());
   exception->SetString(
       site_settings::kEmbeddingOrigin, embedding_origin.spec());
   exception->SetString(site_settings::kSource, provider_name);
+  exception->SetBoolean(kIncognito, incognito);

[stevenjb, 2016/09/01 15:47:55]

It *looks* like the only thing we use |incognito| for is to set
exception[kIncognito] = true.

If that is the case, I think this code would be more clear with a separate call:
site_settings::SetIncognito(exceptions).

Otherwise, anywhere we call these functions we need to document the boolean,
e.g. true /* incognito */ or false /* not incognito */. (Or use a named
variable).

[Finnur, 2016/09/01 16:48:17]

Correct. 

We send down (to .js) a single Array<SiteException> where some of the
SiteExceptions have the |incognito| bit set while others don't.

I think your comment can be read in two ways. Either you are saying:

1) Once we've gathered the exceptions for incognito, we should (before gathering
the other exceptions) iterate through the incognito exceptions and add the
incognito flag to each one. 

... OR ...

2) We should change what we send down to .js, from:
  Array<SiteException>
... to something like ...
[ {incognito : boolean, exceptions: Array<SiteException>}, ... ]


My concern about 1 is that it is less efficient than the current solution and my
concern about 2 is that changing the format we send down is a breaking change
for the old settings code, which I was hoping to avoid (with a non-breaking
change like adding an extra field)...

[stevenjb, 2016/09/01 17:07:54]

I was suggesting (1). I don't think the inefficiency of an extra call to
SetBoolean is a concern, but since you commented all of the calls to this,
that's fine.

   if (object) {
     exception->SetString(kObjectName, name);
     exception->Set(kObject, object->CreateDeepCopy());
   }
   return exception;
 }
 
 void GetChooserExceptionsFromProfile(
     Profile* profile,
     bool incognito,
@@ skipping 41 matching lines @@
       const SortedObjects& sorted_objects = one_origin_objects_entry.second;
 
       // Skip the embedded settings which will be added below.
       if (requesting_origin != embedding_origin) {
         has_embedded_entries = true;
         continue;
       }
 
       for (const auto& sorted_objects_entry : sorted_objects) {
         this_provider_exceptions.push_back(GetChooserExceptionForPage(
-            requesting_origin, embedding_origin, source,
-            sorted_objects_entry.first, sorted_objects_entry.second));
+            requesting_origin, embedding_origin, source, incognito,
+            sorted_objects_entry.first,
+            sorted_objects_entry.second));
       }
     }
 
     if (has_embedded_entries) {
       // Add a "parent" entry that simply acts as a heading for all entries
       // where |requesting_origin| has been embedded.
       this_provider_exceptions.push_back(
           GetChooserExceptionForPage(requesting_origin, requesting_origin,
-                                     source, std::string(), nullptr));
+                                     source, incognito, std::string(),
+                                     nullptr));
 
       // Add the "children" for any embedded settings.
       for (const auto& one_origin_objects_entry : one_origin_objects) {
         const GURL& embedding_origin = one_origin_objects_entry.first;
         const SortedObjects& sorted_objects = one_origin_objects_entry.second;
 
         // Skip the non-embedded setting which we already added above.
         if (requesting_origin == embedding_origin)
           continue;
 
         for (const auto& sorted_objects_entry : sorted_objects) {
           this_provider_exceptions.push_back(GetChooserExceptionForPage(
-              requesting_origin, embedding_origin, source,
+              requesting_origin, embedding_origin, source, incognito,
               sorted_objects_entry.first, sorted_objects_entry.second));
         }
       }
     }
   }
 
   for (auto& one_provider_exceptions : all_provider_exceptions) {
     for (auto& exception : one_provider_exceptions)
       exceptions->Append(std::move(exception));
   }
 }
 
 }  // namespace site_settings