MD Settings: Set all content setting values in Site Details Javascript.

chrome/browser/ui/webui/settings/site_settings_handler.cc

Index: chrome/browser/ui/webui/settings/site_settings_handler.cc
diff --git a/chrome/browser/ui/webui/settings/site_settings_handler.cc b/chrome/browser/ui/webui/settings/site_settings_handler.cc
index 8d1981cadf58bb580a5d833209a5a17ab03542a8..44c92bb378fe359da33bd1732aca015df2ede13c 100644
--- a/chrome/browser/ui/webui/settings/site_settings_handler.cc
+++ b/chrome/browser/ui/webui/settings/site_settings_handler.cc
@@ -18,12 +18,15 @@
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/web_site_settings_uma_util.h"
 #include "chrome/browser/permissions/chooser_context_base.h"
+#include "chrome/browser/permissions/permission_manager.h"
+#include "chrome/browser/permissions/permission_result.h"
 #include "chrome/browser/permissions/permission_uma_util.h"
 #include "chrome/browser/permissions/permission_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/site_settings_helper.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/content_settings/core/browser/content_settings_utils.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "components/crx_file/id_util.h"
@@ -99,6 +102,80 @@ void AddExceptionsGrantedByHostedApps(content::BrowserContext* context,
   }
 }
 
+// Retrieves the corresponding string, according to the following precedence
+// order from highest to lowest priority:
+//    1. Enterprise policy.
+//    2. Extensions.
+//    3. User-set per-origin setting.
+//    4. Embargo.
+//    5. User-set patterns.
+//    6. User-set global default for a ContentSettingsType.
+//    7. Chrome's built-in default.
+std::string ConvertContentSettingSourceToString(
+    content_settings::SettingSource content_settings_source,
+    PermissionStatusSource permission_status_source) {
+  switch (content_settings_source) {
+    case content_settings::SETTING_SOURCE_POLICY:
+      return site_settings::kPolicyProviderId;
+    case content_settings::SETTING_SOURCE_EXTENSION:
+      return site_settings::kExtensionProviderId;
+
+    case content_settings::SETTING_SOURCE_NONE:
+    case content_settings::SETTING_SOURCE_USER:
+    // The above two sources fall into #3, #5 and #6.
+    case content_settings::SETTING_SOURCE_WHITELIST:
+    case content_settings::SETTING_SOURCE_SUPERVISED:
+      // These are not possible.

[raymes, 2017/06/20 23:42:36]

Hmm, I think these may actually be possible. WHITELIST may not happen in these
cases by chance, but supervised could. I think supervised should use the same
string as the policy provider. Maybe we just return user-defined for whitelist
for now, but if these values are ever returned, the setting shouldn't be
modifiable.

[Patti Lor, 2017/06/21 06:36:37]

Ah, OK, thanks for clarifying. I did some investigating for supervised (made a
supervised account and tried to use it) and could only find a way to
whitelist/blacklist specific origins, not change the content settings of
anything; but I'll defer to you on this. Fixed.

[raymes, 2017/06/22 00:07:03]

I think it's possible for geolocation/camera based on
content_settings_supervised_provider.cc. With that said, I haven't actually
tested it.

[Patti Lor, 2017/06/22 05:31:59]

Oh ok, interesting. Thanks for the FYI.

+      break;
+  }
+
+  switch (permission_status_source) {
+    case PermissionStatusSource::UNSPECIFIED:
+      break;
+    case PermissionStatusSource::SAFE_BROWSING_BLACKLIST:
+    case PermissionStatusSource::KILL_SWITCH:
+    case PermissionStatusSource::MULTIPLE_DISMISSALS:
+    case PermissionStatusSource::MULTIPLE_IGNORES:
+      // The above four sources fall into #4.
+      // TODO(patricialor): Plumb this through to the Web UI.
+      return site_settings::kPreferencesSource;
+  }

[raymes, 2017/06/20 23:42:36]

You've done a good job of this logic considering that it's virtually impossible
to get right!

I might suggest that we reverse the order of checks though. For example, it's
possible that KILL_SWITCH takes precedence over POLICY.

So we might just want to move this switch statement first. If UNSPECIFIED is
returned then fall back to the content settings source. This won't be completely
correct because there are cases right now where we return UNSPECIFIED which
aren't due to falling back to the HostContentSettingsMap. But I think it will be
slightly more correct.

We should definitely prioritize fixing up these decision reasons after v0 :)

