CrOS Files App: Remove ImageLoaderClient::tasks_, opt_isValid and some always-undefined args.

ImageLoaderClient::tasks_ is effectively unused since r316185. All it
does now is cause a big JS memory leak whenever a thumbnail is loaded.

This obsoletes ThumbnailLoader.OptimizationMode. Only the Gallery app's
"ribbon" uses this, and it just passed the default arg. It is the only
caller of ThumbnailLoader.load(), so there's no need to have any optional
arguments.

Bug: 881680
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ic1e9a7affee8e81ed3ba4b235b956dd2c3c71d13
Reviewed-on: https://chromium-review.googlesource.com/1214966Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589849}

ui/file_manager/file_manager/foreground/js/thumbnail_loader.js

@@ -118,15 +118,6 @@ ThumbnailLoader.FillMode = {
   AUTO: 3   // Try to fill, but if incompatible aspect ratio, then fit.
 };
 
-/**
- * Optimization mode for downloading thumbnails.
- * @enum {number}
- */
-ThumbnailLoader.OptimizationMode = {
-  NEVER_DISCARD: 0,    // Never discards downloading. No optimization.
-  DISCARD_DETACHED: 1  // Canceled if the container is not attached anymore.
-};
-
 /**
  * Type of element to store the image.
  * @enum {number}
@@ -174,29 +165,18 @@ ThumbnailLoader.prototype.getLoadTarget = function() {
 /**
  * Loads and attaches an image.
  *
- * @param {Element} box Container element.
+ * @param {!Element} box Container element.
  * @param {ThumbnailLoader.FillMode} fillMode Fill mode.
- * @param {ThumbnailLoader.OptimizationMode=} opt_optimizationMode Optimization
- *     for downloading thumbnails. By default optimizations are disabled.
- * @param {function(Image)=} opt_onSuccess Success callback, accepts the image.
- * @param {function()=} opt_onError Error callback.
- * @param {function()=} opt_onGeneric Callback for generic image used.
- * @param {number=} opt_autoFillThreshold Auto fill threshold.
- * @param {number=} opt_boxWidth Container box's width. If not specified, the
- *     given |box|'s clientWidth will be used.
- * @param {number=} opt_boxHeight Container box's height. If not specified, the
- *     given |box|'s clientHeight will be used.
+ * @param {function(Image)} onSuccess Success callback, accepts the image.
+ * @param {number} autoFillThreshold Auto fill threshold.
+ * @param {number} boxWidth Container box's width.
+ * @param {number} boxHeight Container box's height.
  */
