Change getContentMimeType getContentMetadata to use FileEntry

Change the fileManagerPrivate API to use FileEntry, like all other API
in that API group. Move the Blob handling code lower down into the

  file_manager_private_custom_bindings.js

code. Since Blob reading could fail, use an errorUUID to represent the
invalid blob and make the C++ generate a chrome.runtime.lastError when
the errorUUID is seen. Attach the actual DOMError to that lastError in
the return code in the custom bindings JS (to aid debugging).

Update the apitest JS to use FileEntry instead of Blob. Update apitest
C++ to copy more testing files to the "testing" volume.

Add new files_manager test resource "empty.txt", which has no content,
to apitest the "empty Blob" case.

Bug: 1114622
Change-Id: Ie69c012ec23fac847ea19bbfcc09ad9b23c13c4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359872Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798614}

chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc

@@ -468,7 +468,7 @@ IN_PROC_BROWSER_TEST_F(FileManagerPrivateApiTest, MediaMetadata) {
   const base::FilePath test_dir = temp_dir_.GetPath();
   AddLocalFileSystem(browser()->profile(), test_dir);
 
-  // Get the source tree media/test/data directory path.
+  // Get source media/test/data directory path.
   base::FilePath root_dir;
   CHECK(base::PathService::Get(base::DIR_SOURCE_ROOT, &root_dir));
   const base::FilePath media_test_data_dir =
@@ -479,7 +479,7 @@ IN_PROC_BROWSER_TEST_F(FileManagerPrivateApiTest, MediaMetadata) {
     return media_test_data_dir.Append(base::FilePath::FromUTF8Unsafe(file));
   };
 
-  // Create test files.
+  // Create media test files.
   {
     base::ScopedAllowBlockingForTesting allow_io;
 
@@ -490,6 +490,32 @@ IN_PROC_BROWSER_TEST_F(FileManagerPrivateApiTest, MediaMetadata) {
     ASSERT_TRUE(base::CopyFile(audio, test_dir.Append(audio.BaseName())));
   }
 
+  // Get source chrome/test/data/chromeos/file_manager directory path.
+  const base::FilePath files_test_data_dir = root_dir.AppendASCII("chrome")
+                                                 .AppendASCII("test")
+                                                 .AppendASCII("data")
+                                                 .AppendASCII("chromeos")
+                                                 .AppendASCII("file_manager");
+
+  // Returns a path to a chrome/test/data/chromeos/file_manager test file.
+  auto get_files_test_data_dir = [&](const std::string& file) {
+    return files_test_data_dir.Append(base::FilePath::FromUTF8Unsafe(file));
+  };
+
+  // Create files test files.
+  {
+    base::ScopedAllowBlockingForTesting allow_io;
+
+    const base::FilePath broke = get_files_test_data_dir("broken.jpg");
+    ASSERT_TRUE(base::CopyFile(broke, test_dir.Append(broke.BaseName())));
+
+    const base::FilePath empty = get_files_test_data_dir("empty.txt");
+    ASSERT_TRUE(base::CopyFile(empty, test_dir.Append(empty.BaseName())));
+
+    const base::FilePath image = get_files_test_data_dir("image3.jpg");
+    ASSERT_TRUE(base::CopyFile(image, test_dir.Append(image.BaseName())));
+  }
+
   ASSERT_TRUE(RunComponentExtensionTest("file_browser/media_metadata"));
 }
 

chrome/browser/chromeos/extensions/file_manager/private_api_media_parser.cc

@@ -34,6 +34,10 @@ FileManagerPrivateInternalGetContentMimeTypeFunction::Run() {
   std::string blob_uuid;
   EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &blob_uuid));
 