[Patti Lor, 2017/06/21 06:36:37]

Ah, thanks for pointing that out. Fixed. I added a comment to document one of
the incorrect cases (the latest patchset will make embargo look like it takes
precedence over a setting the user has explicitly overridden embargo for).

+
+  return site_settings::kPreferencesSource;
+}
+
+ContentSetting GetContentSettingForOrigin(const GURL& origin,
+                                          ContentSettingsType content_type,
+                                          Profile* profile,
+                                          std::string* source_string) {
+  // TODO(patricialor): In future, PermissionManager should know about all
+  // content settings, not just the permissions, plus all the possible sources,
+  // and the calls to HostContentSettingsMap should be removed.
+  content_settings::SettingInfo info;
+  HostContentSettingsMap* map =
+      HostContentSettingsMapFactory::GetForProfile(profile);
+  std::unique_ptr<base::Value> value = map->GetWebsiteSetting(
+      origin, origin, content_type, std::string(), &info);
+
+  // Retrieve the content setting.
+  PermissionResult result(CONTENT_SETTING_DEFAULT,
+                          PermissionStatusSource::UNSPECIFIED);
+  if (PermissionUtil::IsPermission(content_type)) {
+    result = PermissionManager::Get(profile)->GetPermissionStatus(
+        content_type, origin, origin);
+  } else {
+    DCHECK(value.get());
+    DCHECK_EQ(base::Value::Type::INTEGER, value->GetType());
+    result.content_setting =
+        content_settings::ValueToContentSetting(value.get());
+  }
+
+  // Retrieve the source of the content setting.
+  *source_string =
+      ConvertContentSettingSourceToString(info.source, result.source);
+  return result.content_setting;
+}
+
 }  // namespace
 
 
