Skip to content

Commit

Permalink
CCA: Fix issue that thumbnail might not be updated when do bulk deletion
Browse files Browse the repository at this point in the history
When delete many files at once, sometimes the next candidate file will
be deleted while generating the thumbnail and cause error. This CL fixes
the following issues by:

1. Catch DOMException when operating the candidate thumbnail file and
   retry after the camera folder becomes stable.
2. Drive-by fix an issue that might fail when reporting error.

Bug: b:255502878
Test: Manually
Change-Id: I6a3e345571d1dc987e09af64742c0d54dcc58bf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3993623
Reviewed-by: Chu-Hsuan Yang <chuhsuan@chromium.org>
Auto-Submit: Wei Lee <wtlee@chromium.org>
Commit-Queue: Chu-Hsuan Yang <chuhsuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1067450}
  • Loading branch information
Wei Lee authored and Chromium LUCI CQ committed Nov 4, 2022
1 parent ed348a4 commit 36befd6
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 15 deletions.
8 changes: 4 additions & 4 deletions ash/webui/camera_app_ui/resources/js/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@ function getStackFrames(error: Error): StackFrame[] {
}
};

let frames: StackFrame[];
let frames: StackFrame[] = [];
if (typeof error.stack === 'string') {
// TODO(b/223324206): There is a known issue that when reporting error from
// intent instance, the type from |error.stack| will be a string instead.
frames = parseStackTrace(error.stack);
} else {
} else if (Array.isArray(error.stack)) {
// Generally, error.stack returns whatever Error.prepareStackTrace returns.
// Since we override Error.prepareStackTrace to return StackFrame[] here,
// using "as unknown" first so that we can cast the type to StackFrame[].
Expand All @@ -96,10 +96,10 @@ function getErrorDescription(error: Error): string {
/**
* Gets formatted string stack from error.
*/
function formatErrorStack(error: Error, frames: StackFrame[]|null): string {
function formatErrorStack(error: Error, frames: StackFrame[]): string {
const errorDesc = getErrorDescription(error);
return errorDesc +
(frames ?? [])
frames
.map(({fileName, funcName, lineNo, colNo}) => {
let position = '';
if (lineNo !== -1) {
Expand Down
65 changes: 54 additions & 11 deletions ash/webui/camera_app_ui/resources/js/gallerybutton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
MimeType,
VideoType,
} from './type.js';
import {WaitableEvent} from './waitable_event.js';

/**
* Cover photo of gallery button.
Expand Down Expand Up @@ -100,6 +101,8 @@ export class GalleryButton implements ResultSaver {

private readonly coverPhoto: HTMLImageElement;

private retryingCheckCover = false;

constructor() {
this.coverPhoto = dom.getFrom(this.button, 'img', HTMLImageElement);

Expand Down Expand Up @@ -138,8 +141,12 @@ export class GalleryButton implements ResultSaver {
this.coverPhoto.src = cover?.url ?? '';

if (file !== null) {
ChromeHelper.getInstance().monitorFileDeletion(file.name, () => {
this.checkCover();
ChromeHelper.getInstance().monitorFileDeletion(file.name, async () => {
try {
await this.checkCover();
} catch (e) {
reportError(ErrorType.CHECK_COVER_FAILURE, ErrorLevel.ERROR, e);
}
});
}
}
Expand Down Expand Up @@ -167,15 +174,51 @@ export class GalleryButton implements ResultSaver {
await this.updateCover(null);
return;
}
const filesWithTime = await Promise.all(
files.map(async (file) => ({
file,
time: (await file.getLastModificationTime()),
})));
const lastFile =
filesWithTime.reduce((last, cur) => last.time > cur.time ? last : cur)
.file;
await this.updateCover(lastFile);

try {
const filesWithTime = await Promise.all(
files.map(async (file) => ({
file,
time: (await file.getLastModificationTime()),
})));
const lastFile =
filesWithTime.reduce((last, cur) => last.time > cur.time ? last : cur)
.file;
await this.updateCover(lastFile);
} catch (e) {
// The file might be deleted at any time and cause the operation
// interrupted. Since it might take a while when doing bulk deletion, only
// try check cover again if the amount of files become stable.
if (e instanceof DOMException && !this.retryingCheckCover) {
this.retryingCheckCover = true;
try {
await this.waitUntilCameraFolderStable();
await this.checkCover();
} finally {
this.retryingCheckCover = false;
}
} else {
throw e;
}
}
}

private async waitUntilCameraFolderStable(): Promise<void> {
let prevFileCount = (await filesystem.getEntries()).length;
const cameraFolderStable = new WaitableEvent();

async function checkFileCount() {
const newFileCount = (await filesystem.getEntries()).length;
if (prevFileCount === newFileCount) {
clearInterval(intervalId);
cameraFolderStable.signal();
} else {
prevFileCount = newFileCount;
}
}

const intervalId = setInterval(checkFileCount, 500);
return cameraFolderStable.wait();
}

async savePhoto(blob: Blob, name: string, metadata: Metadata|null):
Expand Down
1 change: 1 addition & 0 deletions ash/webui/camera_app_ui/resources/js/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ export interface ErrorInfo {
*/
export enum ErrorType {
BROKEN_THUMBNAIL = 'broken-thumbnail',
CHECK_COVER_FAILURE = 'check-cover-failed',
DEVICE_INFO_UPDATE_FAILURE = 'device-info-update-failure',
DEVICE_NOT_EXIST = 'device-not-exist',
EMPTY_FILE = 'empty-file',
Expand Down

0 comments on commit 36befd6

Please sign in to comment.