Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7227)

Unified Diff: chrome/browser/ui/website_settings/website_settings.cc

Issue 2702923002: Page Info: Hide default permissions with a value of Ask if the default is Ask. (Closed)
Patch Set: Page Info: Add cross-platform logic to hide permissions with default values behind a button. Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 74add62f94d44c0fe10dbeaa7189e5c264acf9d2..92f6ebdf3ba8914ccf7b1069a4dae114675e1566 100644
--- a/chrome/browser/ui/website_settings/website_settings.cc
+++ b/chrome/browser/ui/website_settings/website_settings.cc
@@ -11,6 +11,7 @@
#include <vector>
#include "base/command_line.h"
+#include "base/feature_list.h"
#include "base/i18n/time_formatting.h"
#include "base/macros.h"
#include "base/memory/ptr_util.h"
@@ -84,6 +85,10 @@ using content::BrowserThread;
namespace {
+// TODO(crbug.com/695725): Change this to FEATURE_DISABLED_BY_DEFAULT
+const base::Feature kPageInfoAlwaysShowAllPermissions{
+ "PageInfoAlwaysShowAllPermissions", base::FEATURE_ENABLED_BY_DEFAULT};
+
// Events for UMA. Do not reorder or change!
enum SSLCertificateDecisionsDidRevoke {
USER_CERT_DECISIONS_NOT_REVOKED = 0,
@@ -218,6 +223,7 @@ WebsiteSettings::WebsiteSettings(
tab_specific_content_settings),
content::WebContentsObserver(web_contents),
ui_(ui),
+ show_all_permissions_(false),
show_info_bar_(false),
site_url_(url),
site_identity_status_(SITE_IDENTITY_STATUS_UNKNOWN),
@@ -231,6 +237,10 @@ WebsiteSettings::WebsiteSettings(
security_level_(security_state::NONE) {
Init(url, security_info);
+ if (base::FeatureList::IsEnabled(kPageInfoAlwaysShowAllPermissions)) {
+ show_all_permissions_ = true;
+ }
+
PresentSitePermissions();
PresentSiteData();
PresentSiteIdentity();
@@ -318,6 +328,15 @@ void WebsiteSettings::OnSitePermissionChanged(ContentSettingsType type,
show_info_bar_ = true;
// Refresh the UI to reflect the new setting.
+#if defined(OS_MACOSX)
+ // TODO(crbug.com/695690): This is currently only needed on Mac, and breaks
+ // accessibility.
+ PresentSitePermissions();
+#endif
+}
+
+void WebsiteSettings::OnPresentAllSitePermissions() {
+ show_all_permissions_ = true;
PresentSitePermissions();
}
@@ -644,6 +663,9 @@ void WebsiteSettings::PresentSitePermissions() {
PermissionInfoList permission_info_list;
ChosenObjectInfoList chosen_object_info_list;
+ bool anyPermissionsShown = false;
+ bool anyPermissionsHidden = false;
+
WebsiteSettingsUI::PermissionInfo permission_info;
for (size_t i = 0; i < arraysize(kPermissionType); ++i) {
permission_info.type = kPermissionType[i];
@@ -654,6 +676,7 @@ void WebsiteSettings::PresentSitePermissions() {
content_settings::SettingInfo info;
std::unique_ptr<base::Value> value = content_settings_->GetWebsiteSetting(
site_url_, site_url_, permission_info.type, std::string(), &info);
+
DCHECK(value.get());
if (value->GetType() == base::Value::Type::INTEGER) {
permission_info.setting =
@@ -675,7 +698,13 @@ void WebsiteSettings::PresentSitePermissions() {
NULL);
}
- permission_info_list.push_back(permission_info);
+ if (show_all_permissions_ ||
+ permission_info.setting != CONTENT_SETTING_DEFAULT) {
+ permission_info_list.push_back(permission_info);
+ anyPermissionsShown = true;
+ } else {
+ anyPermissionsHidden = true;
+ }
}
for (const ChooserUIInfo& ui_info : kChooserUIInfo) {
@@ -686,11 +715,25 @@ void WebsiteSettings::PresentSitePermissions() {
chosen_object_info_list.push_back(
base::MakeUnique<WebsiteSettingsUI::ChosenObjectInfo>(
ui_info, std::move(object)));
+ anyPermissionsShown = true;
}
}
ui_->SetPermissionInfo(permission_info_list,
std::move(chosen_object_info_list));
+
+ WebsiteSettingsUI::VisiblePermissions visible_permissions;
+ if (anyPermissionsShown) {
+ if (anyPermissionsHidden) {
+ visible_permissions =
+ WebsiteSettingsUI::VISIBLE_PERMISSIONS_SOME_BUT_NOT_ALL;
+ } else {
+ visible_permissions = WebsiteSettingsUI::VISIBLE_PERMISSIONS_ALL;
+ }
+ } else {
+ visible_permissions = WebsiteSettingsUI::VISIBLE_PERMISSIONS_NONE;
+ }
+ ui_->UpdatePermissionButton(visible_permissions);
}
void WebsiteSettings::PresentSiteData() {

Powered by Google App Engine
This is Rietveld 408576698