@@ -143,12 +220,12 @@ void SiteSettingsHandler::RegisterMessages() {
       base::Bind(&SiteSettingsHandler::HandleResetCategoryPermissionForOrigin,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
-      "setCategoryPermissionForOrigin",
-      base::Bind(&SiteSettingsHandler::HandleSetCategoryPermissionForOrigin,
+      "getCategoryPermissionForOrigin",
+      base::Bind(&SiteSettingsHandler::HandleGetCategoryPermissionForOrigin,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
-      "getSiteDetails",
-      base::Bind(&SiteSettingsHandler::HandleGetSiteDetails,
+      "setCategoryPermissionForOrigin",
+      base::Bind(&SiteSettingsHandler::HandleSetCategoryPermissionForOrigin,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "isPatternValid",
@@ -501,6 +578,41 @@ void SiteSettingsHandler::HandleResetCategoryPermissionForOrigin(
       content_type, ContentSetting::CONTENT_SETTING_DEFAULT);
 }
 
+void SiteSettingsHandler::HandleGetCategoryPermissionForOrigin(
+    const base::ListValue* args) {
+  AllowJavascript();
+
+  CHECK_EQ(3U, args->GetSize());
+  const base::Value* callback_id;
+  CHECK(args->Get(0, &callback_id));
+  std::string type;
+  CHECK(args->GetString(1, &type));
+  std::string origin;
+  CHECK(args->GetString(2, &origin));
+
+  const GURL origin_url(origin);
+  ContentSettingsType content_type = static_cast<ContentSettingsType>(
+      static_cast<int>(site_settings::ContentSettingsTypeFromGroupName(type)));
+  std::string source_string = "";
+  ContentSetting content_setting = GetContentSettingForOrigin(
+      origin_url, content_type, profile_, &source_string);
+  std::string content_setting_string =
+      content_settings::ContentSettingToString(content_setting);
+
+  auto raw_site_exception = base::MakeUnique<base::DictionaryValue>();
+  raw_site_exception->SetString(site_settings::kEmbeddingOrigin, "");

[raymes, 2017/06/20 23:42:36]

nit: I'm not sure if it matters, but we could just keep the embedding origin the
same as the origin.

[Patti Lor, 2017/06/21 06:36:37]

Done.

+  raw_site_exception->SetBoolean(site_settings::kIncognito,
+                                 profile_->IsOffTheRecord());
+  raw_site_exception->SetString(site_settings::kOrigin, origin);
+  raw_site_exception->SetString(site_settings::kDisplayName, origin);
+  raw_site_exception->SetString(site_settings::kSetting,
+                                content_setting_string);
+  raw_site_exception->SetString(site_settings::kSource, source_string);
+  // The remaining "embeddingDisplayName" field is handled later in Javascript.
+
+  ResolveJavascriptCallback(*callback_id, *raw_site_exception);
+}
+
 void SiteSettingsHandler::HandleSetCategoryPermissionForOrigin(
     const base::ListValue* args) {
   CHECK_EQ(5U, args->GetSize());
@@ -551,69 +663,6 @@ void SiteSettingsHandler::HandleSetCategoryPermissionForOrigin(
   WebSiteSettingsUmaUtil::LogPermissionChange(content_type, setting);
 }
 
-void SiteSettingsHandler::HandleGetSiteDetails(
-    const base::ListValue* args) {
-  AllowJavascript();
-
-  CHECK_EQ(2U, args->GetSize());
-  const base::Value* callback_id;
-  CHECK(args->Get(0, &callback_id));
-  std::string site;
-  CHECK(args->GetString(1, &site));
-
-  // A subset of the ContentSettingsType enum that we show in the settings UI.
-  const ContentSettingsType kSettingsDetailTypes[] = {
-    CONTENT_SETTINGS_TYPE_COOKIES,
-    CONTENT_SETTINGS_TYPE_IMAGES,
-    CONTENT_SETTINGS_TYPE_JAVASCRIPT,
-    CONTENT_SETTINGS_TYPE_PLUGINS,
-    CONTENT_SETTINGS_TYPE_POPUPS,
-    CONTENT_SETTINGS_TYPE_GEOLOCATION,
-    CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
-    CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC,
-    CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
-    CONTENT_SETTINGS_TYPE_PROTOCOL_HANDLERS,
-    CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
-    CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC,
-    CONTENT_SETTINGS_TYPE_USB_CHOOSER_DATA,
-    CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,
-  };
-
-  // Create a list to be consistent with existing API, we are expecting a single
-  // element (or none).
-  std::unique_ptr<base::ListValue> exceptions(new base::ListValue);
-  for (size_t type = 0; type < arraysize(kSettingsDetailTypes); ++type) {
-    ContentSettingsType content_type = kSettingsDetailTypes[type];
-
-    HostContentSettingsMap* map =
-        HostContentSettingsMapFactory::GetForProfile(profile_);
-    const auto* extension_registry =
-        extensions::ExtensionRegistry::Get(profile_);
-    site_settings::GetExceptionsFromHostContentSettingsMap(
-        map, content_type, extension_registry, web_ui(), /*incognito=*/false,
-        /*filter=*/&site, exceptions.get());
-
-    if (profile_->HasOffTheRecordProfile()) {
-      Profile* incognito = profile_->GetOffTheRecordProfile();
-      map = HostContentSettingsMapFactory::GetForProfile(incognito);
-      extension_registry = extensions::ExtensionRegistry::Get(incognito);
-      site_settings::GetExceptionsFromHostContentSettingsMap(
-          map, content_type, extension_registry, web_ui(), /*incognito=*/true,
-          /*filter=*/&site, exceptions.get());
-    }
-  }
-
-  if (!exceptions->GetSize()) {
-    RejectJavascriptCallback(*callback_id, base::Value());
-    return;
-  }
-
-  // We only need a single response element.
-  const base::DictionaryValue* exception = nullptr;
-  exceptions->GetDictionary(0, &exception);
-  ResolveJavascriptCallback(*callback_id, *exception);
-}
-
 void SiteSettingsHandler::HandleIsPatternValid(
     const base::ListValue* args) {
   CHECK_EQ(2U, args->GetSize());