Commit c32aeeeb authored by Noel Gordon's avatar Noel Gordon Committed by Commit Bot

Make file tasks C++ control FilesApp PDF/SWF viewing actions

Viewing PDF files in a renderer (via chrome PDFium) is controlled by a
system pref PluginsAlwaysOpenPdfExternally, which is read once-only at
FileApp startup. Dynamic pref changes are ignored (see bug).

Remove loadTimeData setters for PDF and SWF file viewing actions: file
private_api_strings.cc ShouldBeOpenedWithPlugin calls begone, and same
for the file_task.js loadTimeData code in FilesApp.

Instead, call ShouldBeOpenedWithPlugin in file_tasks.cc. When the list
of file tasks is requested by FilesApp, this new code enables/disables
FilesApp PDF/SWF viewing actions ... dynamically based on prefs.

Add an integration test: tests that QuickView does not show a PDF when
the system PluginsAlwaysOpenPdfExternally pref is enabled.

Test: browser_tests --gtest_filter="*QuickViewPdfPreviewsDisabled*
Bug: 999103
Change-Id: I455fdbd794247a0a57105a2abd6ed929c752a8bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1808943
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: default avatarSam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697515}
parent 63bf1ef9
......@@ -11,7 +11,6 @@
#include "base/strings/stringprintf.h"
#include "chrome/browser/chromeos/crostini/crostini_util.h"
#include "chrome/browser/chromeos/file_manager/file_manager_string_util.h"
#include "chrome/browser/chromeos/file_manager/open_with_browser.h"
#include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
#include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
#include "chrome/browser/profiles/profile.h"
......@@ -32,19 +31,10 @@ FileManagerPrivateGetStringsFunction::~FileManagerPrivateGetStringsFunction() =
ExtensionFunction::ResponseAction FileManagerPrivateGetStringsFunction::Run() {
auto dict = GetFileManagerStrings();
const std::string empty;
dict->SetBoolean("VIDEO_PLAYER_NATIVE_CONTROLS_ENABLED",
base::FeatureList::IsEnabled(
chromeos::features::kVideoPlayerNativeControls));
dict->SetBoolean("PDF_VIEW_ENABLED",
file_manager::util::ShouldBeOpenedWithPlugin(
Profile::FromBrowserContext(browser_context()),
FILE_PATH_LITERAL(".pdf"), empty));
dict->SetBoolean("SWF_VIEW_ENABLED",
file_manager::util::ShouldBeOpenedWithPlugin(
Profile::FromBrowserContext(browser_context()),
FILE_PATH_LITERAL(".swf"), empty));
// TODO(crbug.com/868747): Find a better solution for demo mode.
dict->SetBoolean("HIDE_SPACE_INFO",
chromeos::DemoSession::IsDeviceInDemoMode());
......@@ -67,6 +57,7 @@ ExtensionFunction::ResponseAction FileManagerPrivateGetStringsFunction::Run() {
Profile::FromBrowserContext(browser_context())));
dict->SetBoolean("FILES_NG_ENABLED",
base::FeatureList::IsEnabled(chromeos::features::kFilesNG));
dict->SetString("UI_LOCALE", extension_l10n_util::CurrentLocaleOrDefault());
return RespondNow(OneArgument(std::move(dict)));
......
......@@ -424,6 +424,7 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
// QuickView PDF test fails on MSAN, crbug.com/768070
#if !defined(MEMORY_SANITIZER)
TestCase("openQuickViewPdf"),
TestCase("openQuickViewPdfPreviewsDisabled"),
#endif
TestCase("openQuickViewKeyboardUpDownChangesView"),
TestCase("openQuickViewKeyboardLeftRightChangesView"),
......
......@@ -53,6 +53,7 @@
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/api/file_manager_private.h"
#include "chrome/common/pref_names.h"
#include "chromeos/components/drivefs/drivefs_host.h"
#include "chromeos/components/drivefs/fake_drivefs.h"
#include "chromeos/constants/chromeos_features.h"
......@@ -2079,6 +2080,14 @@ void FileManagerBrowserTestBase::OnCommand(const std::string& name,
return;
}
if (name == "setPdfPreviewEnabled") {
bool enabled;
ASSERT_TRUE(value.GetBoolean("enabled", &enabled));
profile()->GetPrefs()->SetBoolean(prefs::kPluginsAlwaysOpenPdfExternally,
!enabled);
return;
}
if (name == "setCrostiniEnabled") {
bool enabled;
ASSERT_TRUE(value.GetBoolean("enabled", &enabled));
......
......@@ -124,7 +124,7 @@ bool ContainsGoogleDocument(const std::vector<extensions::EntryInfo>& entries) {
return false;
}
// Leaves tasks handled by the file manger itself as is and removes all others.
// Removes all tasks except tasks handled by file manager.
void KeepOnlyFileManagerInternalTasks(std::vector<FullTaskDescriptor>* tasks) {
std::vector<FullTaskDescriptor> filtered;
for (size_t i = 0; i < tasks->size(); ++i) {
......@@ -134,6 +134,22 @@ void KeepOnlyFileManagerInternalTasks(std::vector<FullTaskDescriptor>* tasks) {
tasks->swap(filtered);
}
// Removes task |actions| handled by file manager.
void RemoveFileManagerInternalActions(const std::set<std::string>& actions,
std::vector<FullTaskDescriptor>* tasks) {
std::vector<FullTaskDescriptor> filtered;
for (size_t i = 0; i < tasks->size(); ++i) {
const auto& action = (*tasks)[i].task_descriptor().action_id;
if ((*tasks)[i].task_descriptor().app_id != kFileManagerAppId) {
filtered.push_back((*tasks)[i]);
} else if (actions.find(action) == actions.end()) {
filtered.push_back((*tasks)[i]);
}
}
tasks->swap(filtered);
}
// Returns true if the given task is a handler by built-in apps like the Files
// app itself or QuickOffice etc. They are used as the initial default app.
bool IsFallbackFileHandler(const FullTaskDescriptor& task) {
......@@ -199,6 +215,16 @@ void PostProcessFoundTasks(
// Google documents can only be handled by internal handlers.
if (ContainsGoogleDocument(entries))
KeepOnlyFileManagerInternalTasks(result_list.get());
// Remove file manager internal view-pdf and view-swf actions if needed.
std::set<std::string> disabled_actions;
if (!util::ShouldBeOpenedWithPlugin(profile, FILE_PATH_LITERAL(".pdf"), ""))
disabled_actions.emplace("view-pdf");
if (!util::ShouldBeOpenedWithPlugin(profile, FILE_PATH_LITERAL(".swf"), ""))
disabled_actions.emplace("view-swf");
if (!disabled_actions.empty())
RemoveFileManagerInternalActions(disabled_actions, result_list.get());
ChooseAndSetDefaultTask(*profile->GetPrefs(), entries, result_list.get());
std::move(callback).Run(std::move(result_list));
}
......
......@@ -390,18 +390,10 @@ class FileTasks {
task.title = loadTimeData.getString('TASK_IMPORT_CROSTINI_IMAGE');
task.verb = undefined;
} else if (taskParts[2] === 'view-swf') {
// Do not render this task if disabled.
if (!loadTimeData.getBoolean('SWF_VIEW_ENABLED')) {
continue;
}
task.iconType = 'generic';
task.title = loadTimeData.getString('TASK_VIEW');
task.verb = undefined;
} else if (taskParts[2] === 'view-pdf') {
// Do not render this task if disabled.
if (!loadTimeData.getBoolean('PDF_VIEW_ENABLED')) {
continue;
}
task.iconType = 'pdf';
task.title = loadTimeData.getString('TASK_VIEW');
task.verb = undefined;
......
......@@ -558,6 +558,49 @@
chrome.test.assertEq('application/pdf', type);
};
/**
* Tests that Quick View does not display a PDF file preview when that is
* disabled by system settings (preferences).
*/
testcase.openQuickViewPdfPreviewsDisabled = async () => {
const caller = getCaller();
/**
* The #innerContentPanel resides in the #quick-view shadow DOM as a child
* of the #dialog element, and contains the file preview result.
*/
const contentPanel = ['#quick-view', '#dialog[open] #innerContentPanel'];
// Disable PDF previews.
await sendTestMessage({name: 'setPdfPreviewEnabled', enabled: false});
// Open Files app on Downloads containing ENTRIES.tallPdf.
const appId =
await setupAndWaitUntilReady(RootPath.DOWNLOADS, [ENTRIES.tallPdf], []);
// Open the file in Quick View.
await openQuickView(appId, ENTRIES.tallPdf.nameText);
// Wait for the innerContentPanel to load and display its content.
function checkInnerContentPanel(elements) {
let haveElements = Array.isArray(elements) && elements.length === 1;
if (!haveElements || elements[0].styles.display !== 'flex') {
return pending(caller, 'Waiting for inner content panel to load.');
}
// Check: the PDF preview should not be shown.
chrome.test.assertEq('No preview available', elements[0].text);
return;
}
await repeatUntil(async () => {
return checkInnerContentPanel(await remoteCall.callRemoteTestUtil(
'deepQueryAllElements', appId, [contentPanel, ['display']]));
});
// Check: the correct file mimeType should be displayed.
const mimeType = await getQuickViewMetadataBoxField(appId, 'Type');
chrome.test.assertEq('application/pdf', mimeType);
};
/**
* Tests opening Quick View and scrolling its <webview> which contains a tall
* html document.
......
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