Ensure the destination url reported to ImportHistory is correctly encoded.

Eliminates a bug where files will spaces in name were not being marked as synced.

Also:
1) Remove an unused method (with a TODO body) in ImportTask.

BUG=461174
TEST=browser_test: FileManagerJsTest.*

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

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

ui/file_manager/file_manager/background/js/media_import_handler.js

@@ -400,7 +400,7 @@ importer.MediaImportHandler.ImportTask.prototype.copy_ =
    */
   var onComplete = function(destinationEntry) {
     this.cancelCallback_ = null;
-    this.markAsCopied_(entry, destinationDirectory);
+    this.markAsCopied_(entry, destinationEntry);
     this.notify(importer.TaskQueue.UpdateType.PROGRESS);
     resolver.resolve(destinationEntry);
   };
@@ -440,24 +440,13 @@ importer.MediaImportHandler.ImportTask.prototype.copy_ =
   return resolver.promise;
 };
 
-/**
- * A callback to notify listeners when a file has been copied.
- * @param {string} sourceUrl
- * @param {Entry} destination
- */
-importer.MediaImportHandler.ImportTask.prototype.onEntryChanged_ =
-    function(sourceUrl, destination) {
-  // TODO(kenobi): Add code to notify observers when entries are created.
-};
-
 /**
  * @param {!FileEntry} entry
- * @param {!DirectoryEntry} destinationDirectory
+ * @param {!FileEntry} destinationEntry
  */
 importer.MediaImportHandler.ImportTask.prototype.markAsCopied_ =
-    function(entry, destinationDirectory) {
+    function(entry, destinationEntry) {
   this.remainingFilesCount_--;
-  var destinationUrl = destinationDirectory.toURL() + '/' + entry.name;
   this.historyLoader_.getHistory().then(
       /**
        * @param {!importer.ImportHistory} history
@@ -467,7 +456,7 @@ importer.MediaImportHandler.ImportTask.prototype.markAsCopied_ =
         history.markCopied(
             entry,
             this.destination_,
-            destinationUrl);
+            destinationEntry.toURL());
       }.bind(this))
       .catch(importer.getLogger().catcher('import-task-mark-as-copied'));
 };

ui/file_manager/file_manager/background/js/media_import_handler_unittest.js

@@ -74,7 +74,7 @@ function setUp() {
 
   importHistory = new importer.TestImportHistory();
   mediaScanner = new TestMediaScanner();
-  destinationFileSystem = new MockFileSystem(destinationFactory);
+  destinationFileSystem = new MockFileSystem('googleDriveFilesystem');
   destinationFactory = Promise.resolve(destinationFileSystem.root);
   duplicateFinderFactory = new importer.TestDuplicateFinder.Factory();
 
@@ -130,6 +130,49 @@ function testImportMedia(callback) {
   scanResult.finalize();
 }
 
+function testImportMedia_EmploysEncodedUrls(callback) {
+  var media = setupFileSystem([
+    '/DCIM/photos0/Mom and Dad.jpg',
+  ]);
+
+  var scanResult = new TestScanResult(media);
+  var importTask = mediaImporter.importFromScanResult(
+        scanResult,
+        importer.Destination.GOOGLE_DRIVE,
+        destinationFactory);
+
+  var promise = new Promise(
+      function(resolve, reject) {
+        importTask.addObserver(
+            /**
+             * @param {!importer.TaskQueue.UpdateType} updateType
+             * @param {!importer.TaskQueue.Task} task
+             */
+            function(updateType, task) {
+              switch (updateType) {
+                case importer.TaskQueue.UpdateType.COMPLETE:
+                  resolve(destinationFileSystem.root.getAllChildren());
+                  break;
+                case importer.TaskQueue.UpdateType.ERROR:
+                  reject('Task failed :(');
+                  break;
+              }
+            });
+      })
+      .then(
+        function(copiedEntries) {
+          var expected = 'Mom%20and%20Dad.jpg';
+          var url = copiedEntries[0].toURL();
+          assertTrue(url.length > expected.length);
+          var actual = url.substring(url.length - expected.length);
+          assertEquals(expected, actual);
+        });
+
+  reportPromise(promise, callback);
+
+  scanResult.finalize();
+}
+
 // Verifies that when files with duplicate names are imported, that they don't
 // overwrite one another.
 function testImportMediaWithDuplicateFilenames(callback) {
@@ -390,6 +433,11 @@ function testImportWithDuplicates(callback) {
 }
 
 function testImportWithErrors(callback) {
+
+  // Quiet the logger just in this test where we expect errors.
+  // Elsewhere, it's better for errors to be seen by test authors.
+  importer.setupTestLogger().quiet();
+
   var media = setupFileSystem([
     '/DCIM/photos0/IMG00001.jpg',
     '/DCIM/photos0/IMG00002.jpg',

ui/file_manager/file_manager/common/js/mock_entry.js

@@ -109,7 +109,12 @@ MockEntry.prototype = {
  * @return {string} Fake URL.
  */
 MockEntry.prototype.toURL = function() {
-  return this.filesystem.rootURL + this.fullPath;
+  var segments = this.fullPath.split('/');
+  for (var i = 0; i < segments.length; i++) {
+    segments[i] = encodeURIComponent(segments[i]);
+  }
+
+  return this.filesystem.rootURL + segments.join('/');
 };
 
 /**

ui/file_manager/file_manager/common/js/test_importer_common.js

@@ -8,9 +8,12 @@ var importer = importer || {};
 /**
  * Sets up a logger for use in unit tests.  The test logger doesn't attempt to
  * access chrome's sync file system.  Call this during setUp.
+ * @return {!importer.TestLogger}
  */
 importer.setupTestLogger = function() {
-  importer.logger_ = new importer.TestLogger();
+  var logger = new importer.TestLogger();
+  importer.logger_ = logger;
+  return logger;
 };
 
 /**
@@ -24,18 +27,33 @@ importer.setupTestLogger = function() {
 importer.TestLogger = function() {
   /** @public {!TestCallRecorder} */
   this.errorRecorder = new TestCallRecorder();
+
+  /** @type {boolean} Should we print stuff to console */
+  this.quiet_ = false;
+};
+
+/**
+ * Causes logger to stop printing anything to console.
+ * This can be useful when errors are expected in tests.
+ */
+importer.TestLogger.prototype.quiet = function() {
+  this.quiet_ = true;
 };
 
 /** @override */
 importer.TestLogger.prototype.info = function(content) {
-  console.log(content);
+  if (!this.quiet_) {
+    console.log(content);
+  }
 };
 
 /** @override */
 importer.TestLogger.prototype.error = function(content) {
   this.errorRecorder.callback(content);
-  console.error(content);
-  console.error(new Error('Error stack').stack);
+  if (!this.quiet_) {
+    console.error(content);
+    console.error(new Error('Error stack').stack);
+  }
 };
 
 /** @override */
@@ -43,6 +61,8 @@ importer.TestLogger.prototype.catcher = function(context) {
   return function(error) {
     this.error('Caught promise error. Context: ' + context +
         ' Error: ' + error.message);
-    console.error(error.stack);
+    if (!this.quiet_) {
+      console.error(error.stack);
+    }
   }.bind(this);
 };