0

Cleanup PdfHonorJsContentSettings feature flag

The feature has been enabled by default on ToT since crrev.com/813922,
which was released with M88.

Bug: 696650
Change-Id: Idb9f2fef29052c8b420de5867a5f43b9f5298984
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2543188
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828907}
This commit is contained in:
Daniel Hosseinian
2020-11-18 21:44:06 +00:00
committed by Commit Bot
parent a062a3aafa
commit 36982484a5
8 changed files with 8 additions and 77 deletions

@ -4493,11 +4493,6 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kPdfFormSaveDescription, kOsDesktop,
FEATURE_VALUE_TYPE(chrome_pdf::features::kSaveEditedPDFForm)},
{"pdf-honor-js-content-settings",
flag_descriptions::kPdfHonorJsContentSettingsName,
flag_descriptions::kPdfHonorJsContentSettingsDescription, kOsDesktop,
FEATURE_VALUE_TYPE(chrome_pdf::features::kPdfHonorJsContentSettings)},
{"pdf-viewer-update", flag_descriptions::kPdfViewerUpdateName,
flag_descriptions::kPdfViewerUpdateDescription, kOsDesktop,
FEATURE_VALUE_TYPE(chrome_pdf::features::kPDFViewerUpdate)},

@ -4741,12 +4741,6 @@ const char kPdfFormSaveName[] = "Save PDF Forms";
const char kPdfFormSaveDescription[] =
"Enable saving PDFs with filled form data.";
const char kPdfHonorJsContentSettingsName[] =
"Honor JavaScript content settings in PDFs";
const char kPdfHonorJsContentSettingsDescription[] =
"Enable whether an origin's JavaScript content settings are honored in "
"PDFs opened from that origin.";
const char kPdfViewerUpdateName[] = "PDF Viewer Update";
const char kPdfViewerUpdateDescription[] =
"When enabled, the PDF viewer will display an updated UI with new "

@ -2783,9 +2783,6 @@ extern const char kPaintPreviewStartupDescription[];
extern const char kPdfFormSaveName[];
extern const char kPdfFormSaveDescription[];
extern const char kPdfHonorJsContentSettingsName[];
extern const char kPdfHonorJsContentSettingsDescription[];
extern const char kPdfViewerUpdateName[];
extern const char kPdfViewerUpdateDescription[];
#endif // BUILDFLAG(ENABLE_PLUGINS)

