Gallery.app : Truncates the file after a blob of new image is created.

Previously Gallery.app truncates the file before a blob of new image is created.
This means that when some error happens during the creation of new image,
the file is overwritten with 0 bytes.
To avoid it, this CL creates the blob before truncates the file.

BUG=463837
TEST=out/Release/browser_tests --gtest_filter=GalleryJsTest.GalleryItemTest

Review URL: https://codereview.chromium.org/988273002

Cr-Commit-Position: refs/heads/master@{#319817}

ui/file_manager/gallery/js/gallery_item.js

@@ -260,38 +260,56 @@ Gallery.Item.prototype.saveToFile = function(
   };
 
   var doSave = function(newFile, fileEntry) {
-    var metadataPromise = metadataModel.get(
+    var blob;
+    var fileWriter;
+
+    metadataModel.get(
         [fileEntry],
-        ['mediaMimeType', 'contentMimeType', 'ifd', 'exifLittleEndian']);
-    metadataPromise.then(function(metadataItems) {
-      fileEntry.createWriter(function(fileWriter) {
-        var writeContent = function() {
-          fileWriter.onwriteend = onSuccess.bind(null, fileEntry);
-          var metadataItem = metadataItems[0];
-          metadataItem.modificationTime = new Date();
-          var metadataEncoder = ImageEncoder.encodeMetadata(
-              metadataItem, canvas, /* quality for thumbnail*/ 0.8);
-          // Contrary to what one might think 1.0 is not a good default. Opening
-          // and saving an typical photo taken with consumer camera increases
-          // its file size by 50-100%. Experiments show that 0.9 is much better.
-          // It shrinks some photos a bit, keeps others about the same size, but
-          // does not visibly lower the quality.
-          fileWriter.write(ImageEncoder.getBlob(canvas, metadataEncoder, 0.9));
-        }.bind(this);
-        fileWriter.onerror = function(error) {
-          onError(error);
-          // Disable all callbacks on the first error.
-          fileWriter.onerror = null;
-          fileWriter.onwriteend = null;
-        };
-        if (newFile) {
-          writeContent();
-        } else {
-          fileWriter.onwriteend = writeContent;
+        ['mediaMimeType', 'contentMimeType', 'ifd', 'exifLittleEndian']
+    ).then(function(metadataItems) {
+      // Create the blob of new image.
+      var metadataItem = metadataItems[0];
+      metadataItem.modificationTime = new Date();
+      var metadataEncoder = ImageEncoder.encodeMetadata(
+          metadataItem, canvas, /* quality for thumbnail*/ 0.8);
+      // Contrary to what one might think 1.0 is not a good default. Opening
+      // and saving an typical photo taken with consumer camera increases
+      // its file size by 50-100%. Experiments show that 0.9 is much better.
+      // It shrinks some photos a bit, keeps others about the same size, but
+      // does not visibly lower the quality.
+      blob = ImageEncoder.getBlob(canvas, metadataEncoder, 0.9);
+    }).then(function() {
+      // Create writer.
+      return new Promise(function(fullfill, reject) {
+        fileEntry.createWriter(fullfill, reject);
+      });
+    }).then(function(writer) {
+      fileWriter = writer;
+
+      // Truncates the file to 0 byte if it overwrites.
+      return new Promise(function(fulfill, reject) {
+        if (!newFile) {
+          fileWriter.onerror = reject;
+          fileWriter.onwriteend = fulfill;
           fileWriter.truncate(0);
+        } else {
+          fulfill(null);
         }
-      }.bind(this), onError);
-    }.bind(this));
+      });
+    }).then(function() {
+      // Writes the blob of new image.
+      return new Promise(function(fulfill, reject) {
+        fileWriter.onerror = reject;
+        fileWriter.onwriteend = fulfill;
+        fileWriter.write(blob);
+      });
+    }).then(onSuccess.bind(null, fileEntry))
+    .catch(function(error) {
+      onError(error);
+      // Disable all callbacks on the first error.
+      fileWriter.onerror = null;
+      fileWriter.onwriteend = null;
+    });
   }.bind(this);
 
   var getFile = function(dir, newFile) {

ui/file_manager/gallery/js/gallery_item_unittest.js

@@ -15,12 +15,10 @@ var ImageUtil = {
 };
 
 /**
- * Mock of ImageEncoder
+ * Mock of ImageEncoder. Since some test changes the behavior of ImageEncoder,
+ * this is initialized in setUp().
  */
-var ImageEncoder = {
-  encodeMetadata: function() {},
-  getBlob: function() {}
-};
+var ImageEncoder;
 
 /**
  * Load time data.
@@ -30,6 +28,30 @@ loadTimeData.data = {
   DOWNLOADS_DIRECTORY_LABEL: ''
 };
 
+function setUp() {
+  ImageEncoder = {
+    encodeMetadata: function() {},
+    getBlob: function() {}
+  };
+}
+
+/**
+ * Returns a mock of metadata model.
+ * @private
+ * @return {!MetadataModel}
+ */
+function getMockMetadataModel() {
+  return {
+    get: function(entries, names) {
+      return Promise.resolve([
+        {size: 200}
+      ]);
+    },
+    notifyEntriesChanged: function() {
+    }
+  };
+}
+
 /**
  * Tests for GalleryItem#saveToFile.
  */
@@ -50,16 +72,11 @@ function testSaveToFile(callback) {
     });
   };
   var entryChanged = false;
-  var metadataModel = {
-    get: function(entries, names) {
-      return Promise.resolve([
-        {size: 200}
-      ]);
-    },
-    notifyEntriesChanged: function() {
-      entryChanged = true;
-    }
+  var metadataModel = getMockMetadataModel();
+  metadataModel.notifyEntriesChanged = function() {
+    entryChanged = true;
   };
+
   var item = new Gallery.Item(
       entry,
       {isReadOnly: false},
@@ -80,3 +97,96 @@ function testSaveToFile(callback) {
             assertTrue(entryChanged);
           }), callback);
 }
