1) Add a drop down revealing a popup panel with import details (with is a WIP,...

1) Add a drop down revealing a popup panel with import details (with is a WIP, but the current state should be sufficient to land).

2) Update scanner to publish updates on all scan activity (so the details reflects valid information while scanning). In doing this I pulled all of the deduping logic up out of ScanResult into MediaScanner...improving separation of concern between the classes.

3) Rework update model to eliminate a flicker resulting from the brief update to show zero results when a file selection is transitioning from n to n+/-1.

Sadly, this change results in a net loss of readability, especially WRT having good clear decision points. I've added comments to help out in this respect.

FYI, I'm OOO for the rest of the day. I think this CL is good to land, rough though it may be.

In a followup CL:

1) I'll make some subsequent improvements to unittests in the wake of removing the "GetCommandUpdate" coverage (removed because GetCommandUpdate exists no more).

2) This change still exhibits one odd behavior, ignoring entirely a selection when it consistes of all deduped files...in that case we *do* want to see zero results. Will address at the time of test improvements.

BUG=420680
TEST=browser_test: FileManagerJsTest.*

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

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

ui/file_manager/file_manager/background/js/mock_media_scanner.js

@@ -47,11 +47,11 @@ TestMediaScanner.prototype.removeObserver = function(observer) {
 
 /** @override */
 TestMediaScanner.prototype.scan = function(entries) {
-  var result = new TestScanResult(this.fileEntries);
-  result.totalBytes = this.totalBytes;
-  result.scanDuration = this.scanDuration;
-  this.scans_.push(result);
-  return result;
+  var scan = new TestScanResult(this.fileEntries);
+  scan.totalBytes = this.totalBytes;
+  scan.scanDuration = this.scanDuration;
+  this.scans_.push(scan);
+  return scan;
 };
 
 /**
@@ -61,6 +61,19 @@ TestMediaScanner.prototype.finalizeScans = function() {
   this.scans_.forEach(this.finalize.bind(this));
 };
 
+/**
+ * Notifies observers that the most recently started scan has been updated.
+ * @param {!importer.ScanResult} result
+ */
+TestMediaScanner.prototype.update = function() {
+  assertTrue(this.scans_.length > 0);
+  var scan = this.scans_[this.scans_.length - 1];
+  this.observers.forEach(
+      function(observer) {
+        observer(importer.ScanEvent.UPDATED, scan);
+      });
+};
+
 /**
  * Notifies observers that a scan has finished.
  * @param {!importer.ScanResult} result

ui/file_manager/file_manager/common/js/importer_common.js

@@ -8,7 +8,8 @@ var importer = importer || {};
 /** @enum {string} */
 importer.ScanEvent = {
   FINALIZED: 'finalized',
-  INVALIDATED: 'invalidated'
+  INVALIDATED: 'invalidated',
+  UPDATED: 'updated'
 };
 
 /**

ui/file_manager/file_manager/foreground/css/file_manager.css

@@ -343,6 +343,33 @@ body.selecting .dialog-header {
   margin: 8px;
 }
 
+#cloud-import-details-button {
+  margin-left: -15px;
+}
+
+#cloud-import-details {
+  background-color: white;
+  height: 300px;
+  opacity: .85;
+  padding: 10px;
+  position: absolute;
+  right: 0px;
+  top: 49px;
+  transition: all 0.2s ease;
+  width: 250px;
+  z-index: 5;
+}
+
+#cloud-import-details.offscreen {
+  opacity: 0;
+  right: -202px;
+}
+
+#cloud-import-details div {
+  border: 0px 0px 1px 0px solid gray;
+  padding: 10px;
+};
+
 #files-selected-label {
   display: none;
   /* TODO(fukino): Move dynamically with the navigation area's separator. */

ui/file_manager/file_manager/foreground/js/import_controller_unittest.js

@@ -57,227 +57,6 @@ function setUp() {
   mediaImporter = new TestImportRunner();
 }
 
-function testGetCommandUpdate_HiddenWhenDriveUnmounted(callback) {
-  var controller = createController(
-      VolumeManagerCommon.VolumeType.MTP,
-      'mtp-volume',
-      [
-        '/DCIM/',
-        '/DCIM/photos0/',
-        '/DCIM/photos1/IMG00001.jpg'
-      ],
-      '/DCIM');
-
-  environment.isDriveMounted = false;
-  var promise = controller.getCommandUpdate().then(
-      function(response) {
-        assertFalse(response.visible);
-        assertFalse(response.executable);
-
-        mediaScanner.assertScanCount(0);
-      });
-
-  reportPromise(promise, callback);
-}
-
-function testGetCommandUpdate_HiddenForNonMediaVolume(callback) {
-  var controller = createController(
-      VolumeManagerCommon.VolumeType.MTP,
-      'drive',
-      [
-        '/DCIM/',
-        '/DCIM/photos0/',
-        '/DCIM/photos0/IMG00001.jpg'
-      ],
-      '/DCIM');
-
-  environment.isDriveMounted = false;
-
-  var promise = controller.getCommandUpdate().then(
-      function(response) {
-        assertFalse(response.visible);
-        assertFalse(response.executable);
-
-        mediaScanner.assertScanCount(0);
-      });
-
-  reportPromise(promise, callback);
-}
-
-function testGetCommandUpdate_InitiatesScan(callback) {
-  var controller = createController(
-      VolumeManagerCommon.VolumeType.MTP,
-      'mtp-volume',
-      [
-        '/DCIM/',
-        '/DCIM/photos0/',
-        '/DCIM/photos0/IMG00001.jpg',
-        '/DCIM/photos0/IMG00002.jpg',
-        '/DCIM/photos1/',
-        '/DCIM/photos1/IMG00001.jpg',
-        '/DCIM/photos1/IMG00003.jpg'
-      ],
-      '/DCIM');
-
-  var promise = controller.getCommandUpdate().then(
-      function(response) {
-        assertTrue(response.visible);
-        assertFalse(response.executable);
-        assertEquals(
-            response.label,
-            MESSAGES.CLOUD_IMPORT_SCANNING_BUTTON_LABEL);
-        mediaScanner.assertScanCount(1);
-      });
-
-  reportPromise(promise, callback);
-}
-
-function testGetCommandUpdate_CanExecuteAfterScanIsFinalized(callback) {
-  var controller = createController(
-      VolumeManagerCommon.VolumeType.MTP,
-      'mtp-volume',
-      [
-        '/DCIM/',
-        '/DCIM/photos0/',
-        '/DCIM/photos0/IMG00001.jpg',
-        '/DCIM/photos0/IMG00002.jpg',
-        '/DCIM/photos1/',
-        '/DCIM/photos1/IMG00001.jpg',
-        '/DCIM/photos1/IMG00003.jpg'
-      ],
-      '/DCIM');
-
-  var fileSystem = new MockFileSystem('testFs');
-  mediaScanner.fileEntries.push(
-      new MockFileEntry(fileSystem, '/DCIM/photos0/IMG00001.jpg', {size: 0}));
-
-  environment.directoryChangedListener_();  // initiates a scan.
-  var promise = widget.updatePromise.then(
-      function() {
-        widget.resetPromise();
-        mediaScanner.finalizeScans();
-        return widget.updatePromise;
-      }).then(
-          function(response) {
-            assertTrue(response.visible);
-            assertTrue(response.executable);
-            assertEquals(
-                response.label,
-                MESSAGES.CLOUD_IMPORT_BUTTON_LABEL);
-          });
-
-  reportPromise(promise, callback);
-}
-
-function testGetCommandUpdate_DisabledForInsufficientLocalStorage(callback) {
-  var controller = createController(
-      VolumeManagerCommon.VolumeType.MTP,
-      'mtp-volume',
-      [
-        '/DCIM/',
-        '/DCIM/photos0/',
-        '/DCIM/photos0/IMG00001.jpg',
-        '/DCIM/photos0/IMG00002.jpg',
-        '/DCIM/photos1/',
-        '/DCIM/photos1/IMG00001.jpg',
-        '/DCIM/photos1/IMG00003.jpg'
-      ],
-      '/DCIM');
-
-  var fileSystem = new MockFileSystem('testFs');
-  mediaScanner.fileEntries.push(
-      new MockFileEntry(
-          fileSystem,
-          '/DCIM/photos0/IMG00001.jpg',
-          {size: 1000000}));
-
-  environment.freeStorageSpace = 100;
-  environment.directoryChangedListener_();  // initiates a scan.
-  var promise = widget.updatePromise.then(
-      function() {
-        widget.resetPromise();
-        mediaScanner.finalizeScans();
-        return widget.updatePromise;
-      }).then(
-          function(response) {
-            assertTrue(response.visible);
-            assertFalse(response.executable);
-            assertEquals(
-                response.label,
-                MESSAGES.CLOUD_IMPORT_INSUFFICIENT_SPACE_BUTTON_LABEL);
-          });
-
-  reportPromise(promise, callback);
-}
-
-function testGetCommandUpdate_CannotExecuteEmptyScanResult(callback) {
-  var controller = createController(
-      VolumeManagerCommon.VolumeType.MTP,
-      'mtp-volume',
-      [
-        '/DCIM/',
-        '/DCIM/photos0/',
-        '/DCIM/photos0/IMG00001.trader',
-        '/DCIM/photos0/IMG00002.joes',
-        '/DCIM/photos1/',
-        '/DCIM/photos1/IMG00001.parking',
-        '/DCIM/photos1/IMG00003.lots'
-      ],
-      '/DCIM');
-
-  var promise = controller.getCommandUpdate().then(
-      function() {
-        mediaScanner.finalizeScans();
-
-        return controller.getCommandUpdate().then(
-            function(response) {
-              assertTrue(response.visible);
-              assertFalse(response.executable);
-              assertEquals(
-                  response.label,
-                  MESSAGES.CLOUD_IMPORT_EMPTY_SCAN_BUTTON_LABEL);
-            });
-      });
-
-  reportPromise(promise, callback);
-}
-
-function testGetCommandUpdate_DisabledWhileImporting(callback) {
-  var controller = createController(
-      VolumeManagerCommon.VolumeType.MTP,
-      'mtp-volume',
-      [
-        '/DCIM/',
-        '/DCIM/photos0/',
-        '/DCIM/photos0/IMG00001.jpg',
-        '/DCIM/photos0/IMG00002.jpg',
-      ],
-      '/DCIM');
-
-// First we need to force the controller into a scanning state.
-environment.directoryChangedListener_();
-
-var promise = widget.updatePromise.then(
-    function() {
-      widget.resetPromise();
-      widget.executeListener();
-      mediaImporter.assertImportsStarted(1);
-      // return the reset promise so as to allow execution
-      // to complete before the test is finished...even though
-      // we're not waiting on anything in particular.
-      return controller.getCommandUpdate();
-    }).then(
-        function(response) {
-          assertTrue(response.visible);
-          assertFalse(response.executable);
-          assertEquals(
-              response.label,
-              MESSAGES.CLOUD_IMPORT_ACTIVE_IMPORT_BUTTON_LABEL);
-        });
-
-  reportPromise(promise, callback);
-}
-
 function testClick_StartsImport(callback) {
   var controller = createController(
       VolumeManagerCommon.VolumeType.MTP,
@@ -302,7 +81,6 @@ function testClick_StartsImport(callback) {
             // return the reset promise so as to allow execution
             // to complete before the test is finished...even though
             // we're not waiting on anything in particular.
-            return widget.updatePromise;
           }),
       callback);
 }