@ -1064,36 +1064,12 @@ INSTANTIATE_TEST_SUITE_P(/* no prefix */, PDFExtensionJSTest, testing::Bool());
class PDFExtensionContentSettingJSTest
: public PDFExtensionJSTestBase,
public testing::WithParamInterface<std::pair<bool, bool>> {
public testing::WithParamInterface<bool> {
public:
~PDFExtensionContentSettingJSTest() override = default;
protected:
const std::vector<base::Feature> GetEnabledFeatures() const override {
std::vector<base::Feature> features;
if (ShouldHonorJsContentSettings()) {
features.push_back(chrome_pdf::features::kPdfHonorJsContentSettings);
}
if (ShouldEnablePDFViewerUpdate()) {
features.push_back(chrome_pdf::features::kPDFViewerUpdate);
}
return features;
}
const std::vector<base::Feature> GetDisabledFeatures() const override {
std::vector<base::Feature> features;
if (!ShouldHonorJsContentSettings()) {
features.push_back(chrome_pdf::features::kPdfHonorJsContentSettings);
}
if (!ShouldEnablePDFViewerUpdate()) {
features.push_back(chrome_pdf::features::kPDFViewerUpdate);
}
return features;
}
bool ShouldEnablePDFViewerUpdate() const override { return GetParam().first; }
bool ShouldHonorJsContentSettings() const { return GetParam().second; }
bool ShouldEnablePDFViewerUpdate() const override { return GetParam(); }
// When blocking JavaScript, block the exact query from pdf/main.js while
// still allowing enough JavaScript to run in the extension for the test
@ -1108,10 +1084,6 @@ class PDFExtensionContentSettingJSTest
ContentSettingsType::JAVASCRIPT,
enabled ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK);
}
std::string GetDisabledJsTestFile() const {
return ShouldHonorJsContentSettings() ? "nobeep_test.js" : "beep_test.js";
}
};
IN_PROC_BROWSER_TEST_P(PDFExtensionContentSettingJSTest, Beep) {
@ -1120,13 +1092,13 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionContentSettingJSTest, Beep) {
IN_PROC_BROWSER_TEST_P(PDFExtensionContentSettingJSTest, NoBeep) {
SetPdfJavaScript(/*enabled=*/false);
RunTestsInJsModule(GetDisabledJsTestFile(), "test-beep.pdf");
RunTestsInJsModule("nobeep_test.js", "test-beep.pdf");
}
IN_PROC_BROWSER_TEST_P(PDFExtensionContentSettingJSTest, BeepThenNoBeep) {
RunTestsInJsModule("beep_test.js", "test-beep.pdf");
SetPdfJavaScript(/*enabled=*/false);
RunTestsInJsModuleNewTab(GetDisabledJsTestFile(), "test-beep.pdf");
RunTestsInJsModuleNewTab("nobeep_test.js", "test-beep.pdf");
// Make sure there are two PDFs in the same process.
const int tab_count = browser()->tab_strip_model()->count();
@ -1136,7 +1108,7 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionContentSettingJSTest, BeepThenNoBeep) {
IN_PROC_BROWSER_TEST_P(PDFExtensionContentSettingJSTest, NoBeepThenBeep) {
SetPdfJavaScript(/*enabled=*/false);
RunTestsInJsModule(GetDisabledJsTestFile(), "test-beep.pdf");
RunTestsInJsModule("nobeep_test.js", "test-beep.pdf");
SetPdfJavaScript(/*enabled=*/true);
RunTestsInJsModuleNewTab("beep_test.js", "test-beep.pdf");
@ -1148,10 +1120,7 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionContentSettingJSTest, NoBeepThenBeep) {
INSTANTIATE_TEST_SUITE_P(/* no prefix */,
PDFExtensionContentSettingJSTest,
testing::ValuesIn({std::make_pair(true, false),
std::make_pair(false, false),
std::make_pair(true, true),
std::make_pair(false, true)}));
testing::Bool());
// Service worker tests are regression tests for
// https://crbug.com/916514.

@ -555,10 +555,8 @@ bool OutOfProcessInstance::Init(uint32_t argc,
success =
base::StringToInt(argv[i], &top_toolbar_height_in_viewport_coords_);
} else if (strcmp(argn[i], "javascript") == 0) {
if (base::FeatureList::IsEnabled(features::kPdfHonorJsContentSettings)) {
if (strcmp(argv[i], "allow") != 0)
script_option = PDFiumFormFiller::ScriptOption::kNoJavaScript;
}
if (strcmp(argv[i], "allow") != 0)
script_option = PDFiumFormFiller::ScriptOption::kNoJavaScript;
} else if (strcmp(argn[i], "has-edits") == 0) {
has_edits = true;
}

@ -12,9 +12,6 @@ namespace features {
const base::Feature kAccessiblePDFForm = {"AccessiblePDFForm",
base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kPdfHonorJsContentSettings = {
"PdfHonorJsContentSettings", base::FEATURE_ENABLED_BY_DEFAULT};
// "Incremental loading" refers to loading the PDF as it arrives.
// TODO(crbug.com/1064175): Remove this once incremental loading is fixed.
const base::Feature kPdfIncrementalLoading = {

@ -14,7 +14,6 @@ namespace chrome_pdf {
namespace features {
extern const base::Feature kAccessiblePDFForm;
extern const base::Feature kPdfHonorJsContentSettings;
extern const base::Feature kPdfIncrementalLoading;
extern const base::Feature kPdfPartialLoading;
extern const base::Feature kPdfViewerPresentationMode;

@ -5264,24 +5264,6 @@
]
}
],
"PdfHonorJsContentSettings": [
{
"platforms": [
"chromeos",
"linux",
"mac",
"windows"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"PdfHonorJsContentSettings"
]
}
]
}
],
"PerProcessReclaim": [
{
"platforms": [