+
+/**
+ * Tests for GalleryItem#saveToFile. In this test case, fileWriter.write fails
+ * with an error.
+ */
+function testSaveToFileWriteFailCase(callback) {
+  var fileSystem = new MockFileSystem('volumeId');
+  fileSystem.populate(['/test.jpg']);
+  var entry = fileSystem.entries['/test.jpg'];
+
+  entry.createWriter = function(callback) {
+    callback({
+      write: function() {
+        Promise.resolve().then(function() {
+          this.onerror(new Error());
+        }.bind(this));
+      },
+      truncate: function() {
+        Promise.resolve().then(function() {
+          this.onwriteend();
+        }.bind(this));
+      }
+    });
+  };
+
+  var item = new Gallery.Item(
+      entry,
+      {isReadOnly: false},
+      {size: 100},
+      {},
+      /* original */ true);
+  reportPromise(
+      new Promise(item.saveToFile.bind(
+          item,
+          {getLocationInfo: function() { return {}; }},
+          getMockMetadataModel(),
+          /* fallbackDir */ null,
+          /* overwrite */ true,
+          document.createElement('canvas'))).then(function(result) {
+            assertFalse(result);
+          }), callback);
+}
+
+/**
+ * Tests for GalleryItem#saveToFile. In this test case, ImageEncoder.getBlob
+ * fails with an error. This test case confirms that no write operation runs
+ * when it fails to get a blob of new image.
+ */
+function testSaveToFileGetBlobFailCase(callback) {
+  ImageEncoder.getBlob = function() {
+    throw new Error();
+  };
+
+  var fileSystem = new MockFileSystem('volumeId');
+  fileSystem.populate(['/test.jpg']);
+  var entry = fileSystem.entries['/test.jpg'];
+
+  var writeOperationRun = false;
+  entry.createWriter = function(callback) {
+    callback({
+      write: function() {
+        Promise.resolve().then(function() {
+          writeOperationRun = true;
+          this.onwriteend();
+        }.bind(this));
+      },
+      truncate: function() {
+        Promise.resolve().then(function() {
+          writeOperationRun = true;
+          this.onwriteend();
+        }.bind(this));
+      }
+    });
+  };
+
+  var item = new Gallery.Item(
+      entry,
+      {isReadOnly: false},
+      {size: 100},
+      {},
+      /* original */ true);
+  reportPromise(
+      new Promise(item.saveToFile.bind(
+          item,
+          {getLocationInfo: function() { return {}; }},
+          getMockMetadataModel(),
+          /* fallbackDir */ null,
+          /* overwrite */ true,
+          document.createElement('canvas'))).then(function(result) {
+            assertFalse(result);
+            assertFalse(writeOperationRun);
+          }), callback);
+}