Commit 717a34a3 authored by Tommy C. Li's avatar Tommy C. Li Committed by Commit Bot

Flash: Delete PluginsFieldTrial abstraction.

Now that kPreferHtmlOverPlugins feature has launched, enabled by
default, and now removed from the code as a flag, we can delete this
abstraction.

This folds the mapping of ASK => DETECT logic into PluginUtils
directly and cleans up the callsites.

This CL should not change Chrome's behavior in any way.

Bug: 945035
Change-Id: I4298aed3b4c5f95e4323cb658c9d42ead6c31cfb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1542496Reviewed-by: default avatarRaymes Khoury <raymes@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#645354}
parent 9e2de384
...@@ -4571,8 +4571,6 @@ jumbo_split_static_library("browser") { ...@@ -4571,8 +4571,6 @@ jumbo_split_static_library("browser") {
"plugins/plugin_status_pref_setter.h", "plugins/plugin_status_pref_setter.h",
"plugins/plugin_utils.cc", "plugins/plugin_utils.cc",
"plugins/plugin_utils.h", "plugins/plugin_utils.h",
"plugins/plugins_field_trial.cc",
"plugins/plugins_field_trial.h",
"plugins/plugins_resource_service.cc", "plugins/plugins_resource_service.cc",
"plugins/plugins_resource_service.h", "plugins/plugins_resource_service.h",
"plugins/reload_plugin_infobar_delegate.cc", "plugins/reload_plugin_infobar_delegate.cc",
......
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
#include "chrome/browser/plugins/plugin_finder.h" #include "chrome/browser/plugins/plugin_finder.h"
#include "chrome/browser/plugins/plugin_metadata.h" #include "chrome/browser/plugins/plugin_metadata.h"
#include "chrome/browser/plugins/plugin_utils.h" #include "chrome/browser/plugins/plugin_utils.h"
#include "chrome/browser/plugins/plugins_field_trial.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/render_messages.h" #include "chrome/common/render_messages.h"
...@@ -210,8 +209,6 @@ bool ChromePluginServiceFilter::IsPluginAvailable( ...@@ -210,8 +209,6 @@ bool ChromePluginServiceFilter::IsPluginAvailable(
context_info_it->second->host_content_settings_map.get(); context_info_it->second->host_content_settings_map.get();
ContentSetting flash_setting = PluginUtils::GetFlashPluginContentSetting( ContentSetting flash_setting = PluginUtils::GetFlashPluginContentSetting(
settings_map, main_frame_origin, plugin_content_url, &is_managed); settings_map, main_frame_origin, plugin_content_url, &is_managed);
flash_setting = PluginsFieldTrial::EffectiveContentSetting(
settings_map, CONTENT_SETTINGS_TYPE_PLUGINS, flash_setting);
if (flash_setting == CONTENT_SETTING_ALLOW) if (flash_setting == CONTENT_SETTING_ALLOW)
return true; return true;
......
...@@ -20,7 +20,6 @@ ...@@ -20,7 +20,6 @@
#include "chrome/browser/plugins/plugin_finder.h" #include "chrome/browser/plugins/plugin_finder.h"
#include "chrome/browser/plugins/plugin_metadata.h" #include "chrome/browser/plugins/plugin_metadata.h"
#include "chrome/browser/plugins/plugin_prefs.h" #include "chrome/browser/plugins/plugin_prefs.h"
#include "chrome/browser/plugins/plugins_field_trial.h"
#include "chrome/common/chrome_content_client.h" #include "chrome/common/chrome_content_client.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h"
......
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include "chrome/browser/content_settings/tab_specific_content_settings.h" #include "chrome/browser/content_settings/tab_specific_content_settings.h"
#include "chrome/browser/permissions/permission_manager.h" #include "chrome/browser/permissions/permission_manager.h"
#include "chrome/browser/plugins/plugin_utils.h" #include "chrome/browser/plugins/plugin_utils.h"
#include "chrome/browser/plugins/plugins_field_trial.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "components/content_settings/core/common/content_settings_types.h" #include "components/content_settings/core/common/content_settings_types.h"
...@@ -78,8 +77,6 @@ void FlashDownloadInterception::InterceptFlashDownloadNavigation( ...@@ -78,8 +77,6 @@ void FlashDownloadInterception::InterceptFlashDownloadNavigation(
ContentSetting flash_setting = PluginUtils::GetFlashPluginContentSetting( ContentSetting flash_setting = PluginUtils::GetFlashPluginContentSetting(
host_content_settings_map, url::Origin::Create(source_url), source_url, host_content_settings_map, url::Origin::Create(source_url), source_url,
nullptr); nullptr);
flash_setting = PluginsFieldTrial::EffectiveContentSetting(
host_content_settings_map, CONTENT_SETTINGS_TYPE_PLUGINS, flash_setting);
if (flash_setting == CONTENT_SETTING_DETECT_IMPORTANT_CONTENT) { if (flash_setting == CONTENT_SETTING_DETECT_IMPORTANT_CONTENT) {
PermissionManager* manager = PermissionManager::Get(profile); PermissionManager* manager = PermissionManager::Get(profile);
...@@ -133,9 +130,6 @@ bool FlashDownloadInterception::ShouldStopFlashDownloadAction( ...@@ -133,9 +130,6 @@ bool FlashDownloadInterception::ShouldStopFlashDownloadAction(
ContentSetting flash_setting = PluginUtils::GetFlashPluginContentSetting( ContentSetting flash_setting = PluginUtils::GetFlashPluginContentSetting(
host_content_settings_map, url::Origin::Create(source_url), source_url, host_content_settings_map, url::Origin::Create(source_url), source_url,
nullptr); nullptr);
flash_setting = PluginsFieldTrial::EffectiveContentSetting(
host_content_settings_map, CONTENT_SETTINGS_TYPE_PLUGINS,
flash_setting);
return flash_setting == CONTENT_SETTING_DETECT_IMPORTANT_CONTENT || return flash_setting == CONTENT_SETTING_DETECT_IMPORTANT_CONTENT ||
flash_setting == CONTENT_SETTING_BLOCK; flash_setting == CONTENT_SETTING_BLOCK;
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "chrome/browser/permissions/permission_request_id.h" #include "chrome/browser/permissions/permission_request_id.h"
#include "chrome/browser/plugins/flash_temporary_permission_tracker.h" #include "chrome/browser/plugins/flash_temporary_permission_tracker.h"
#include "chrome/browser/plugins/plugin_utils.h" #include "chrome/browser/plugins/plugin_utils.h"
#include "chrome/browser/plugins/plugins_field_trial.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/site_settings_helper.h" #include "chrome/browser/ui/webui/site_settings_helper.h"
#include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/content_settings/core/browser/host_content_settings_map.h"
...@@ -47,8 +46,6 @@ ContentSetting FlashPermissionContext::GetPermissionStatusInternal( ...@@ -47,8 +46,6 @@ ContentSetting FlashPermissionContext::GetPermissionStatusInternal(
ContentSetting flash_setting = PluginUtils::GetFlashPluginContentSetting( ContentSetting flash_setting = PluginUtils::GetFlashPluginContentSetting(
host_content_settings_map, url::Origin::Create(embedding_origin), host_content_settings_map, url::Origin::Create(embedding_origin),
requesting_origin, nullptr); requesting_origin, nullptr);
flash_setting = PluginsFieldTrial::EffectiveContentSetting(
host_content_settings_map, content_settings_type(), flash_setting);
if (flash_setting == CONTENT_SETTING_DETECT_IMPORTANT_CONTENT) if (flash_setting == CONTENT_SETTING_DETECT_IMPORTANT_CONTENT)
return CONTENT_SETTING_ASK; return CONTENT_SETTING_ASK;
return flash_setting; return flash_setting;
......
...@@ -25,7 +25,6 @@ ...@@ -25,7 +25,6 @@
#include "chrome/browser/plugins/plugin_metadata.h" #include "chrome/browser/plugins/plugin_metadata.h"
#include "chrome/browser/plugins/plugin_prefs.h" #include "chrome/browser/plugins/plugin_prefs.h"
#include "chrome/browser/plugins/plugin_utils.h" #include "chrome/browser/plugins/plugin_utils.h"
#include "chrome/browser/plugins/plugins_field_trial.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser_otr_state.h" #include "chrome/browser/ui/browser_otr_state.h"
#include "chrome/common/buildflags.h" #include "chrome/common/buildflags.h"
...@@ -274,20 +273,13 @@ void PluginInfoHostImpl::Context::DecidePluginStatus( ...@@ -274,20 +273,13 @@ void PluginInfoHostImpl::Context::DecidePluginStatus(
plugin_identifier, &plugin_setting, &uses_default_content_setting, plugin_identifier, &plugin_setting, &uses_default_content_setting,
&is_managed); &is_managed);
// TODO(tommycli): Remove once we deprecate the plugin ASK policy.
bool legacy_ask_user = plugin_setting == CONTENT_SETTING_ASK;
plugin_setting = PluginsFieldTrial::EffectiveContentSetting(
host_content_settings_map_, CONTENT_SETTINGS_TYPE_PLUGINS,
plugin_setting);
DCHECK(plugin_setting != CONTENT_SETTING_DEFAULT); DCHECK(plugin_setting != CONTENT_SETTING_DEFAULT);
DCHECK(plugin_setting != CONTENT_SETTING_ASK); DCHECK(plugin_setting != CONTENT_SETTING_ASK);
if (*status == chrome::mojom::PluginStatus::kFlashHiddenPreferHtml) { if (*status == chrome::mojom::PluginStatus::kFlashHiddenPreferHtml) {
if (plugin_setting == CONTENT_SETTING_BLOCK) { if (plugin_setting == CONTENT_SETTING_BLOCK) {
*status = is_managed && !legacy_ask_user *status = is_managed ? chrome::mojom::PluginStatus::kBlockedByPolicy
? chrome::mojom::PluginStatus::kBlockedByPolicy : chrome::mojom::PluginStatus::kBlockedNoLoading;
: chrome::mojom::PluginStatus::kBlockedNoLoading;
} }
return; return;
} }
...@@ -329,11 +321,8 @@ void PluginInfoHostImpl::Context::DecidePluginStatus( ...@@ -329,11 +321,8 @@ void PluginInfoHostImpl::Context::DecidePluginStatus(
!run_all_flash_in_allow_mode_.GetValue())) { !run_all_flash_in_allow_mode_.GetValue())) {
*status = chrome::mojom::PluginStatus::kPlayImportantContent; *status = chrome::mojom::PluginStatus::kPlayImportantContent;
} else if (plugin_setting == CONTENT_SETTING_BLOCK) { } else if (plugin_setting == CONTENT_SETTING_BLOCK) {
// For managed users with the ASK policy, we allow manually running plugins *status = is_managed ? chrome::mojom::PluginStatus::kBlockedByPolicy
// via context menu. This is the closest to admin intent. : chrome::mojom::PluginStatus::kBlocked;
*status = is_managed && !legacy_ask_user
? chrome::mojom::PluginStatus::kBlockedByPolicy
: chrome::mojom::PluginStatus::kBlocked;
} }
#if BUILDFLAG(ENABLE_EXTENSIONS) #if BUILDFLAG(ENABLE_EXTENSIONS)
......
...@@ -93,6 +93,11 @@ void GetPluginContentSettingInternal( ...@@ -93,6 +93,11 @@ void GetPluginContentSettingInternal(
*setting = CONTENT_SETTING_BLOCK; *setting = CONTENT_SETTING_BLOCK;
} }
} }
// For Plugins, ASK is obsolete. Show as DETECT_IMPORTANT_CONTENT to reflect
// actual behavior.
if (*setting == ContentSetting::CONTENT_SETTING_ASK)
*setting = ContentSetting::CONTENT_SETTING_DETECT_IMPORTANT_CONTENT;
} }
} // namespace } // namespace
......
// Copyright 2015 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/plugins/plugins_field_trial.h"
#include <string>
#include "base/feature_list.h"
#include "base/strings/string_number_conversions.h"
#include "chrome/browser/plugins/plugin_utils.h"
// static
ContentSetting PluginsFieldTrial::EffectiveContentSetting(
const HostContentSettingsMap* host_content_settings_map,
ContentSettingsType type,
ContentSetting setting) {
if (type != CONTENT_SETTINGS_TYPE_PLUGINS ||
setting != ContentSetting::CONTENT_SETTING_ASK) {
return setting;
}
// For Plugins, ASK is obsolete. Show as DETECT_IMPORTANT_CONTENT to reflect
// actual behavior.
return ContentSetting::CONTENT_SETTING_DETECT_IMPORTANT_CONTENT;
}
// Copyright 2015 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.
#ifndef CHROME_BROWSER_PLUGINS_PLUGINS_FIELD_TRIAL_H_
#define CHROME_BROWSER_PLUGINS_PLUGINS_FIELD_TRIAL_H_
#include "base/macros.h"
#include "components/content_settings/core/common/content_settings.h"
#include "components/content_settings/core/common/content_settings_types.h"
class HostContentSettingsMap;
// This class manages the Plugins field trials.
class PluginsFieldTrial {
public:
// Returns the effective content setting for plugins. Passes non-plugin
// content settings through without modification.
// TODO(tommycli): Eliminate this method and fold the logic into
// HostContentSettingsMap.
static ContentSetting EffectiveContentSetting(
const HostContentSettingsMap* host_content_settings_map,
ContentSettingsType type,
ContentSetting setting);
private:
DISALLOW_IMPLICIT_CONSTRUCTORS(PluginsFieldTrial);
};
#endif // CHROME_BROWSER_PLUGINS_PLUGINS_FIELD_TRIAL_H_
...@@ -13,7 +13,6 @@ ...@@ -13,7 +13,6 @@
#include "chrome/browser/permissions/permission_manager.h" #include "chrome/browser/permissions/permission_manager.h"
#include "chrome/browser/permissions/permission_result.h" #include "chrome/browser/permissions/permission_result.h"
#include "chrome/browser/permissions/permission_util.h" #include "chrome/browser/permissions/permission_util.h"
#include "chrome/browser/plugins/plugins_field_trial.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
...@@ -187,18 +186,11 @@ ContentSetting GetEffectiveSetting(Profile* profile, ...@@ -187,18 +186,11 @@ ContentSetting GetEffectiveSetting(Profile* profile,
if (effective_setting == CONTENT_SETTING_DEFAULT) if (effective_setting == CONTENT_SETTING_DEFAULT)
effective_setting = default_setting; effective_setting = default_setting;
#if BUILDFLAG(ENABLE_PLUGINS)
HostContentSettingsMap* host_content_settings_map =
HostContentSettingsMapFactory::GetForProfile(profile);
effective_setting = PluginsFieldTrial::EffectiveContentSetting(
host_content_settings_map, type, effective_setting);
// Display the UI string for ASK instead of DETECT for Flash. // Display the UI string for ASK instead of DETECT for Flash.
// TODO(tommycli): Just migrate the actual content setting to ASK. // TODO(tommycli): Just migrate the actual content setting to ASK.
if (effective_setting == CONTENT_SETTING_DETECT_IMPORTANT_CONTENT) if (effective_setting == CONTENT_SETTING_DETECT_IMPORTANT_CONTENT)
effective_setting = CONTENT_SETTING_ASK; effective_setting = CONTENT_SETTING_ASK;
#endif
return effective_setting; return effective_setting;
} }
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#include "chrome/browser/ui/page_info/permission_menu_model.h" #include "chrome/browser/ui/page_info/permission_menu_model.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/plugins/plugins_field_trial.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
#include "content/public/common/origin_util.h" #include "content/public/common/origin_util.h"
...@@ -24,22 +23,14 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile, ...@@ -24,22 +23,14 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile,
DCHECK(!callback_.is_null()); DCHECK(!callback_.is_null());
base::string16 label; base::string16 label;
// Retrieve the string to show for the default setting for this permission. DCHECK_NE(permission_.default_setting, CONTENT_SETTING_NUM_SETTINGS);
ContentSetting effective_default_setting = permission_.default_setting;
DCHECK_NE(effective_default_setting, CONTENT_SETTING_NUM_SETTINGS);
#if BUILDFLAG(ENABLE_PLUGINS)
effective_default_setting = PluginsFieldTrial::EffectiveContentSetting(
host_content_settings_map_, permission_.type,
permission_.default_setting);
#endif // BUILDFLAG(ENABLE_PLUGINS)
// The Material UI for site settings uses comboboxes instead of menubuttons, // The Material UI for site settings uses comboboxes instead of menubuttons,
// which means the elements of the menu themselves have to be shorter, instead // which means the elements of the menu themselves have to be shorter, instead
// of simply setting a shorter label on the menubutton. // of simply setting a shorter label on the menubutton.
label = PageInfoUI::PermissionActionToUIString( label = PageInfoUI::PermissionActionToUIString(
profile, permission_.type, CONTENT_SETTING_DEFAULT, profile, permission_.type, CONTENT_SETTING_DEFAULT,
effective_default_setting, permission_.source); permission_.default_setting, permission_.source);
AddCheckItem(CONTENT_SETTING_DEFAULT, label); AddCheckItem(CONTENT_SETTING_DEFAULT, label);
...@@ -47,13 +38,13 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile, ...@@ -47,13 +38,13 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile,
if (ShouldShowAllow(url)) { if (ShouldShowAllow(url)) {
label = PageInfoUI::PermissionActionToUIString( label = PageInfoUI::PermissionActionToUIString(
profile, permission_.type, CONTENT_SETTING_ALLOW, profile, permission_.type, CONTENT_SETTING_ALLOW,
effective_default_setting, permission_.source); permission_.default_setting, permission_.source);
AddCheckItem(CONTENT_SETTING_ALLOW, label); AddCheckItem(CONTENT_SETTING_ALLOW, label);
} }
// Retrieve the string to show for blocking the permission. // Retrieve the string to show for blocking the permission.
label = PageInfoUI::PermissionActionToUIString( label = PageInfoUI::PermissionActionToUIString(
profile, info.type, CONTENT_SETTING_BLOCK, effective_default_setting, profile, info.type, CONTENT_SETTING_BLOCK, permission_.default_setting,
info.source); info.source);
AddCheckItem(CONTENT_SETTING_BLOCK, label); AddCheckItem(CONTENT_SETTING_BLOCK, label);
...@@ -61,7 +52,7 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile, ...@@ -61,7 +52,7 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile,
// permission. // permission.
if (ShouldShowAsk(url)) { if (ShouldShowAsk(url)) {
label = PageInfoUI::PermissionActionToUIString( label = PageInfoUI::PermissionActionToUIString(
profile, info.type, CONTENT_SETTING_ASK, effective_default_setting, profile, info.type, CONTENT_SETTING_ASK, permission_.default_setting,
info.source); info.source);
AddCheckItem(CONTENT_SETTING_ASK, label); AddCheckItem(CONTENT_SETTING_ASK, label);
} }
...@@ -70,14 +61,7 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile, ...@@ -70,14 +61,7 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile,
PermissionMenuModel::~PermissionMenuModel() {} PermissionMenuModel::~PermissionMenuModel() {}
bool PermissionMenuModel::IsCommandIdChecked(int command_id) const { bool PermissionMenuModel::IsCommandIdChecked(int command_id) const {
ContentSetting setting = permission_.setting; return permission_.setting == command_id;
#if BUILDFLAG(ENABLE_PLUGINS)
setting = PluginsFieldTrial::EffectiveContentSetting(
host_content_settings_map_, permission_.type, permission_.setting);
#endif // BUILDFLAG(ENABLE_PLUGINS)
return setting == command_id;
} }
bool PermissionMenuModel::IsCommandIdEnabled(int command_id) const { bool PermissionMenuModel::IsCommandIdEnabled(int command_id) const {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment