support animated GIF

ui/file_manager/gallery/js/image_editor/image_view.js

Index: ui/file_manager/gallery/js/image_editor/image_view.js
diff --git a/ui/file_manager/gallery/js/image_editor/image_view.js b/ui/file_manager/gallery/js/image_editor/image_view.js
index 5e8bffc3e88123597e85c35682cf43ef73a071ca..13e644d3e851f31e547cc0c0a4173c082e7c8c71 100644
--- a/ui/file_manager/gallery/js/image_editor/image_view.js
+++ b/ui/file_manager/gallery/js/image_editor/image_view.js
@@ -38,18 +38,11 @@ function ImageView(container, viewport, metadataModel) {
   this.contentCallbacks_ = [];
 
   /**
-   * The element displaying the current content.
-   * @type {HTMLCanvasElement}
-   * @private
-   */
-  this.screenImage_ = null;
-
-  /**
    * The content canvas element.

[hirono, 2016/01/20 06:07:09]

This is existing document error, but could you update the comment?

[ryoh, 2016/01/21 03:19:55]

Done.

    * @type {(HTMLCanvasElement|HTMLImageElement)}
    * @private
    */
-  this.contentCanvas_ = null;
+  this.contentImage_ = null;
 
   /**
    * True if the image is a preview (not full res).
@@ -179,21 +172,13 @@ ImageView.prototype.getZIndex = function() { return -1; };
  * @override
  */
 ImageView.prototype.draw = function() {
-  if (!this.contentCanvas_)  // Do nothing if the image content is not set.
+  if (!this.contentImage_)  // Do nothing if the image content is not set.
     return;
   this.setTransform_(
-      this.contentCanvas_,
+      this.contentImage_,
       this.viewport_,
       new ImageView.Effect.None(),
       ImageView.Effect.DEFAULT_DURATION);
-  if ((this.screenImage_ && this.setupDeviceBuffer(this.screenImage_)) ||
-      this.displayedContentGeneration_ !== this.contentGeneration_) {
-    this.displayedContentGeneration_ = this.contentGeneration_;
-    ImageUtil.trace.resetTimer('paint');
-    this.paintDeviceRect(
-        this.contentCanvas_, ImageRect.createFromImage(this.contentCanvas_));
-    ImageUtil.trace.reportTimer('paint');
-  }
 };
 
 /**
@@ -201,23 +186,13 @@ ImageView.prototype.draw = function() {
  * change or offset change) with animation.
  */
 ImageView.prototype.applyViewportChange = function() {
-  var zooming = this.viewport_.getZoom() > 1;
-  if (this.contentCanvas_) {
-    // Show full resolution image only for zooming.
-    this.contentCanvas_.style.opacity = zooming ? '1' : '0';
+  if (this.contentImage_) {
     this.setTransform_(
-        this.contentCanvas_,
+        this.contentImage_,
         this.viewport_,
         new ImageView.Effect.None(),
         ImageView.Effect.DEFAULT_DURATION);
   }
-  if (this.screenImage_) {
-      this.setTransform_(
-          this.screenImage_,
-          this.viewport_,
-          new ImageView.Effect.None(),
-          ImageView.Effect.DEFAULT_DURATION);
-  }
 };
 
 /**
@@ -235,15 +210,15 @@ ImageView.prototype.invalidateCaches = function() {
 };
 
 /**
- * @return {HTMLCanvasElement} The content canvas element.
+ * @return {HTMLCanvasElement|HTMLImageElement} The content image(or element).

[yawano, 2016/01/20 06:23:09]

(or canvas) element?

[ryoh, 2016/01/21 03:19:55]

Done.

  */
-ImageView.prototype.getCanvas = function() { return this.contentCanvas_; };
+ImageView.prototype.getImage = function() { return this.contentImage_; };
 
 /**
  * @return {boolean} True if the a valid image is currently loaded.
  */
 ImageView.prototype.hasValidImage = function() {
-  return !!(!this.preview_ && this.contentCanvas_ && this.contentCanvas_.width);
+  return !!(!this.preview_ && this.contentImage_ && this.contentImage_.width);
 };
 
 /**
@@ -282,7 +257,7 @@ ImageView.prototype.paintDeviceRect = function(canvas, imageRect) {
       imageRect.height * scaleY);
 
   ImageRect.drawImage(
-      this.screenImage_.getContext('2d'), canvas, deviceRect, imageRect);
+      this.contentImage_.getContext('2d'), canvas, deviceRect, imageRect);

[hirono, 2016/01/20 06:07:10]

Can we ensure this.contentImage_ is a canvas here?

[ryoh, 2016/01/21 03:19:55]

Done.

 };
 
 /**
@@ -328,11 +303,12 @@ ImageView.prototype.setupDeviceBuffer = function(canvas) {
  * @return {!ImageData} A new ImageData object.
  */
 ImageView.prototype.getScreenImageDataWith = function(width, height) {

[hirono, 2016/01/20 06:07:09]

Could you update the caller of this method so that it does not repeat scaling
large image twice?
https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/ga...

[ryoh, 2016/01/21 03:19:55]

Done.

-  // If specified size is same with current screen image size, just return it.
-  if (width === this.screenImage_.width &&
-      height === this.screenImage_.height) {
-    return this.screenImage_.getContext('2d').getImageData(
-        0, 0, this.screenImage_.width, this.screenImage_.height);
+  // If specified size is same with current screen image size
+  // and it's a canvas, just return it.
+  if (width === this.contentImage_.width &&
+      height === this.contentImage_.height &&
+      this.contentImage_.tagName === 'canvas') {
+    return this.contentImage_.getImageData(0,0,width,height);

[yawano, 2016/01/20 06:23:09]

nit: put space after ",".

[ryoh, 2016/01/21 03:19:55]

Done.

   }
 
   // Resize if these sizes are different.
@@ -341,8 +317,8 @@ ImageView.prototype.getScreenImageDataWith = function(width, height) {
   resizeCanvas.height = height;
 
   var context = resizeCanvas.getContext('2d');
-  context.drawImage(this.screenImage_,
-      0, 0, this.screenImage_.width, this.screenImage_.height,
+  context.drawImage(this.contentImage_,
+      0, 0, this.contentImage_.width, this.contentImage_.height,
       0, 0, resizeCanvas.width, resizeCanvas.height);
   return context.getImageData(0, 0, resizeCanvas.width, resizeCanvas.height);
 };
@@ -541,19 +517,8 @@ ImageView.prototype.unload = function(opt_zoomToRect) {
     clearTimeout(this.unloadTimer_);
     this.unloadTimer_ = null;
   }
-  if (opt_zoomToRect && this.screenImage_) {

[hirono, 2016/01/20 06:07:09]

I think we cannot remove this zoom out effect. Could you double check?

[ryoh, 2016/01/21 03:19:55]

Yeah, we can't remove. Thanks.

-    var effect = this.createZoomEffect(opt_zoomToRect);
-    this.setTransform_(this.screenImage_, this.viewport_, effect);
-    this.screenImage_.setAttribute('fade', true);
-    this.unloadTimer_ = setTimeout(function() {
-      this.unloadTimer_ = null;
-      this.unload(null /* force unload */);
-    }.bind(this), effect.getSafeInterval());
-    return;
-  }
   this.container_.textContent = '';
-  this.contentCanvas_ = null;
-  this.screenImage_ = null;
+  this.contentImage_ = null;
 };
 
 /**
@@ -566,38 +531,28 @@ ImageView.prototype.unload = function(opt_zoomToRect) {
 ImageView.prototype.replaceContent_ = function(
     content, opt_width, opt_height, opt_preview) {
 
-  if (this.contentCanvas_ && this.contentCanvas_.parentNode === this.container_)
-    this.container_.removeChild(this.contentCanvas_);
+  if (this.contentImage_ && this.contentImage_.parentNode === this.container_)
+    this.container_.removeChild(this.contentImage_);
 
-  this.screenImage_ = assertInstanceof(this.document_.createElement('canvas'),
-      HTMLCanvasElement);
-  this.screenImage_.className = 'image';
-
-  this.contentCanvas_ = content;
+  this.contentImage_ = content;
   this.invalidateCaches();
   this.viewport_.setImageSize(
-      opt_width || this.contentCanvas_.width,
-      opt_height || this.contentCanvas_.height);
+      opt_width || this.contentImage_.width,
+      opt_height || this.contentImage_.height);
   this.draw();
 
-  this.container_.appendChild(this.screenImage_);
-
   this.preview_ = opt_preview || false;
   // If this is not a thumbnail, cache the content and the screen-scale image.
   if (this.hasValidImage()) {
     // Insert the full resolution canvas into DOM so that it can be printed.
-    this.container_.appendChild(this.contentCanvas_);
-    this.contentCanvas_.classList.add('fullres');
+    this.container_.appendChild(this.contentImage_);
+    this.contentImage_.classList.add('image');
     this.setTransform_(
-        this.contentCanvas_, this.viewport_, null, 0);
+        this.contentImage_, this.viewport_, null, 0);

[hirono, 2016/01/20 06:07:09]

nit: It looks to fit in one line.

[ryoh, 2016/01/21 03:19:55]

Done.

 
-    this.contentItem_.contentImage = this.contentCanvas_;
-    this.contentItem_.screenImage = this.screenImage_;
+    this.contentItem_.contentImage = this.contentImage_;
 
-    // TODO(kaznacheev): It is better to pass screenImage_ as it is usually
-    // much smaller than contentCanvas_ and still contains the entire image.
-    // Once we implement zoom/pan we should pass contentCanvas_ instead.
-    this.updateThumbnail_(this.screenImage_);
+    this.updateThumbnail_(this.contentImage_);
 
     this.contentRevision_++;
     for (var i = 0; i !== this.contentCallbacks_.length; i++) {
@@ -621,7 +576,7 @@ ImageView.prototype.addContentCallback = function(callback) {
 /**
  * Updates the cached thumbnail image.
  *
- * @param {!HTMLCanvasElement} canvas The source canvas.
+ * @param {!HTMLCanvasElement|!HTMLImageElement} canvas The source canvas.

[hirono, 2016/01/20 06:07:09]

Please update 'canvas' with right name.

[ryoh, 2016/01/21 03:19:55]

Done.

  * @private
  */
 ImageView.prototype.updateThumbnail_ = function(canvas) {
@@ -640,33 +595,29 @@ ImageView.prototype.updateThumbnail_ = function(canvas) {
 /**
  * Replaces the displayed image, possibly with slide-in animation.
  *
- * @param {!(HTMLCanvasElement|HTMLImageElement)} content The image element.
+ * @param {!(HTMLCanvasElement|HTMLImageElement)} newContentImage
+ *    The image element.
  * @param {ImageView.Effect=} opt_effect Transition effect object.
  * @param {number=} opt_width Image width.
  * @param {number=} opt_height Image height.
  * @param {boolean=} opt_preview True if the image is a preview (not full res).
  */
 ImageView.prototype.replace = function(
-    content, opt_effect, opt_width, opt_height, opt_preview) {
-  var oldScreenImage = this.screenImage_;
+    newContentImage, opt_effect, opt_width, opt_height, opt_preview) {
+  var oldContentImage = this.contentImage_;
   var oldViewport = this.viewport_.clone();
-  this.replaceContent_(content, opt_width, opt_height, opt_preview);
+  this.replaceContent_(newContentImage, opt_width, opt_height, opt_preview);
   if (!opt_effect) {
-    if (oldScreenImage)
-      oldScreenImage.parentNode.removeChild(oldScreenImage);
     return;
   }
 
-  assert(this.screenImage_);
-  var newScreenImage = this.screenImage_;
+  assert(this.contentImage_);
   this.viewport_.resetView();
 
-  if (oldScreenImage)
-    ImageUtil.setAttribute(newScreenImage, 'fade', true);
-  this.setTransform_(
-      newScreenImage, this.viewport_, opt_effect, 0 /* instant */);
+  if (oldContentImage)
+    ImageUtil.setAttribute(newContentImage, 'fade', true);
   this.setTransform_(
-      content, this.viewport_, opt_effect, 0 /* instant */);
+      newContentImage, this.viewport_, opt_effect, 0 /* instant */);
 
   // We need to call requestAnimationFrame twice here. The first call is for
   // commiting the styles of beggining of transition that are assigned above.
@@ -675,28 +626,23 @@ ImageView.prototype.replace = function(
   requestAnimationFrame(function() {
     requestAnimationFrame(function() {
       this.setTransform_(
-          newScreenImage,
+          newContentImage,
           this.viewport_,
           null,
           opt_effect ? opt_effect.getDuration() : undefined);
-      this.setTransform_(
-          content,
-          this.viewport_,
-          null,
-          opt_effect ? opt_effect.getDuration() : undefined);
-      if (oldScreenImage) {
-        ImageUtil.setAttribute(newScreenImage, 'fade', false);
-        ImageUtil.setAttribute(oldScreenImage, 'fade', true);
+      if (oldContentImage) {
+        ImageUtil.setAttribute(newContentImage, 'fade', false);
+        ImageUtil.setAttribute(oldContentImage, 'fade', true);
         var reverse = opt_effect.getReverse();
         if (reverse) {
-          this.setTransform_(oldScreenImage, oldViewport, reverse);
+          this.setTransform_(oldContentImage, oldViewport, reverse);
           setTimeout(function() {
-            if (oldScreenImage.parentNode)
-              oldScreenImage.parentNode.removeChild(oldScreenImage);
+            if (oldContentImage.parentNode)
+              oldContentImage.parentNode.removeChild(oldContentImage);
           }, reverse.getSafeInterval());
         } else {
-            if (oldScreenImage.parentNode)
-              oldScreenImage.parentNode.removeChild(oldScreenImage);
+            if (oldContentImage.parentNode)
+              oldContentImage.parentNode.removeChild(oldContentImage);
         }
       }
     }.bind(this));
@@ -746,15 +692,15 @@ ImageView.prototype.createZoomEffect = function(screenRect) {
  */
 ImageView.prototype.replaceAndAnimate = function(
     canvas, imageCropRect, rotate90) {
-  assert(this.screenImage_);
+  assert(this.contentImage_);
 
   var oldImageBounds = {
     width: this.viewport_.getImageBounds().width,
     height: this.viewport_.getImageBounds().height
   };
-  var oldScreenImage = this.screenImage_;
+  var oldScreenImage = this.contentImage_;
   this.replaceContent_(canvas);
-  var newScreenImage = this.screenImage_;
+  var newScreenImage = this.contentImage_;
   var effect = rotate90 ?
       new ImageView.Effect.Rotate(rotate90 > 0) :
       new ImageView.Effect.Zoom(
@@ -762,9 +708,6 @@ ImageView.prototype.replaceAndAnimate = function(
 
   this.setTransform_(newScreenImage, this.viewport_, effect, 0 /* instant */);
 
-  oldScreenImage.parentNode.appendChild(newScreenImage);
-  oldScreenImage.parentNode.removeChild(oldScreenImage);
-
   // Let the layout fire, then animate back to non-transformed state.
   setTimeout(
       this.setTransform_.bind(
@@ -783,25 +726,18 @@ ImageView.prototype.replaceAndAnimate = function(
  * @return {number} Animation duration.
  */
 ImageView.prototype.animateAndReplace = function(canvas, imageCropRect) {
-  var oldScreenImage = this.screenImage_;
+  var oldScreenImage = this.contentImage_;
   this.replaceContent_(canvas);
-  var newScreenImage = this.screenImage_;
+  var newScreenImage = this.contentImage_;
   var setFade = ImageUtil.setAttribute.bind(null, assert(newScreenImage),
       'fade');
   setFade(true);
-  oldScreenImage.parentNode.insertBefore(newScreenImage, oldScreenImage);
   var effect = new ImageView.Effect.Zoom(
       this.viewport_.getImageBounds().width,
       this.viewport_.getImageBounds().height,
       imageCropRect);
 
-  // Animate to the transformed state.
-  this.setTransform_(oldScreenImage, this.viewport_, effect);

[hirono, 2016/01/20 06:07:09]

It looks we cannot remove this effect.

[ryoh, 2016/01/21 03:19:55]

Done.

   setTimeout(setFade.bind(null, false), 0);
-  setTimeout(function() {
-    if (oldScreenImage.parentNode)
-      oldScreenImage.parentNode.removeChild(oldScreenImage);
-  }, effect.getSafeInterval());
 
   return effect.getSafeInterval();
 };