+  if (blob_uuid.empty()) {
+    return RespondNow(Error("fileEntry.file() blob error."));
+  }
+
   content::GetUIThreadTaskRunner({})->PostTask(
       FROM_HERE,
       base::BindOnce(
@@ -83,6 +87,10 @@ FileManagerPrivateInternalGetContentMetadataFunction::Run() {
   const std::unique_ptr<Params> params(Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params);
 
+  if (params->blob_uuid.empty()) {
+    return RespondNow(Error("fileEntry.file() blob error."));
+  }
+
   bool include_images = false;
   if (params->type ==
       api::file_manager_private::CONTENT_METADATA_TYPE_METADATATAGSIMAGES) {

chrome/common/extensions/api/file_manager_private.idl

@@ -942,27 +942,27 @@ interface Functions {
   static void getFileTasks([instanceof=Entry] object[] entries,
                            GetFileTasksCallback callback);
 
-  // Gets the MIME type of a file.
-  // |entry| Entry to be checked.
+  // Gets the MIME type of an entry.
+  // |entry| The entry to be checked.
   // |callback|
   [nocompile]
   static void getMimeType([instanceof=Entry] object entry,
                           GetMimeTypeCallback callback);
 
   // Gets the content sniffed MIME type of a file.
-  // |fileBlob| Blob created from the file.
+  // |fileEntry| The file entry to be checked.
   // |callback|
   [nocompile]
-  static void getContentMimeType([instanceof=Blob] object fileBlob,
+  static void getContentMimeType([instanceof=FileEntry] object fileEntry,
                                  GetContentMimeTypeCallback callback);
 
   // Gets metadata from an Audio or Video file.
-  // |fileBlob| Blob created from the file.
+  // |fileEntry| The file entry to be checked.
   // |mimeType| Content sniffed mimeType of the file.
   // |type| Content metadata type to extact.
   // |callback|
   [nocompile]
-  static void getContentMetadata([instanceof=Blob] object fileBlob,
+  static void getContentMetadata([instanceof=FileEntry] object fileEntry,
                                  DOMString mimeType,
                                  ContentMetadataType type,
                                  GetContentMetadataCallback callback);

chrome/renderer/resources/extensions/file_manager_private_custom_bindings.js

@@ -119,36 +119,61 @@ apiBridge.registerCustomHook(function(bindingsAPI) {
   });
 
   apiFunctions.setHandleRequest('getContentMimeType',
-      function(fileBlob, callback) {
-    var uuid = blobNatives.GetBlobUuid(fileBlob);
+      function(fileEntry, callback) {
+    fileEntry.file(blob => {
+      var blobUUID = blobNatives.GetBlobUuid(blob);
 
-    if (!fileBlob || !fileBlob.size) {
-      callback(undefined);
-      return;
-    }
+      if (!blob || !blob.size) {
+        callback(undefined);
+        return;
+      }
+
+      var onGetContentMimeType = function(blob, mimeType) {
+        callback(mimeType ? mimeType : undefined);
+      }.bind(this, blob);  // Bind a blob reference: crbug.com/415792#c12
 
-    var onGetContentMimeType = function(blob, mimeType) {
-      callback(mimeType ? mimeType : undefined);
-    }.bind(this, fileBlob);  // Bind a blob reference: crbug.com/415792#c12
+      fileManagerPrivateInternal.getContentMimeType(
+          blobUUID, onGetContentMimeType);
+    }, (error) => {
+      var errorUUID = '';
 
-    fileManagerPrivateInternal.getContentMimeType(uuid, onGetContentMimeType);
+      var onGetContentMimeType = function() {
+        chrome.runtime.lastError.DOMError = /** @type {!DOMError} */ (error);
+        callback(undefined);
+      }.bind(this);
+
+      fileManagerPrivateInternal.getContentMimeType(
+          errorUUID, onGetContentMimeType);
+    });
   });
 
   apiFunctions.setHandleRequest('getContentMetadata',
-      function(fileBlob, mimeType, type, callback) {
-    var uuid = blobNatives.GetBlobUuid(fileBlob);
+      function(fileEntry, mimeType, type, callback) {
+    fileEntry.file(blob => {
+      var blobUUID = blobNatives.GetBlobUuid(blob);
 
-    if (!fileBlob || !fileBlob.size) {
-      callback(undefined);
-      return;
-    }
+      if (!blob || !blob.size) {
+        callback(undefined);
+        return;
+      }
+
+      var onGetContentMetadata = function(blob, metadata) {
+        callback(metadata ? metadata : undefined);
+      }.bind(this, blob);  // Bind a blob reference: crbug.com/415792#c12
 
-    var onGetContentMetadata = function(blob, metadata) {
-      callback(metadata ? metadata : undefined);
-    }.bind(this, fileBlob);  // Bind a blob reference: crbug.com/415792#c12
+      fileManagerPrivateInternal.getContentMetadata(
+          blobUUID, mimeType, type, onGetContentMetadata);
+    }, (error) => {
+      var errorUUID = '';
 
-    fileManagerPrivateInternal.getContentMetadata(
-        uuid, mimeType, type, onGetContentMetadata);
+      var onGetContentMetadata = function() {
+        chrome.runtime.lastError.DOMError = /** @type {!DOMError} */ (error);
+        callback(undefined);
+      }.bind(this);
+
+      fileManagerPrivateInternal.getContentMetadata(
+          errorUUID, mimeType, type, onGetContentMetadata);
+    });
   });
 
   apiFunctions.setHandleRequest('pinDriveFile', function(entry, pin, callback) {

chrome/test/data/extensions/api_test/file_browser/media_metadata/test.js

@@ -5,16 +5,19 @@
 /**
  * Test files should be created before running the tests.
  */
-let audioBlob = null;
-let videoBlob = null;
+let audioEntry = null;
+let brokeEntry = null;
+let emptyEntry = null;
+let imageEntry = null;
+let videoEntry = null;
 
 /*
- * getContentMineType of an empty blob is undefined.
+ * getContentMineType of an empty entry is undefined.
  */
 function testGetContentMimeTypeEmpty() {
-  const blob = new Blob([]);
+  const entry = emptyEntry;
 
-  chrome.fileManagerPrivate.getContentMimeType(blob, (mimeType) => {
+  chrome.fileManagerPrivate.getContentMimeType(entry, (mimeType) => {
     chrome.test.assertEq(undefined, mimeType);
     chrome.test.assertNoLastError();
     chrome.test.succeed();
@@ -25,16 +28,10 @@ function testGetContentMimeTypeEmpty() {
  * getContentMineType detects content mime types: image.
  */
 function testGetContentMimeTypeImage() {
-  const image = new Uint8Array([
-    71, 73, 70,  56,  57,  97, 1,   0, 1, 0, 128, 0,  0, 0,
-    0,  0,  255, 255, 255, 33, 249, 4, 1, 0, 0,   0,  0, 44,
-    0,  0,  0,   0,   1,   0,  1,   0, 0, 2, 1,   68, 0, 59
-  ]);
-
-  const blob = new Blob([image]);
+  const entry = imageEntry;
 
-  chrome.fileManagerPrivate.getContentMimeType(blob, (mimeType) => {
-    chrome.test.assertEq('image/gif', mimeType);
+  chrome.fileManagerPrivate.getContentMimeType(entry, (mimeType) => {
+    chrome.test.assertEq('image/jpeg', mimeType);
     chrome.test.assertNoLastError();
     chrome.test.succeed();
   });
@@ -44,9 +41,9 @@ function testGetContentMimeTypeImage() {
  * getContentMineType detects content mime types: audio.
  */
 function testGetContentMimeTypeAudio() {
-  const blob = audioBlob;
+  const entry = audioEntry;
 
-  chrome.fileManagerPrivate.getContentMimeType(blob, (mimeType) => {
+  chrome.fileManagerPrivate.getContentMimeType(entry, (mimeType) => {
     chrome.test.assertEq('audio/mpeg', mimeType);
     chrome.test.assertNoLastError();
     chrome.test.succeed();
@@ -57,9 +54,9 @@ function testGetContentMimeTypeAudio() {
  * getContentMineType detects content mime types: video.
  */
 function testGetContentMimeTypeVideo() {
-  const blob = videoBlob;
+  const entry = videoEntry;
 
-  chrome.fileManagerPrivate.getContentMimeType(blob, (mimeType) => {
+  chrome.fileManagerPrivate.getContentMimeType(entry, (mimeType) => {
     chrome.test.assertEq('video/mp4', mimeType);
     chrome.test.assertNoLastError();
     chrome.test.succeed();
@@ -71,9 +68,9 @@ function testGetContentMimeTypeVideo() {
  * chrome.runtime.lastError in that case.
  */
 function testGetContentMimeTypeUnknownMimeTypeError() {
-  const blob = new Blob([42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42]);
+  const entry = brokeEntry;
 
-  chrome.fileManagerPrivate.getContentMimeType(blob, (mimeType) => {
+  chrome.fileManagerPrivate.getContentMimeType(entry, (mimeType) => {
     chrome.test.assertEq(undefined, mimeType);
 
     if (!chrome.runtime.lastError) {
@@ -118,13 +115,13 @@ function verifyExpectedAudioMetadata(metadata) {
 }
 
 /*
- * getContentMetadata of an empty blob is undefined.
+ * getContentMetadata of an empty entry is undefined.
  */
 function testGetContentMetadataEmpty() {
-  const blob = new Blob([]);
+  const entry = emptyEntry;
 
   chrome.fileManagerPrivate.getContentMetadata(
-      blob, 'audio/mpeg', 'metadataTags', (metadata) => {
+      entry, 'audio/mpeg', 'metadataTags', (metadata) => {
     chrome.test.assertEq(undefined, metadata);
     chrome.test.assertNoLastError();
     chrome.test.succeed();
@@ -135,10 +132,10 @@ function testGetContentMetadataEmpty() {
  * getContentMetadata 'metadataTags' returns tags only.
  */
 function testGetContentMetadataAudioTags() {
-  const blob = audioBlob;
+  const entry = audioEntry;
 
   chrome.fileManagerPrivate.getContentMetadata(
-      blob, 'audio/mpeg', 'metadataTags', (metadata) => {
+      entry, 'audio/mpeg', 'metadataTags', (metadata) => {
     chrome.test.assertEq('audio/mpeg', metadata.mimeType);
     chrome.test.assertNoLastError();
 
@@ -153,10 +150,10 @@ function testGetContentMetadataAudioTags() {
  * getContentMetadata 'metadataTagsImages' returns tags and images.
  */
 function testGetContentMetadataAudioTagsImages() {
-  const blob = audioBlob;
+  const entry = audioEntry;
 
   chrome.fileManagerPrivate.getContentMetadata(
-      blob, 'audio/mpeg', 'metadataTagsImages', (metadata) => {
+      entry, 'audio/mpeg', 'metadataTagsImages', (metadata) => {
     chrome.test.assertEq('audio/mpeg', metadata.mimeType);
     chrome.test.assertNoLastError();
 
@@ -217,10 +214,10 @@ function verifyExpectedVideoMetadata(metadata) {
  * video file has no attached images.
  */
 function testGetContentMetadataVideoTagsImages() {
-  const blob = videoBlob;
+  const entry = videoEntry;
 
   chrome.fileManagerPrivate.getContentMetadata(
-      blob, 'video/mp4', 'metadataTagsImages', (metadata) => {
+      entry, 'video/mp4', 'metadataTagsImages', (metadata) => {
     chrome.test.assertEq('video/mp4', metadata.mimeType);
     chrome.test.assertNoLastError();
 
@@ -235,10 +232,10 @@ function testGetContentMetadataVideoTagsImages() {
  * getContentMetadata returns the input mime type in the metadata mime type.
  */
 function testGetContentMetadataRetainsInputMimeType() {
-  const blob = audioBlob;
+  const entry = audioEntry;
 
   chrome.fileManagerPrivate.getContentMetadata(
-      blob, 'audio/input-type', 'metadataTags', (metadata) => {
+      entry, 'audio/input-type', 'metadataTags', (metadata) => {
     chrome.test.assertEq('audio/input-type', metadata.mimeType);
     chrome.test.assertNoLastError();
     chrome.test.succeed();
@@ -250,10 +247,10 @@ function testGetContentMetadataRetainsInputMimeType() {
  * the video has width and height.
  */
 function testGetContentMetadataVideoResetsAudioMime() {
-  const blob = videoBlob;
+  const entry = videoEntry;
 
   chrome.fileManagerPrivate.getContentMetadata(
-      blob, 'audio/input-type', 'metadataTagsImages', (metadata) => {
+      entry, 'audio/input-type', 'metadataTagsImages', (metadata) => {
     chrome.test.assertEq('video/input-type', metadata.mimeType);
     chrome.test.assertNoLastError();
 
@@ -269,10 +266,10 @@ function testGetContentMetadataVideoResetsAudioMime() {
  * chrome.runtime.lastError given other mime types.
  */
 function testGetContentMetadataUnsupportedMimetypeError() {
-  const blob = new Blob([71, 73, 70, 56, 57, 97, 1, 0, 1, 0, 128, 0]);
+  const entry = imageEntry;
 
   chrome.fileManagerPrivate.getContentMetadata(
-      blob, 'image/gif', 'metadataTags', (metadata) => {
+      entry, 'image/jpeg', 'metadataTags', (metadata) => {
     chrome.test.assertEq(undefined, metadata);
 
     if (!chrome.runtime.lastError) {
@@ -308,30 +305,24 @@ function resolveTestFileSystem() {
 }
 
 /*
- * Resolves the content of |fileName| as a Blob.
+ * Resolves the fileEntry for |fileName|.
  */
-function resolveFileBlob(fileSystem, fileName) {
+function resolveFileEntry(fileSystem, fileName) {
   return new Promise((resolve) => {
     const failure = (error) => {
-      chrome.test.fail('While reading file system: ' + error);
-    };
-
-    const readEntry = (fileEntry) => {
-      fileEntry.file((file) => {
-        const reader = new FileReader();
-        reader.onerror = failure;
-        reader.onload = () => resolve(new Blob([reader.result]));
-        reader.readAsArrayBuffer(file);
-      }, failure);
+      chrome.test.fail('While resolving ' + fileName + ': ' + error);
     };
 
-    fileSystem.root.getFile(fileName, {}, readEntry, failure);
+    fileSystem.root.getFile(fileName, {}, resolve, failure);
   });
 }
 
 resolveTestFileSystem().then(async (fileSystem) => {
-  audioBlob = await resolveFileBlob(fileSystem, 'id3_png_test.mp3');
-  videoBlob = await resolveFileBlob(fileSystem, '90rotation.mp4');
+  audioEntry = await resolveFileEntry(fileSystem, 'id3_png_test.mp3');
+  brokeEntry = await resolveFileEntry(fileSystem, 'broken.jpg');
+  emptyEntry = await resolveFileEntry(fileSystem, 'empty.txt');
+  imageEntry = await resolveFileEntry(fileSystem, 'image3.jpg');
+  videoEntry = await resolveFileEntry(fileSystem, '90rotation.mp4');
 
   chrome.test.runTests([
     // fileManagerPrivate.getContentMimeType tests.

third_party/closure_compiler/externs/file_manager_private.js

@@ -622,23 +622,23 @@ chrome.fileManagerPrivate.setDefaultTask = function(taskId, entries, mimeTypes,
 chrome.fileManagerPrivate.getFileTasks = function(entries, callback) {};
 
 /**
- * Gets the MIME type of a file.
- * @param {!Entry} entry The file Entry.
+ * Gets the MIME type of an entry.
+ * @param {!Entry} entry The Entry.
  * @param {function((string|undefined))} callback The MIME type callback.
  */
 chrome.fileManagerPrivate.getMimeType = function(entry, callback) {};
 
 /**
  * Gets the content sniffed MIME type of a file.
- * @param {!Blob} fileBlob Blob created from the file Entry.
+ * @param {!FileEntry} fileEntry The file entry.
  * @param {function((string|undefined))} callback The MIME type callback.
  * $(ref:runtime.lastError) will be set if there was an error.
  */
-chrome.fileManagerPrivate.getContentMimeType = function(fileBlob, callback) {};
+chrome.fileManagerPrivate.getContentMimeType = function(fileEntry, callback) {};
 
 /**
  * Gets the content metadata from an Audio or Video file.
- * @param {!Blob} fileBlob Blob created from the file Entry.
+ * @param {!FileEntry} fileEntry The file entry.
  * @param {string} mimeType The content sniffed mimeType of the file.
  * @param {boolean} includeImages If true, return metadata tags and thumbnail
  *     images. If false, return metadata tags only.
@@ -646,7 +646,7 @@ chrome.fileManagerPrivate.getContentMimeType = function(fileBlob, callback) {};
  *     callback The content MediaMetadata callback.
  * $(ref:runtime.lastError) will be set if there was an error.
  */
-chrome.fileManagerPrivate.getContentMetadata = function(fileBlob, mimeType,
+chrome.fileManagerPrivate.getContentMetadata = function(fileEntry, mimeType,
     includeImages, callback) {};
 
 /**