Commit a5087a2f authored by Zain Afzal's avatar Zain Afzal Committed by Commit Bot

Added save as IPC.

Due to some ux decisions the media app now needs to support a "save as"
operation where the current file is saved in a new location and the app
is navigated to the new file. This means that "saveCopy" is no longer an
accurate name so we are renaming it to "saveAs". Additionally this CL
adds some additional logic to slip the newly saved file into the
navigable files list so users can navigate back to the original file or
any other file in the directory.

To prevent TOT being broken this CL leaves saveCopy as is for older
version of the media app client code. Once the client is updated
saveCopy will be deleted in a separate CL.

Google3 side of this change: cl/323288209.

Bug: b/161193454
Change-Id: Ic66dcfdbcefbee985a9764701cb0bcee229020aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2316189
Commit-Queue: Zain Afzal <zafzal@google.com>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794829}
parent f7ff8bd5
......@@ -195,6 +195,55 @@ guestMessagePipe.registerHandler(Message.SAVE_COPY, async (message) => {
await saveBlobToFile(handle, blob);
});
guestMessagePipe.registerHandler(Message.SAVE_AS, async (message) => {
const {blob, oldFileToken, pickedFileToken} =
/** @type {!SaveAsMessage} */ (message);
const oldFileDescriptor = currentFiles.find(fd => fd.token === oldFileToken);
/** @type {!FileDescriptor} */
const pickedFileDescriptor = {
// We silently take over the old file's file descriptor by taking its token,
// note we can be passed an undefined token if the file we are saving was
// dragged into the media app.
token: oldFileToken || tokenGenerator.next().value,
file: null,
handle: tokenMap.get(pickedFileToken)
};
const oldFileIndex = currentFiles.findIndex(fd => fd.token === oldFileToken);
tokenMap.set(pickedFileDescriptor.token, pickedFileDescriptor.handle);
// Give the old file a new token, if we couldn't find the old file we assume
// its been deleted (or pasted/dragged into the media app) and skip this
// step.
if (oldFileDescriptor) {
oldFileDescriptor.token = generateToken(oldFileDescriptor.handle);
}
try {
// Note `pickedFileHandle` could be the same as a `FileSystemFileHandle`
// that exists in `tokenMap`. Possibly even the `File` currently open. But
// that's OK. E.g. the next overwrite-file request will just invoke
// `saveBlobToFile` in the same way. Note there may be no currently writable
// file (e.g. save from clipboard).
await saveBlobToFile(pickedFileDescriptor.handle, blob);
} catch (/** @type {!DOMException} */ e) {
// If something went wrong revert the token back to its original
// owner so future file actions function correctly.
if (oldFileDescriptor && oldFileToken) {
oldFileDescriptor.token = oldFileToken;
tokenMap.set(oldFileToken, oldFileDescriptor.handle);
}
throw e;
}
// Note: oldFileIndex may be `-1` here which causes the new file to be added
// to the start of the array, this is WAI.
currentFiles.splice(oldFileIndex + 1, 0, pickedFileDescriptor);
// Silently update entry index without triggering a reload of the media app.
entryIndex = oldFileIndex + 1;
/** @type {!SaveAsResponse} */
const response = {newFilename: pickedFileDescriptor.handle.name};
return response;
});
/**
* Shows a file picker to get a writable file.
* @param {string} suggestedName
......@@ -231,7 +280,8 @@ const tokenGenerator = (function*() {
assertCast(crypto).getRandomValues(randomBuffer);
for (let i = 0; i < randomBuffer.length; ++i) {
const token = randomBuffer[i];
if (!tokenMap.has(token)) {
// Disallow "0" as a token.
if (token && !tokenMap.has(token)) {
yield Number(token);
}
}
......
......@@ -76,6 +76,14 @@ mediaApp.AbstractFile.prototype.deleteOriginalFile;
* @type {function(string): !Promise<number>|undefined}
*/
mediaApp.AbstractFile.prototype.renameOriginalFile;
/**
* A function that will save the provided blob in the file pointed to by
* pickedFileToken. Once saved the new file takes over this.token and becomes
* currently writable. The original file is given a new token
* and pushed forward in the navigation order.
* @type {function(!Blob, number): !Promise<undefined>|undefined}
*/
mediaApp.AbstractFile.prototype.saveAs;
/**
* Wraps an HTML FileList object.
......
......@@ -21,6 +21,7 @@ const Message = {
OVERWRITE_FILE: 'overwrite-file',
RENAME_FILE: 'rename-file',
REQUEST_SAVE_FILE: 'request-save-file',
SAVE_AS: 'save-as',
SAVE_COPY: 'save-copy'
};
......@@ -138,3 +139,22 @@ let RequestSaveFileResponse;
* @typedef {{blob: !Blob, token: number}}
*/
let SaveCopyMessage;
/**
* Message sent by the unprivileged context to the privileged context requesting
* for the provided blob to be saved in the location specified by
* `pickedFileToken`. Once saved the new file takes over oldFileToken if it is
* provided, else it gives itself a fresh token, then it becomes currently
* writable. The file specified by oldFileToken is given a new token and pushed
* forward in the navigation order. This method can be called with any file, not
* just the currently writable file.
* @typedef {{blob: !Blob, oldFileToken: ?number, pickedFileToken: number}}
*/
let SaveAsMessage;
/**
* Response message sent by the privileged context with the name of the new
* current file.
* @typedef {{newFilename: string}}
*/
let SaveAsResponse;
......@@ -70,6 +70,23 @@ class ReceivedFile {
Message.RENAME_FILE, {token: this.token, newFilename: newName}));
return renameResponse.renameResult;
}
/**
* @override
* @param {!Blob} blob
* @param {number} pickedFileToken
* @return {!Promise<undefined>}
*/
async saveAs(blob, pickedFileToken) {
/** @type {!SaveAsMessage} */
const message = {blob, oldFileToken: this.token, pickedFileToken};
const result = /** @type {!SaveAsResponse} */ (
await parentMessagePipe.sendMessage(Message.SAVE_AS, message));
this.name = result.newFilename;
this.blob = blob;
this.size = blob.size;
this.mimeType = blob.type;
}
}
/**
......
......@@ -23,7 +23,7 @@ let TestMessageResponseData;
* renameLastFile: (string|undefined),
* requestFullscreen: (boolean|undefined),
* requestSaveFile: (boolean|undefined),
* saveCopy: (boolean|undefined),
* saveAs: (string|undefined),
* testQuery: string,
* }}
*/
......
......@@ -97,15 +97,23 @@ async function runTestQuery(data) {
existingFile.name, existingFile.mimeType);
result = token.toString();
}
} else if (data.saveCopy) {
} else if (data.saveAs) {
const existingFile = assertCast(lastReceivedFileList).item(0);
if (!existingFile) {
result = 'saveCopy failed, no file loaded';
result = 'saveAs failed, no file loaded';
} else {
const token = await DELEGATE.requestSaveFile(
existingFile.name, existingFile.mimeType);
await DELEGATE.saveCopy(existingFile, token);
result = 'file successfully saved';
const file = firstReceivedItem();
try {
const token = await DELEGATE.requestSaveFile(
existingFile.name, existingFile.mimeType);
const testBlob = new Blob([data.saveAs]);
await assertCast(file.saveAs).call(file, testBlob, token);
result = file.name;
extraResultData = {blobText: await file.blob.text()};
} catch (/** @type{!Error} */ error) {
result = `saveAs failed Error: ${error}`;
extraResultData = {filename: file.name};
}
}
} else if (data.getFileErrors) {
result =
......
......@@ -445,10 +445,10 @@ TEST_F('MediaAppUIBrowserTest', 'NavigateWithUnopenableSibling', async () => {
assertEquals(result, '222');
assertEquals(currentFiles.length, 3);
// The error stays on the third, now unopenable. But, since we've advanced,
// it has now rotated into the second slot. But! Also we don't validate it
// until it rotates into the first slot, so the error won't be present yet.
// If we implement pre-loading, this expectation can change to
// The error stays on the third, now unopenable. But, since we've advanced, it
// has now rotated into the second slot. But! Also we don't validate it until
// it rotates into the first slot, so the error won't be present yet. If we
// implement pre-loading, this expectation can change to
// ',NotAllowedError,'.
assertEquals(await getFileErrors(), ',,');
......@@ -841,20 +841,68 @@ TEST_F('MediaAppUIBrowserTest', 'RequestSaveFileIPC', async () => {
testDone();
});
// Tests the IPC behind the saveCopy delegate function.
TEST_F('MediaAppUIBrowserTest', 'SaveCopyIPC', async () => {
// Tests the IPC behind the saveAs function on received files.
TEST_F('MediaAppUIBrowserTest', 'SaveAsIPC', async () => {
// Mock out choose file system entries since it can only be interacted with
// via trusted user gestures.
const newFileHandle = new FakeFileSystemFileHandle();
const newFileHandle = new FakeFileSystemFileHandle('new_file.jpg');
window.showSaveFilePicker = () => Promise.resolve(newFileHandle);
const testImage = await createTestImageFile(10, 10);
await loadFile(testImage, new FakeFileSystemFileHandle());
const testHandle = new FakeFileSystemFileHandle('original_file.jpg');
await loadFile(testImage, testHandle);
const originalFileToken = currentFiles[0].token;
assertEquals(entryIndex, 0);
const result = await sendTestMessage({saveCopy: true});
assertEquals(result.testQueryResult, 'file successfully saved');
const result = await sendTestMessage({saveAs: 'foo'});
// Make sure the receivedFile object has the correct state.
assertEquals(result.testQueryResult, 'new_file.jpg');
assertEquals(await result.testQueryResultData['blobText'], 'foo');
// Confirm the right string was written to the new file.
const writeResult = await newFileHandle.lastWritable.closePromise;
assertEquals(await writeResult.text(), await testImage.text());
assertEquals(await writeResult.text(), 'foo');
// Make sure we have created a new file descriptor, and that
// the original file is still available.
assertEquals(entryIndex, 1);
assertEquals(currentFiles[0].handle, testHandle);
assertEquals(currentFiles[0].handle.name, 'original_file.jpg');
assertNotEquals(currentFiles[0].token, originalFileToken);
assertEquals(currentFiles[1].handle, newFileHandle);
assertEquals(currentFiles[1].handle.name, 'new_file.jpg');
assertEquals(currentFiles[1].token, originalFileToken);
assertEquals(tokenMap.get(currentFiles[0].token), currentFiles[0].handle);
assertEquals(tokenMap.get(currentFiles[1].token), currentFiles[1].handle);
testDone();
});
// Tests the error handling behind the saveAs function on received files.
TEST_F('MediaAppUIBrowserTest', 'SaveAsErrorHandling', async () => {
// Prevent the trusted context from throwing errors which cause the test to
// fail.
guestMessagePipe.logClientError = error => console.log(JSON.stringify(error));
guestMessagePipe.rethrowErrors = false;
const newFileHandle = new FakeFileSystemFileHandle('new_file.jpg');
newFileHandle.nextCreateWritableError =
new DOMException('Fake exception', 'FakeError');
window.showSaveFilePicker = () => Promise.resolve(newFileHandle);
const testImage = await createTestImageFile(10, 10);
const testHandle = new FakeFileSystemFileHandle('original_file.jpg');
await loadFile(testImage, testHandle);
const originalFileToken = currentFiles[0].token;
const result = await sendTestMessage({saveAs: 'foo'});
// Make sure we revert back to our original state.
assertEquals(
result.testQueryResult,
'saveAs failed Error: FakeError: save-as: Fake exception');
assertEquals(result.testQueryResultData['filename'], 'original_file.jpg');
assertEquals(entryIndex, 0);
assertEquals(currentFiles.length, 1);
assertEquals(currentFiles[0].handle, testHandle);
assertEquals(currentFiles[0].handle.name, 'original_file.jpg');
assertEquals(currentFiles[0].token, originalFileToken);
assertEquals(tokenMap.get(currentFiles[0].token), currentFiles[0].handle);
testDone();
});
......
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