Move view change analytics tracking into DirectoryModel where implementation details can be hidden.

ui/file_manager/file_manager/foreground/js/directory_model.js

Index: ui/file_manager/file_manager/foreground/js/directory_model.js
diff --git a/ui/file_manager/file_manager/foreground/js/directory_model.js b/ui/file_manager/file_manager/foreground/js/directory_model.js
index c7c8213865b2c1a2474393dbee011110cfb10940..2cf327d4a887dbbaeef9fe2f2aaf5dbab572eed1 100644
--- a/ui/file_manager/file_manager/foreground/js/directory_model.js
+++ b/ui/file_manager/file_manager/foreground/js/directory_model.js
@@ -11,6 +11,9 @@ var SHORT_RESCAN_INTERVAL = 100;
 /**
  * Data model of the file manager.
  *
+ * @constructor
+ * @extends {cr.EventTarget}
+ *
  * @param {boolean} singleSelection True if only one file could be selected
  *                                  at the time.
  * @param {FileFilter} fileFilter Instance of FileFilter.
@@ -18,12 +21,15 @@ var SHORT_RESCAN_INTERVAL = 100;
  *     service.
  * @param {VolumeManagerWrapper} volumeManager The volume manager.
  * @param {!FileOperationManager} fileOperationManager File operation manager.
- * @constructor
- * @extends {cr.EventTarget}
+ * @param {!analytics.Tracker} tracker
  */
-function DirectoryModel(singleSelection, fileFilter,
-                        metadataModel,
-                        volumeManager, fileOperationManager) {
+function DirectoryModel(
+    singleSelection,
+    fileFilter,
+    metadataModel,
+    volumeManager,
+    fileOperationManager,
+    tracker) {
   this.fileListSelection_ = singleSelection ?
       new FileListSingleSelectionModel() : new FileListSelectionModel();
 
@@ -65,6 +71,9 @@ function DirectoryModel(singleSelection, fileFilter,
       fileOperationManager,
       'entries-changed',
       this.onEntriesChanged_.bind(this));
+
+  /** @private {!analytics.Tracker} */
+  this.tracker_ = tracker;
 }
 
 /**
@@ -921,17 +930,23 @@ DirectoryModel.prototype.changeDirectoryEntry = function(
 
           var previousDirEntry =
               this.currentDirContents_.getDirectoryEntry();
-          this.clearAndScan_(
-              newDirectoryContents,
-              function(result) {
-                // Calls the callback of the method when successful.
-                if (result && opt_callback)
-                  opt_callback();
-
-                // Notify that the current task of this.directoryChangeQueue_
-                // is completed.
-                setTimeout(queueTaskCallback, 0);
-              });
+
+          var promises = [];
+
+          promises.push(
+              new Promise(
+                  /** @this {DirectoryModel} */
+                  function(resolve) {
+                    this.clearAndScan_(
+                        newDirectoryContents,
+                        function(result) {
+                          // Calls the callback of the method when successful.
+                          if (result && opt_callback)
+                            opt_callback();
+
+                          resolve(undefined);
+                        });
+                  }.bind(this)));
 
           // For tests that open the dialog to empty directories, everything
           // is loaded at this point.
@@ -945,12 +960,58 @@ DirectoryModel.prototype.changeDirectoryEntry = function(
           event.previousDirEntry = previousDirEntry;
           event.newDirEntry = dirEntry;
           event.volumeChanged = previousVolumeInfo !== currentVolumeInfo;
+
+          if (event.volumeChanged) {
+            promises.push(this.onVolumeChanged_(currentVolumeInfo));
+          }
+
           this.dispatchEvent(event);
+          Promise.all(promises).then(queueTaskCallback);
         }.bind(this));
   }.bind(this, this.changeDirectorySequence_));
 };
 
 /**
+ * Handles volume changed by sending an analytics appView event.
+ *
+ * @param {!VolumeInfo} volumeInfo The new volume info.
+ * @return {!Promise} resolves once handling is done.
+ * @private
+ */
+DirectoryModel.prototype.onVolumeChanged_ = function(volumeInfo) {
+  // NOTE: That dynamic values, like volume name MUST NOT
+  // be sent to GA as that value can contain PII.
+  // VolumeType is an enum.
+  // ...
+  // But we can do stuff like

[mtomasz, 2015/03/24 01:34:58]

nit: I'm OK not to fill to 80 characters, but this is really hard to read. Can
we at least try to fill least half of each line of comments?

[Steve McKay, 2015/03/24 22:39:20]

Done.

+  // figure out if this is a media device or vanilla
+  // removable device.
+  return Promise.resolve(undefined)
+      .then(
+          function() {
+            switch (volumeInfo.volumeType) {
+              case VolumeManagerCommon.VolumeType.REMOVABLE:
+                return importer.hasMediaDirectory(volumeInfo.fileSystem.root)

[mtomasz, 2015/03/24 01:34:58]

I'm worried about performance here. Switching volumes is already heavy. Could
you confirm that this is fast on Lucas with a lot of files/dirs on root?

[Steve McKay, 2015/03/24 22:39:20]

That's a valid concern. Fixed the hasMediaDirectory to check for specific dirs.

+                    .then(
+                        /** @param {boolean} hasMedia */

[mtomasz, 2015/03/24 01:34:58]

nit: Do we need @return here?

[Steve McKay, 2015/03/24 22:39:20]

Done.

+                        function(hasMedia) {
+                          return hasMedia ?
+                              volumeInfo.volumeType + '/with-media-dir' :
+                              volumeInfo.volumeType;
+                        });
+              case VolumeManagerCommon.VolumeType.PROVIDED:
+                // TODO(smckay): Can we supply a human readable name?

[Steve McKay, 2015/03/24 22:39:20]

Tomasz, this is a question for you.

[Steve McKay, 2015/03/24 23:49:22]

Actually. I saw your response. Ignore this.

+                // And should we limit this list to just known providers
+                // like the zip extension.
+                return volumeInfo.volumeType + '/' + volumeInfo.extensionId;
+              default:
+                return volumeInfo.volumeType;
+            }
+          })
+      .then(this.tracker_.sendAppView.bind(this.tracker_));
+};
+
+/**
  * Activates the given directory.
  * This method:
  *  - Changes the current directory, if the given directory is the current