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..e9e3041887d5d651bcf664d2e3e510072dba6e90 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,58 @@ void AddExceptionsGrantedByHostedApps(content::BrowserContext* context,
   }
 }
 
+PermissionResult GetPermissionResultForOrigin(const GURL& requesting_origin,
+                                              const GURL& embedding_origin,
+                                              ContentSettingsType content_type,
+                                              Profile* profile) {
+  // TODO(patricialor): In future, PermissionManager should know about all
+  // content settings, not just the permissions, plus all the possible sources.
+  PermissionResult result(CONTENT_SETTING_DEFAULT,
+                          PermissionStatusSource::UNSPECIFIED);
+  content_settings::SettingInfo info;
+  HostContentSettingsMap* map =
+      HostContentSettingsMapFactory::GetForProfile(profile);
+  std::unique_ptr<base::Value> value = map->GetWebsiteSetting(
+      requesting_origin, embedding_origin, content_type, std::string(), &info);
+
+  // Retrieve the content setting.
+  if (PermissionUtil::IsPermission(content_type)) {
+    result = PermissionManager::Get(profile)->GetPermissionStatus(
+        content_type, requesting_origin, embedding_origin);
+  } else {
+    DCHECK(value.get());
+    if (value->GetType() == base::Value::Type::INTEGER)

[raymes, 2017/06/19 04:13:12]

Could this just be a DCHECK_EQ instead of a NOTREACHED below?

[Patti Lor, 2017/06/20 08:25:54]

Done.

+      result.content_setting =
+          content_settings::ValueToContentSetting(value.get());
+    else
+      NOTREACHED();
+  }
+
+  // Retrieve the source of the content setting. The PermissionManager currently
+  // only knows about SAFE_BROWSING_BLACKLIST, KILL_SWITCH, MULTIPLE_DISMISSALS,
+  // and MULTIPLE_IGNORES, so overwrite the PermissionStatusSource if the source
+  // is none of those.
+  switch (result.source) {
+    case PermissionStatusSource::UNSPECIFIED:
+      break;
+    case PermissionStatusSource::SAFE_BROWSING_BLACKLIST:
+    case PermissionStatusSource::KILL_SWITCH:
+    case PermissionStatusSource::MULTIPLE_DISMISSALS:
+    case PermissionStatusSource::MULTIPLE_IGNORES:
+      return result;
+    case PermissionStatusSource::INSECURE_ORIGIN:
+    case PermissionStatusSource::ENTERPRISE_POLICY:
+    case PermissionStatusSource::EXTENSION:

[raymes, 2017/06/19 04:13:12]

I think having the PermissionStatusSource's that aren't currently used in the
permission system adds some complexity that mightn't be needed right now. Could
we instead convert directly from PermissionStatusSource or content settings
source to the string that we need? Maybe We could have a single helper function
in this file to do that? 

[Patti Lor, 2017/06/20 08:25:54]

Done (with the helper function approach). I realised there was a bug currently
where if there was an embargoed setting that was also controlled by policy or
extension it would return that it was controlled by embargo, so I changed the
order to fix that.

+      // If these are now implemented, update this code to remove unnecessary
+      // calls to HostContentSettingsMap.
+      NOTREACHED();
+  }
+
+  result.source =
+      PermissionUtil::ConvertSettingSourceToPermissionStatusSource(info.source);
+  return result;
+}
+
 }  // namespace
 
 
@@ -143,12 +198,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 +556,60 @@ void SiteSettingsHandler::HandleResetCategoryPermissionForOrigin(
       content_type, ContentSetting::CONTENT_SETTING_DEFAULT);
 }
 
+void SiteSettingsHandler::HandleGetCategoryPermissionForOrigin(
+    const base::ListValue* args) {
+  AllowJavascript();
+
+  CHECK_EQ(4U, 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));
+  std::string embedding_origin;
+  CHECK(args->GetString(3, &embedding_origin));
+
+  ContentSettingsType content_type = static_cast<ContentSettingsType>(
+      static_cast<int>(site_settings::ContentSettingsTypeFromGroupName(type)));
+  PermissionResult result = GetPermissionResultForOrigin(
+      GURL(origin), GURL(embedding_origin), content_type, profile_);
+
+  std::string content_setting_string =
+      content_settings::ContentSettingToString(result.content_setting);
+  std::string source_string = "";
+  switch (result.source) {
+    case PermissionStatusSource::UNSPECIFIED:
+    case PermissionStatusSource::SAFE_BROWSING_BLACKLIST:
+    case PermissionStatusSource::KILL_SWITCH:
+    case PermissionStatusSource::MULTIPLE_DISMISSALS:
+    case PermissionStatusSource::MULTIPLE_IGNORES:
+    case PermissionStatusSource::INSECURE_ORIGIN:
+      // TODO(patricialor): Plumb these sources through to the Web UI.
+      break;
+    case PermissionStatusSource::ENTERPRISE_POLICY:
+      source_string = site_settings::kPolicyProviderId;
+      break;
+    case PermissionStatusSource::EXTENSION:
+      source_string = site_settings::kExtensionProviderId;
+      break;
+  }
+
+  auto raw_site_exception = base::MakeUnique<base::DictionaryValue>();

[raymes, 2017/06/19 04:13:12]

nit: what does raw mean here?

[Patti Lor, 2017/06/20 08:25:54]

This method is supposed to return a RawSiteException (see
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/site_s...).
expandSiteException (called in <site-details>'s onOriginChanged() is meant to
convert that to a SiteException, similar to the original code.

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

Thanks for explaining :)

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

Acknowledged :D

+  raw_site_exception->SetString(site_settings::kEmbeddingOrigin,
+                                embedding_origin);
+  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 +660,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());