Commit 519abc1e authored by Trent Apted's avatar Trent Apted Committed by Commit Bot

Pass a cross-origin File from chrome://media-app to chrome://media-app-guest

Currently we pass an ArrayBuffer which does not work for large files
such as video. Passing a File works, and doesn't require a copy.

Add media/video to the list of handlers now we know it works. Tested
manually with a 5GB video, and covered by existing unit and integration
tests (with smaller files).

Update the existing file_handler.accept map rather than adding a new
handler due to https://crbug.com/1047509.

BUG=996088, b/144865801

Change-Id: I46b3ba53676534e06ba039c9a52c8701b4d1d432
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2029532Reviewed-by: default avatarBugs Nash <bugsnash@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737175}
parent 5cbfa983
...@@ -38,6 +38,9 @@ constexpr base::FilePath::CharType kTestFilesFolderInTestData[] = ...@@ -38,6 +38,9 @@ constexpr base::FilePath::CharType kTestFilesFolderInTestData[] =
// An 800x600 image/png (all blue pixels). // An 800x600 image/png (all blue pixels).
constexpr char kFilePng800x600[] = "image.png"; constexpr char kFilePng800x600[] = "image.png";
// A 1-second long 648x486 VP9-encoded video with stereo Opus-encoded audio.
constexpr char kFileVideoVP9[] = "world.webm";
class MediaAppIntegrationTest : public SystemWebAppIntegrationTest { class MediaAppIntegrationTest : public SystemWebAppIntegrationTest {
public: public:
MediaAppIntegrationTest() { MediaAppIntegrationTest() {
...@@ -141,26 +144,32 @@ IN_PROC_BROWSER_TEST_F(MediaAppIntegrationTest, MediaAppLaunchWithFile) { ...@@ -141,26 +144,32 @@ IN_PROC_BROWSER_TEST_F(MediaAppIntegrationTest, MediaAppLaunchWithFile) {
// file manager and eligible for opening appropriate files / mime types. // file manager and eligible for opening appropriate files / mime types.
IN_PROC_BROWSER_TEST_F(MediaAppIntegrationTest, MediaAppElibibleOpenTask) { IN_PROC_BROWSER_TEST_F(MediaAppIntegrationTest, MediaAppElibibleOpenTask) {
constexpr bool kIsDirectory = false; constexpr bool kIsDirectory = false;
const extensions::EntryInfo test_entry(TestFile(kFilePng800x600), "image/png", const extensions::EntryInfo image_entry(TestFile(kFilePng800x600),
kIsDirectory); "image/png", kIsDirectory);
const extensions::EntryInfo video_entry(TestFile(kFileVideoVP9), "video/webm",
kIsDirectory);
WaitForTestSystemAppInstall(); WaitForTestSystemAppInstall();
std::vector<file_manager::file_tasks::FullTaskDescriptor> result; for (const auto& single_entry : {video_entry, image_entry}) {
file_manager::file_tasks::FindWebTasks(profile(), {test_entry}, &result); SCOPED_TRACE(single_entry.mime_type);
std::vector<file_manager::file_tasks::FullTaskDescriptor> result;
ASSERT_LT(0u, result.size()); file_manager::file_tasks::FindWebTasks(profile(), {single_entry}, &result);
EXPECT_EQ(1u, result.size());
const auto& task = result[0]; ASSERT_LT(0u, result.size());
const auto& descriptor = task.task_descriptor(); EXPECT_EQ(1u, result.size());
const auto& task = result[0];
EXPECT_EQ("Media App", task.task_title()); const auto& descriptor = task.task_descriptor();
EXPECT_EQ(extensions::api::file_manager_private::Verb::VERB_OPEN_WITH,
task.task_verb()); EXPECT_EQ("Media App", task.task_title());
EXPECT_EQ(descriptor.app_id, EXPECT_EQ(extensions::api::file_manager_private::Verb::VERB_OPEN_WITH,
*GetManager().GetAppIdForSystemApp(web_app::SystemAppType::MEDIA)); task.task_verb());
EXPECT_EQ(chromeos::kChromeUIMediaAppURL, descriptor.action_id); EXPECT_EQ(descriptor.app_id, *GetManager().GetAppIdForSystemApp(
EXPECT_EQ(file_manager::file_tasks::TASK_TYPE_WEB_APP, descriptor.task_type); web_app::SystemAppType::MEDIA));
EXPECT_EQ(chromeos::kChromeUIMediaAppURL, descriptor.action_id);
EXPECT_EQ(file_manager::file_tasks::TASK_TYPE_WEB_APP,
descriptor.task_type);
}
} }
// End-to-end test to ensure that the MediaApp successfully registers as a file // End-to-end test to ensure that the MediaApp successfully registers as a file
......
...@@ -19,14 +19,10 @@ function postToGuestWindow(message) { ...@@ -19,14 +19,10 @@ function postToGuestWindow(message) {
/** /**
* Loads a file in the guest. * Loads a file in the guest.
* *
* @param {Blob} blob * @param {File} file
*/ */
async function loadBlob(blob) { async function loadFile(file) {
// It would be better to pass a blob URL here, but that needs cross-origin postToGuestWindow({'file': file});
// access to Blob URLs, which should come with https://crbug.com/1012150.
// Until then, use go/mdn/API/Blob/arrayBuffer (working draft).
const buffer = await blob.arrayBuffer();
postToGuestWindow({'buffer': buffer, 'type': blob.type});
} }
/** /**
...@@ -41,7 +37,7 @@ async function loadFileFromHandle(handle) { ...@@ -41,7 +37,7 @@ async function loadFileFromHandle(handle) {
const fileHandle = /** @type{FileSystemFileHandle} */ (handle); const fileHandle = /** @type{FileSystemFileHandle} */ (handle);
const file = await fileHandle.getFile(); const file = await fileHandle.getFile();
loadBlob(file); loadFile(file);
} }
// Wait for 'load' (and not DOMContentLoaded) to ensure the subframe has been // Wait for 'load' (and not DOMContentLoaded) to ensure the subframe has been
......
...@@ -79,10 +79,37 @@ mediaApp.ClientApi = function() {}; ...@@ -79,10 +79,37 @@ mediaApp.ClientApi = function() {};
mediaApp.ClientApi.prototype.loadFiles = function(files) {}; mediaApp.ClientApi.prototype.loadFiles = function(files) {};
/** /**
* The message structure sent to the guest over postMessage. * The message structure sent to the guest over postMessage. The presence of
* @typedef{{buffer: ArrayBuffer, type: string, handle: (Object|undefined)}} * a particular field determines the instruction being given to the guest.
*
* @record
* @struct
*/
mediaApp.MessageEventData = function() {};
/**
* File data to load. TODO(b/144865801): Remove this (obsolete).
*
* @type {!ArrayBuffer|undefined}
*/
mediaApp.MessageEventData.prototype.buffer;
/**
* MIME type of the data in `buffer`.
*
* @type {string|undefined}
*/
mediaApp.MessageEventData.prototype.type;
/**
* An object that uniquely identifies a FileSystemFileHandle in the host.
*
* @type {!Object|undefined}
*/
mediaApp.MessageEventData.prototype.handle;
/**
* A File to load.
*
* @type {!File|undefined}
*/ */
mediaApp.MessageEventData; mediaApp.MessageEventData.prototype.file;
/** /**
* Launch data that can be read by the app when it first loads. * Launch data that can be read by the app when it first loads.
......
...@@ -17,14 +17,14 @@ class SingleArrayBufferFileList { ...@@ -17,14 +17,14 @@ class SingleArrayBufferFileList {
/** /**
* Loads files associated with a message received from the host. * Loads files associated with a message received from the host.
* @param {!mediaApp.MessageEventData} data * @param {!File} file
*/ */
async function load(data) { async function loadFile(file) {
const fileList = new SingleArrayBufferFileList({ const fileList = new SingleArrayBufferFileList({
blob: new Blob([data.buffer], {type: data.type}), blob: file,
size: data.buffer.byteLength, size: file.size,
mimeType: data.type, mimeType: file.type,
name: 'buffer', name: file.name,
}); });
const app = /** @type {?mediaApp.ClientApi} */ ( const app = /** @type {?mediaApp.ClientApi} */ (
...@@ -42,11 +42,17 @@ function receiveMessage(/** Event */ e) { ...@@ -42,11 +42,17 @@ function receiveMessage(/** Event */ e) {
return; return;
} }
// Tests messages won't have a buffer (and are not handled by this listener). // First ensure the message is our MessageEventData type, then act on it
if ('buffer' in event.data) { // appropriately. Note test messages won't have a file (and are not handled by
// this listener), so it's currently sufficient to just check for `file`.
if ('file' in event.data) {
const message = const message =
/** @type{MessageEvent<mediaApp.MessageEventData>}*/ (event); /** @type{MessageEvent<mediaApp.MessageEventData>}*/ (event);
load(message.data); if (message.data.file) {
loadFile(message.data.file);
} else {
console.error('Unknown message:', message);
}
} }
} }
......
...@@ -15,9 +15,10 @@ ...@@ -15,9 +15,10 @@
"file_handlers": [ "file_handlers": [
{ {
"action": ".", "action": ".",
"name": "Image File", "name": "Media File",
"accept": { "accept": {
"image/*": [ ] "image/*": [ ],
"video/*": [ ]
} }
} }
] ]
......
...@@ -75,11 +75,12 @@ var MediaAppUIBrowserTest = class extends testing.Test { ...@@ -75,11 +75,12 @@ var MediaAppUIBrowserTest = class extends testing.Test {
const TEST_IMAGE_WIDTH = 123; const TEST_IMAGE_WIDTH = 123;
const TEST_IMAGE_HEIGHT = 456; const TEST_IMAGE_HEIGHT = 456;
/** @return {Blob} A 123x456 transparent encoded image/png. */ /** @return {Promise<File>} A 123x456 transparent encoded image/png. */
function createTestImageBlob() { async function createTestImageFile() {
const canvas = new OffscreenCanvas(TEST_IMAGE_WIDTH, TEST_IMAGE_HEIGHT); const canvas = new OffscreenCanvas(TEST_IMAGE_WIDTH, TEST_IMAGE_HEIGHT);
canvas.getContext('2d'); // convertToBlob fails without a rendering context. canvas.getContext('2d'); // convertToBlob fails without a rendering context.
return canvas.convertToBlob(); const blob = await canvas.convertToBlob();
return new File([blob], 'test_file.png', {type: 'image/png'});
} }
// Tests that chrome://media-app is allowed to frame chrome://media-app-guest. // Tests that chrome://media-app is allowed to frame chrome://media-app-guest.
...@@ -100,8 +101,8 @@ TEST_F('MediaAppUIBrowserTest', 'GuestCanLoad', async () => { ...@@ -100,8 +101,8 @@ TEST_F('MediaAppUIBrowserTest', 'GuestCanLoad', async () => {
testDone(); testDone();
}); });
TEST_F('MediaAppUIBrowserTest', 'LoadBlob', async () => { TEST_F('MediaAppUIBrowserTest', 'LoadFile', async () => {
loadBlob(await createTestImageBlob()); loadFile(await createTestImageFile());
const result = const result =
await driver.waitForElementInGuest('img[src^="blob:"]', 'naturalWidth'); await driver.waitForElementInGuest('img[src^="blob:"]', 'naturalWidth');
......
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