Commit 20c350df authored by Noel Gordon's avatar Noel Gordon Committed by Commit Bot

[filelist] Block invalid FileList thumbnail loader requests early

ImageLoaderClient is throwing DOM exceptions while processing requests
{ImageLoaderRequest} that have no request URL. Odd, the request URL is
required (it keys the ImageLoader caches). Tracked the source of these
“no-url” requests down to the FileList thumbnail loader.

The FileList thumbnail loader sends ImageLoaderClient requests for the
file list entries while the user is scrolling the FileList. So it is a
good idea to avoid doing unnecessary work therein (it’s hot code).

Change the FileList thumbnail loader to setup the URL first and reject
early with an ImageLoaderResponse error if there is no URL to request.
Enjoy the faster scrolling, CPU and power savings, that result.

Bug: 965370
Change-Id: I5dd9a1390bc1f92010aea1b4ca2e8a50b67177df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1695661Reviewed-by: default avatarAlex Danilo <adanilo@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676225}
parent 59fcc8db
...@@ -199,15 +199,15 @@ class ThumbnailLoader { ...@@ -199,15 +199,15 @@ class ThumbnailLoader {
} }
/** /**
* Loads thumbnail as data url. If data url of thumbnail can be fetched from * Loads thumbnail as dataUrl. If the thumbnail dataUrl can be fetched from
* metadata, this fetches it from it. Otherwise, this tries to load it from * metadata, this fetches it from it. Otherwise, this tries to load it from
* thumbnail loader. * thumbnail loader.
* Compared with ThumbnailLoader.load, this method does not provide a * Compared with ThumbnailLoader.load, this method does not provide a
* functionality to fit image to a box. * functionality to fit image to a box.
* *
* @param {ThumbnailLoader.FillMode} fillMode Only FIT and OVER_FILL is * @param {ThumbnailLoader.FillMode} fillMode Only FIT and OVER_FILL are
* supported. This takes effect only when external thumbnail source is * supported. This takes effect only when an external thumbnail source
* used. * is used.
* @return {!Promise<{data:string, width:number, height:number}>} A promise * @return {!Promise<{data:string, width:number, height:number}>} A promise
* which is resolved when data url is fetched. * which is resolved when data url is fetched.
* *
...@@ -219,12 +219,26 @@ class ThumbnailLoader { ...@@ -219,12 +219,26 @@ class ThumbnailLoader {
fillMode === ThumbnailLoader.FillMode.OVER_FILL); fillMode === ThumbnailLoader.FillMode.OVER_FILL);
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
// Load by using ImageLoaderClient. let requestUrl = this.thumbnailUrl_;
if (fillMode === ThumbnailLoader.FillMode.OVER_FILL) {
// Use the croppedThumbnailUrl_ if available.
requestUrl = this.croppedThumbnailUrl_ || this.thumbnailUrl_;
}
if (!requestUrl) {
const error = LoadImageResponseStatus.ERROR;
reject(new LoadImageResponse(error, 0));
return;
}
const modificationTime = this.metadata_ && this.metadata_.filesystem && const modificationTime = this.metadata_ && this.metadata_.filesystem &&
this.metadata_.filesystem.modificationTime && this.metadata_.filesystem.modificationTime &&
this.metadata_.filesystem.modificationTime.getTime(); this.metadata_.filesystem.modificationTime.getTime();
let request = LoadImageRequest.createRequest({
url: this.thumbnailUrl_, // Load using ImageLoaderClient.
const request = LoadImageRequest.createRequest({
url: requestUrl,
maxWidth: ThumbnailLoader.THUMBNAIL_MAX_WIDTH, maxWidth: ThumbnailLoader.THUMBNAIL_MAX_WIDTH,
maxHeight: ThumbnailLoader.THUMBNAIL_MAX_HEIGHT, maxHeight: ThumbnailLoader.THUMBNAIL_MAX_HEIGHT,
cache: true, cache: true,
...@@ -234,10 +248,6 @@ class ThumbnailLoader { ...@@ -234,10 +248,6 @@ class ThumbnailLoader {
}); });
if (fillMode === ThumbnailLoader.FillMode.OVER_FILL) { if (fillMode === ThumbnailLoader.FillMode.OVER_FILL) {
// Use cropped thumbnail url if available.
request.url = this.croppedThumbnailUrl_ ? this.croppedThumbnailUrl_ :
this.thumbnailUrl_;
// Set crop option to image loader. Since image of croppedThumbnailUrl_ // Set crop option to image loader. Since image of croppedThumbnailUrl_
// is 360x360 with current implementation, it's no problem to crop it. // is 360x360 with current implementation, it's no problem to crop it.
request.width = 360; request.width = 360;
......
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