Commit f345bd4f authored by Joel Hockey's avatar Joel Hockey Committed by Commit Bot

FilesApp Trash: fix error when Trash is deleted

Our cached reference to .Trash/files and .Trash/info become invalid if
the .Trash dir is ever deleted.

Bug: 953310
Change-Id: Ide1914e4df0ccf932cd8bed30b7a30fab1646003
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2512331Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823036}
parent 17a43904
...@@ -36,6 +36,7 @@ class TrashConfig { ...@@ -36,6 +36,7 @@ class TrashConfig {
* this is the user's homedir (/home/<username>). * this is the user's homedir (/home/<username>).
*/ */
constructor(rootType, topDir, trashDir, prefixPathWithRemoteMount = false) { constructor(rootType, topDir, trashDir, prefixPathWithRemoteMount = false) {
this.id = `${rootType}-${topDir}`;
this.rootType = rootType; this.rootType = rootType;
this.topDir = topDir; this.topDir = topDir;
this.trashDir = trashDir; this.trashDir = trashDir;
...@@ -119,7 +120,14 @@ class Trash { ...@@ -119,7 +120,14 @@ class Trash {
if (!permanentlyDelete) { if (!permanentlyDelete) {
const config = this.shouldMoveToTrash(volumeManager, entry); const config = this.shouldMoveToTrash(volumeManager, entry);
if (config) { if (config) {
return this.trashFileOrDirectory_(entry, config); return this.trashFileOrDirectory_(entry, config).catch(error => {
console.log(
('Error deleting ' + entry.toURL() +
', will refresh trashdir and try again'),
error);
delete this.trashDirs_[config.id];
return this.trashFileOrDirectory_(entry, config);
});
} }
} }
return this.permanentlyDeleteFileOrDirectory_(entry); return this.permanentlyDeleteFileOrDirectory_(entry);
...@@ -151,8 +159,7 @@ class Trash { ...@@ -151,8 +159,7 @@ class Trash {
* @private * @private
*/ */
async getTrashDirs_(entry, config) { async getTrashDirs_(entry, config) {
const key = `${config.rootType}-${config.topDir}`; let trashDirs = this.trashDirs_[config.id];
let trashDirs = this.trashDirs_[key];
if (trashDirs) { if (trashDirs) {
return trashDirs; return trashDirs;
} }
...@@ -169,7 +176,7 @@ class Trash { ...@@ -169,7 +176,7 @@ class Trash {
trashDirs = new TrashDirs(trashFiles, trashInfo); trashDirs = new TrashDirs(trashFiles, trashInfo);
// Check and remove old items max once per session. // Check and remove old items max once per session.
this.removeOldItems_(trashDirs, Date.now()); this.removeOldItems_(trashDirs, Date.now());
this.trashDirs_[key] = trashDirs; this.trashDirs_[config.id] = trashDirs;
return trashDirs; return trashDirs;
} }
......
...@@ -172,8 +172,10 @@ async function testDownloadsHasOwnTrash(done) { ...@@ -172,8 +172,10 @@ async function testDownloadsHasOwnTrash(done) {
const file1 = MockFileEntry.create(fs, '/file1', null, new Blob(['f1'])); const file1 = MockFileEntry.create(fs, '/file1', null, new Blob(['f1']));
const dir2 = MockDirectoryEntry.create(fs, '/Downloads'); const dir2 = MockDirectoryEntry.create(fs, '/Downloads');
const file2 = const file2 =
MockFileEntry.create(fs, '/Downloads/file2', null, new Blob(['f1'])); MockFileEntry.create(fs, '/Downloads/file2', null, new Blob(['f2']));
assertEquals(4, Object.keys(fs.entries).length); const file3 =
MockFileEntry.create(fs, '/Downloads/file3', null, new Blob(['f3']));
assertEquals(5, Object.keys(fs.entries).length);
// Move /file1 to trash. // Move /file1 to trash.
await trash.removeFileOrDirectory(volumeManager, file1, deletePermanently); await trash.removeFileOrDirectory(volumeManager, file1, deletePermanently);
...@@ -182,31 +184,40 @@ async function testDownloadsHasOwnTrash(done) { ...@@ -182,31 +184,40 @@ async function testDownloadsHasOwnTrash(done) {
assertTrue(fs.entries['/.Trash/info'].isDirectory); assertTrue(fs.entries['/.Trash/info'].isDirectory);
assertTrue(fs.entries['/.Trash/files/file1'].isFile); assertTrue(fs.entries['/.Trash/files/file1'].isFile);
assertTrue(fs.entries['/.Trash/info/file1.trashinfo'].isFile); assertTrue(fs.entries['/.Trash/info/file1.trashinfo'].isFile);
assertEquals(8, Object.keys(fs.entries).length); assertEquals(9, Object.keys(fs.entries).length);
// Move /files2 (in Downloads to trash. // Move /Downloads/file2 to trash.
await trash.removeFileOrDirectory(volumeManager, file2, deletePermanently); await trash.removeFileOrDirectory(volumeManager, file2, deletePermanently);
assertTrue(fs.entries['/Downloads/.Trash'].isDirectory); assertTrue(fs.entries['/Downloads/.Trash'].isDirectory);
assertTrue(fs.entries['/Downloads/.Trash/files'].isDirectory); assertTrue(fs.entries['/Downloads/.Trash/files'].isDirectory);
assertTrue(fs.entries['/Downloads/.Trash/info'].isDirectory); assertTrue(fs.entries['/Downloads/.Trash/info'].isDirectory);
assertTrue(fs.entries['/Downloads/.Trash/files/file2'].isFile); assertTrue(fs.entries['/Downloads/.Trash/files/file2'].isFile);
assertTrue(fs.entries['/Downloads/.Trash/info/file2.trashinfo'].isFile); assertTrue(fs.entries['/Downloads/.Trash/info/file2.trashinfo'].isFile);
assertEquals(12, Object.keys(fs.entries).length); assertEquals(13, Object.keys(fs.entries).length);
// Delete /Downloads/.Trash/files/file2. // Delete /Downloads/.Trash/files/file2.
const file2Trashed = fs.entries['/Downloads/.Trash/files/file2']; const file2Trashed = fs.entries['/Downloads/.Trash/files/file2'];
assertFalse(!!trash.shouldMoveToTrash(volumeManager, file2Trashed)); assertFalse(!!trash.shouldMoveToTrash(volumeManager, file2Trashed));
await trash.removeFileOrDirectory( await trash.removeFileOrDirectory(
volumeManager, file2Trashed, deletePermanently); volumeManager, file2Trashed, deletePermanently);
assertEquals(11, Object.keys(fs.entries).length); assertEquals(12, Object.keys(fs.entries).length);
// Delete /Downloads/.Trash. // Delete /Downloads/.Trash.
const downloadsTrash = fs.entries['/Downloads/.Trash']; const downloadsTrash = fs.entries['/Downloads/.Trash'];
assertFalse(!!trash.shouldMoveToTrash(volumeManager, downloadsTrash)); assertFalse(!!trash.shouldMoveToTrash(volumeManager, downloadsTrash));
await trash.removeFileOrDirectory( await trash.removeFileOrDirectory(
volumeManager, downloadsTrash, deletePermanently); volumeManager, downloadsTrash, deletePermanently);
assertEquals(7, Object.keys(fs.entries).length); assertFalse(!!fs.entries['/Downloads/.Trash']);
assertEquals(8, Object.keys(fs.entries).length);
// Move /Downloads/file3 to trash, should recreate /Downloads/.Trash.
await trash.removeFileOrDirectory(volumeManager, file3, deletePermanently);
assertTrue(fs.entries['/Downloads/.Trash'].isDirectory);
assertTrue(fs.entries['/Downloads/.Trash/files'].isDirectory);
assertTrue(fs.entries['/Downloads/.Trash/info'].isDirectory);
assertTrue(fs.entries['/Downloads/.Trash/files/file3'].isFile);
assertTrue(fs.entries['/Downloads/.Trash/info/file3.trashinfo'].isFile);
assertEquals(12, Object.keys(fs.entries).length);
done(); done();
} }
......
...@@ -262,6 +262,7 @@ class MockEntry { ...@@ -262,6 +262,7 @@ class MockEntry {
Promise.resolve().then(() => { Promise.resolve().then(() => {
for (const path in this.filesystem.entries) { for (const path in this.filesystem.entries) {
if (path.startsWith(this.fullPath)) { if (path.startsWith(this.fullPath)) {
this.filesystem.entries[path].removed_ = true;
delete this.filesystem.entries[path]; delete this.filesystem.entries[path];
} }
} }
...@@ -446,6 +447,10 @@ class MockDirectoryEntry extends MockEntry { ...@@ -446,6 +447,10 @@ class MockDirectoryEntry extends MockEntry {
// default them to be no-ops to save on checking their validity later. // default them to be no-ops to save on checking their validity later.
onSuccess = onSuccess || (entry => {}); // no-op onSuccess = onSuccess || (entry => {}); // no-op
onError = onError || (error => {}); // no-op onError = onError || (error => {}); // no-op
if (this.removed_) {
return onError(
/** @type {!FileError} */ ({name: util.FileError.NOT_FOUND_ERR}));
}
option = option || {}; option = option || {};
const fullPath = path[0] === '/' ? path : joinPath(this.fullPath, path); const fullPath = path[0] === '/' ? path : joinPath(this.fullPath, path);
const result = this.filesystem.entries[fullPath]; const result = this.filesystem.entries[fullPath];
......
...@@ -82,6 +82,15 @@ testcase.trashMoveToTrash = async () => { ...@@ -82,6 +82,15 @@ testcase.trashMoveToTrash = async () => {
// Wait for completion of file deletion. // Wait for completion of file deletion.
await remoteCall.waitForElementLost(appId, '#file-list [file-name=".Trash"]'); await remoteCall.waitForElementLost(appId, '#file-list [file-name=".Trash"]');
// Delete photos dir (no dialog),
await remoteCall.waitAndClickElement(
appId, '#file-list [file-name="photos"]');
await remoteCall.waitAndClickElement(appId, '#delete-button');
// Wait for photos to be removed, and .Trash to be recreated.
await remoteCall.waitForElementLost(appId, '#file-list [file-name="photos"]');
await remoteCall.waitForElement(appId, '#file-list [file-name=".Trash"]');
}; };
/** /**
......
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