Views/Permissions: Update desktop UI to display BLOCK for embargoed permissions.

chrome/browser/ui/website_settings/website_settings.cc

Index: chrome/browser/ui/website_settings/website_settings.cc
diff --git a/chrome/browser/ui/website_settings/website_settings.cc b/chrome/browser/ui/website_settings/website_settings.cc
index a8549dddb7e8f68511f29b23c857127dbecb6272..96fcf94b9e8d1d520451e1c562567fba34141cbc 100644
--- a/chrome/browser/ui/website_settings/website_settings.cc
+++ b/chrome/browser/ui/website_settings/website_settings.cc
@@ -32,6 +32,8 @@
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/infobars/infobar_service.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"
@@ -56,6 +58,7 @@
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/elide_url.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/permission_type.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
@@ -674,6 +677,22 @@ void WebsiteSettings::PresentSitePermissions() {
                                                       NULL);
     }
 
+    // For permissions that are still prompting the user and haven't been
+    // explicitly set by another source, check its embargo status.
+    content::PermissionType unused = content::PermissionType::NUM;
+    if (PermissionUtil::GetPermissionType(permission_info.type, &unused) &&

[raymes, 2017/03/06 03:55:05]

I'd like to avoid using PermissionUtil::GetPermissionType here since the
PermissionType is kind of irrelevant for this code. We wanted to make that
function private. Could we have a list of permissions we explicitly want to
check the embargo status of for now?

We should have a TODO here to use GetPermissionStatus for all permissions at
some point at which point we can address how to deal with things that aren't
currently handled by PermissionManager properly.

[Patti Lor, 2017/03/06 06:47:54]

As discussed offline, reverted to use PermissionUtil::IsPermission from patch
set 1 and added the TODO under your name.

+        permission_info.setting == CONTENT_SETTING_DEFAULT &&
+        permission_info.source ==
+            content_settings::SettingSource::SETTING_SOURCE_USER) {
+      PermissionResult permission_result =
+          PermissionManager::Get(profile_)->GetPermissionStatus(
+              permission_info.type, site_url_, site_url_);
+
+      // If under embargo, update |permission_info| to reflect that.
+      if (permission_result.content_setting == CONTENT_SETTING_BLOCK)

[raymes, 2017/03/06 03:55:05]

Is it worth also checking permission_result.source here to see if it came from
embargo?

[dominickn, 2017/03/06 04:10:10]

I suggested not doing that for consistency with the Android implementation
(which similarly just checks that the content_setting is BLOCK). It's also
consistent with the internal permissions code, which doesn't check the source
and returns it straight away.

[Patti Lor, 2017/03/06 06:47:54]

Leaving this as is :)

+        permission_info.setting = permission_result.content_setting;
+    }
+
     permission_info_list.push_back(permission_info);
   }