@@ -332,6 +110,7 @@ function testVolumeUnmount_InvalidatesScans(callback) {
             // A fresh new scan should be started.
             environment.simulateUnmount();
 
+            environment.directoryChangedListener_();
             // Return the new promise, so subsequent "thens" only
             // fire once the widget has been updated again.
             return widget.updatePromise;
@@ -373,23 +152,6 @@ function testSelectionChange_TriggersUpdate(callback) {
   reportPromise(widget.updatePromise, callback);
 }
 
-function testVolumeUnmount_TriggersUpdate(callback) {
-  var controller = createController(
-      VolumeManagerCommon.VolumeType.MTP,
-      'mtp-volume',
-      [
-        '/DCIM/',
-        '/DCIM/photos0/',
-        '/DCIM/photos0/IMG00001.jpg',
-      ],
-      '/DCIM');
-
-  // Faux unmount the volume, then request an update again.
-  // A fresh new scan should be started.
-  environment.simulateUnmount();
-  reportPromise(widget.updatePromise, callback);
-}
-
 function testFinalizeScans_TriggersUpdate(callback) {
   var controller = createController(
       VolumeManagerCommon.VolumeType.MTP,
@@ -467,7 +229,11 @@ function testFinishImport_TriggersUpdate(callback) {
       }).then(
           function() {
             widget.resetPromise();
+            // This will invalidate scans...and trigger a state check...
+            // itself triggering a scan.
             mediaImporter.finishImportTasks();
+            // Trigger the update needed to cause a UI update.
+            mediaScanner.update();
             return widget.updatePromise;
           });
 
@@ -501,6 +267,8 @@ function testCancelImport_TriggersUpdate(callback) {
           function() {
             widget.resetPromise();
             mediaImporter.cancelImportTasks();
+            // Trigger the update needed to cause a UI update.
+            mediaScanner.update();
             return widget.updatePromise;
           });
 
@@ -729,7 +497,8 @@ importer.TestCommandWidget.prototype.resetPromise = function() {
 };
 
 /** @override */
-importer.TestCommandWidget.prototype.addExecuteListener = function(listener) {
+importer.TestCommandWidget.prototype.addImportClickedListener =
+    function(listener) {
   this.executeListener = listener;
 };
 
@@ -739,6 +508,27 @@ importer.TestCommandWidget.prototype.update = function(update) {
   this.updateResolver_.resolve(update);
 };
 
+/** @override */
+importer.TestCommandWidget.prototype.updateDetails = function(scan) {
+  // TODO(smckay)
+};
+
+/** @override */
+importer.TestCommandWidget.prototype.resetDetails = function() {
+  // TODO(smckay)
+};
+
+/** @override */
+importer.TestCommandWidget.prototype.setDetailsVisible = function(visible) {
+  // TODO(smckay)
+};
+
+/** @override */
+importer.TestCommandWidget.prototype.isDetailsVisible = function() {
+  // TODO(smckay)
+};
+
+
 /**
  * @param {!VolumeManagerCommon.VolumeType} volumeType
  * @param {string} volumeId

ui/file_manager/file_manager/main.html

@@ -346,11 +346,16 @@
         <core-icon icon="cloud-queue"></core-icon>
         <paper-ripple fit></paper-ripple>
       </button>
-      <button id="view-button" class="icon-button" tabindex="12">
+      <button id="cloud-import-details-button" class="icon-button" tabindex="12"
+              style='display: none;'>
+        <core-icon icon="arrow-drop-down"></core-icon>
+        <paper-ripple fit></paper-ripple>
+      </button>
+      <button id="view-button" class="icon-button" tabindex="13">
         <core-icon icon="view-module"></core-icon>
         <paper-ripple fit></paper-ripple>
       </button>
-      <button id="gear-button" class="icon-button" tabindex="13" menu="#gear-menu"
+      <button id="gear-button" class="icon-button" tabindex="14" menu="#gear-menu"
               i18n-values="aria-label:GEAR_BUTTON_TOOLTIP"
               aria-activedescendant="gear-menu">
         <core-icon icon="more-vert"></core-icon>
@@ -358,6 +363,34 @@
       </button>
     </div>
 
+    <div id="cloud-import-details" class="offscreen">
+      <h1>Backup to Drive</h1>
+
+      <div id='#open-destination'>
+        <!!--
+          TODO(smckay): Setting volume-type-icon to "drive" breaks
+          functional tests because selection of the drive volume is
+          done using a query that matches this. Need to fix that.
+          See background/test_util.js:105
+        -->
+        <core-icon class="icon volume-icon" volume-type-icon="na">
+        </core-icon> My Photos
+      </div>
+
+      <div class="detail">
+        New photos: <span class="photo-count">0</span>
+      </div>
+      <div class="detail">
+        Space required: <span class="space-required">0</span>
+      </div>
+
+      <div class="media-strip">
+        <tt>// TODO(smckay)</tt>
+      </div>
+
+      <div class="import">Import!</div>
+    </div>
+
     <div class="dialog-container">
       <div class="dialog-navigation-list">
         <div class="dialog-navigation-list-contents">