-ThumbnailLoader.prototype.load = function(box, fillMode, opt_optimizationMode,
-    opt_onSuccess, opt_onError, opt_onGeneric, opt_autoFillThreshold,
-    opt_boxWidth, opt_boxHeight) {
-  opt_optimizationMode = opt_optimizationMode ||
-      ThumbnailLoader.OptimizationMode.NEVER_DISCARD;
-
+ThumbnailLoader.prototype.load = function(
+    box, fillMode, onSuccess, autoFillThreshold, boxWidth, boxHeight) {
   if (!this.thumbnailUrl_) {
     // Relevant CSS rules are in file_types.css.
     box.setAttribute('generic-thumbnail', this.mediaType_);
-    if (opt_onGeneric) opt_onGeneric();
     return;
   }
 
@@ -205,18 +185,17 @@ ThumbnailLoader.prototype.load = function(box, fillMode, opt_optimizationMode,
   this.image_ = new Image();
   this.image_.setAttribute('alt', this.entry_.name);
   this.image_.onload = function() {
-    this.attachImage(assert(box), fillMode, opt_autoFillThreshold,
-                     opt_boxWidth, opt_boxHeight);
-    if (opt_onSuccess)
-      opt_onSuccess(this.image_);
+    this.attachImage_(box, fillMode, autoFillThreshold, boxWidth, boxHeight);
+    onSuccess(this.image_);
   }.bind(this);
   this.image_.onerror = function() {
-    if (opt_onError)
-      opt_onError();
     if (this.fallbackUrl_) {
       this.thumbnailUrl_ = this.fallbackUrl_;
       this.fallbackUrl_ = null;
-      this.load(box, fillMode, opt_optimizationMode, opt_onSuccess);
+      this.load(
+          box, fillMode, onSuccess,
+          ThumbnailLoader.AUTO_FILL_THRESHOLD_DEFAULT_VALUE, box.clientWidth,
+          box.clientHeight);
     } else {
       box.setAttribute('generic-thumbnail', this.mediaType_);
     }
@@ -234,8 +213,7 @@ ThumbnailLoader.prototype.load = function(box, fillMode, opt_optimizationMode,
                          this.metadata_.filesystem.modificationTime &&
                          this.metadata_.filesystem.modificationTime.getTime();
   this.taskId_ = ImageLoaderClient.loadToImage(
-      this.thumbnailUrl_,
-      this.image_,
+      this.thumbnailUrl_, this.image_,
       {
         maxWidth: ThumbnailLoader.THUMBNAIL_MAX_WIDTH,
         maxHeight: ThumbnailLoader.THUMBNAIL_MAX_HEIGHT,
@@ -247,16 +225,7 @@ ThumbnailLoader.prototype.load = function(box, fillMode, opt_optimizationMode,
       function() {},
       function() {
         this.image_.onerror(new Event('load-error'));
-      }.bind(this),
-      function() {
-        if (opt_optimizationMode ==
-            ThumbnailLoader.OptimizationMode.DISCARD_DETACHED &&
-            !box.ownerDocument.contains(box)) {
-          // If the container is not attached, then invalidate the download.
-          return false;
-        }
-        return true;
-      });
+      }.bind(this));
 };
 
 /**
@@ -417,15 +386,14 @@ ThumbnailLoader.prototype.renderMedia_ = function() {
  * Attach the image to a given element.
  * @param {!Element} box Container element.
  * @param {ThumbnailLoader.FillMode} fillMode Fill mode.
- * @param {number=} opt_autoFillThreshold Threshold value which is used for fill
+ * @param {number} autoFillThreshold Threshold value which is used for fill
  *     mode auto.
- * @param {number=} opt_boxWidth Container box's width. If not specified, the
- *     given |box|'s clientWidth will be used.
- * @param {number=} opt_boxHeight Container box's height. If not specified, the
- *     given |box|'s clientHeight will be used.
+ * @param {number} boxWidth Container box's width.
+ * @param {number} boxHeight Container box's height.
+ * @private
  */
-ThumbnailLoader.prototype.attachImage = function(
-    box, fillMode, opt_autoFillThreshold, opt_boxWidth, opt_boxHeight) {
+ThumbnailLoader.prototype.attachImage_ = function(
+    box, fillMode, autoFillThreshold, boxWidth, boxHeight) {
   if (!this.hasValidImage()) {
     box.setAttribute('generic-thumbnail', this.mediaType_);
     return;
@@ -435,10 +403,8 @@ ThumbnailLoader.prototype.attachImage = function(
   var attachableMedia = this.loaderType_ === ThumbnailLoader.LoaderType.CANVAS ?
       this.canvas_ : this.image_;
 
-  var autoFillThreshold = opt_autoFillThreshold ||
-      ThumbnailLoader.AUTO_FILL_THRESHOLD_DEFAULT_VALUE;
-  ThumbnailLoader.centerImage_(box, attachableMedia, fillMode,
-      autoFillThreshold, opt_boxWidth, opt_boxHeight);
+  ThumbnailLoader.centerImage_(
+      box, attachableMedia, fillMode, autoFillThreshold, boxWidth, boxHeight);
 
   if (attachableMedia.parentNode !== box) {
     box.textContent = '';
@@ -472,24 +438,18 @@ ThumbnailLoader.prototype.getImage = function() {
  * @param {ThumbnailLoader.FillMode} fillMode Fill mode.
  * @param {number} autoFillThreshold Threshold value which is used for fill mode
  *     auto.
- * @param {number=} opt_boxWidth Container box's width. If not specified, the
- *     given |box|'s clientWidth will be used.
- * @param {number=} opt_boxHeight Container box's height. If not specified, the
- *     given |box|'s clientHeight will be used.
+ * @param {number} boxWidth Container box's width.
+ * @param {number} boxHeight Container box's height.
  * @private
  */
 ThumbnailLoader.centerImage_ = function(
-    box, img, fillMode, autoFillThreshold, opt_boxWidth,
-    opt_boxHeight) {
+    box, img, fillMode, autoFillThreshold, boxWidth, boxHeight) {
   var imageWidth = img.width;
   var imageHeight = img.height;
 
   var fractionX;
   var fractionY;
 
-  var boxWidth = opt_boxWidth || box.clientWidth;
-  var boxHeight = opt_boxHeight || box.clientHeight;
-
   var fill;
   switch (fillMode) {
     case ThumbnailLoader.FillMode.FILL:

ui/file_manager/gallery/js/ribbon.js

@@ -467,23 +467,17 @@ Ribbon.prototype.setThumbnailImage_ = function(thumbnail, item) {
   };
 
   this.thumbnailModel_.get([item.getEntry()]).then(function(metadataList) {
-    var loader = new ThumbnailLoader(
-        item.getEntry(),
-        ThumbnailLoader.LoaderType.IMAGE,
-        metadataList[0]);
+    const loader = new ThumbnailLoader(
+        item.getEntry(), ThumbnailLoader.LoaderType.IMAGE, metadataList[0]);
+    const box =
+        assertInstanceof(thumbnail.querySelector('.image-wrapper'), Element);
     // Pass 0.35 as auto fill threshold. This value allows to fill 4:3 and 3:2
     // photos in 16:9 box (ratio factors for them are ~1.34 and ~1.18
     // respectively).
     loader.load(
-        thumbnail.querySelector('.image-wrapper'),
-        ThumbnailLoader.FillMode.AUTO,
-        ThumbnailLoader.OptimizationMode.NEVER_DISCARD,
-        hideIndicator /* opt_onSuccess */,
-        undefined /* opt_onError */,
-        undefined /* opt_onGeneric */,
-        0.35 /* opt_autoFillThreshold */,
-        Ribbon.THUMBNAIL_WIDTH /* opt_boxWidth */,
-        Ribbon.THUMBNAIL_HEIGHT /* opt_boxHeight */);
+        box, ThumbnailLoader.FillMode.AUTO, hideIndicator /* onSuccess */,
+        0.35 /* autoFillThreshold */, Ribbon.THUMBNAIL_WIDTH /* boxWidth */,
+        Ribbon.THUMBNAIL_HEIGHT /* boxHeight */);
   });
 };
 

ui/file_manager/image_loader/image_loader_client.js

@@ -13,13 +13,6 @@
  * @constructor
  */
 function ImageLoaderClient() {
-  /**
-   * Hash array with active tasks.
-   * @type {!Object}
-   * @private
-   */
-  this.tasks_ = {};
-
   /**
    * @type {number}
    * @private
@@ -91,31 +84,7 @@ ImageLoaderClient.sendMessage_ = function(request, opt_callback) {
 };
 
 /**
- * Handles a message from the remote image loader and calls the registered
- * callback to pass the response back to the requester.
- *
- * @param {Object} message Response message as a hash array.
- * @private
- */
-ImageLoaderClient.prototype.handleMessage_ = function(message) {
-  if (!(message.taskId in this.tasks_)) {
-    // This task has been canceled, but was already fetched, so it's result
-    // should be discarded anyway.
-    return;
-  }
-
-  var task = this.tasks_[message.taskId];
-
-  // Check if the task is still valid.
-  if (task.isValid())
-    task.accept(message);
-
-  delete this.tasks_[message.taskId];
-};
-
-/**
- * Loads and resizes and image. Use opt_isValid to easily cancel requests
- * which are not valid anymore, which will reduce cpu consumption.
+ * Loads and resizes and image.
  *
  * @param {string} url Url of the requested image.
  * @param {function({status: string, data:string, width:number, height:number})}
@@ -124,31 +93,15 @@ ImageLoaderClient.prototype.handleMessage_ = function(message) {
  *     these values are resized width and height.
  * @param {Object=} opt_options Loader options, such as: scale, maxHeight,
  *     width, height and/or cache.
- * @param {function(): boolean=} opt_isValid Function returning false in case
- *     a request is not valid anymore, eg. parent node has been detached.
  * @return {?number} Remote task id or null if loaded from cache.
  */
-ImageLoaderClient.prototype.load = function(
-    url, callback, opt_options, opt_isValid) {
+ImageLoaderClient.prototype.load = function(url, callback, opt_options) {
   opt_options = /** @type {{cache: (boolean|undefined)}} */(opt_options || {});
-  opt_isValid = opt_isValid || function() { return true; };
 
   // Record cache usage.
   ImageLoaderClient.recordPercentage('Cache.Usage',
       this.cache_.size() / ImageLoaderClient.CACHE_MEMORY_LIMIT * 100.0);
 
-  // Cancel old, invalid tasks.
-  var taskKeys = Object.keys(this.tasks_);
-  for (var index = 0; index < taskKeys.length; index++) {
-    var taskKey = taskKeys[index];
-    var task = this.tasks_[taskKey];
-    if (!task.isValid()) {
-      // Cancel this task since it is not valid anymore.
-      this.cancel(parseInt(taskKey, 10));
-      delete this.tasks_[taskKey];
-    }
-  }
-
   // Replace the extension id.
   var sourceId = chrome.i18n.getMessage('@@extension_id');
   var targetId = ImageLoaderClient.EXTENSION_ID;
@@ -189,8 +142,6 @@ ImageLoaderClient.prototype.load = function(
   // Not available in cache, performing a request to a remote extension.
   var request = opt_options;
   this.lastTaskId_++;
-  var task = {isValid: opt_isValid};
-  this.tasks_[this.lastTaskId_] = task;
 
   request.url = url;
   request.taskId = this.lastTaskId_;
@@ -253,8 +204,7 @@ ImageLoaderClient.createKey = function(url, opt_options) {
 // Helper functions.
 
 /**
- * Loads and resizes and image. Use opt_isValid to easily cancel requests
- * which are not valid anymore, which will reduce cpu consumption.
+ * Loads and resizes and image.
  *
  * @param {string} url Url of the requested image.
  * @param {HTMLImageElement} image Image node to load the requested picture
@@ -263,12 +213,10 @@ ImageLoaderClient.createKey = function(url, opt_options) {
  *     maxHeight, width, height and/or cache.
  * @param {function()} onSuccess Callback for success.
  * @param {function()} onError Callback for failure.
- * @param {function(): boolean=} opt_isValid Function returning false in case
- *     a request is not valid anymore, eg. parent node has been detached.
  * @return {?number} Remote task id or null if loaded from cache.
  */
 ImageLoaderClient.loadToImage = function(
-    url, image, options, onSuccess, onError, opt_isValid) {
+    url, image, options, onSuccess, onError) {
   var callback = function(result) {
     if (result.status == 'error') {
       onError();
@@ -278,6 +226,5 @@ ImageLoaderClient.loadToImage = function(
     onSuccess();
   };
 
-  return ImageLoaderClient.getInstance().load(
-      url, callback, options, opt_isValid);
+  return ImageLoaderClient.getInstance().load(url, callback, options);
 };