CrOS FilesApp: Crostini UMAs

Added 3 FilesApp Crostini UMAs.

1/ Updated FileBrowser.MenuItemSelected to add 'Share with Linux'

2/ FileBrowser.CrostiniShareDialog reports either:
 * None - no dialog shown when using crostini app to open file.
   This indicates that the path is already shared.
 * Share before open - the share before open dialog is shown
 * Unable to open - shown when multiple files from different
   unshared directories are opened.

In order to implement this UMA correctly, the call to
maybeShowCrostiniShareDialog_ has been moved outside of
executeInternal_ so that these functions no longer call each
other recursively.

3/ FileBrowser.CrostiniSharedPaths.Depth.[downloads|drive|other]
tracks the depth from volume root of each path shared with crostini.
This is equivalent to how many '/' are in the path.  E.g. 'Downloads'
or 'My Drive' has depth 0.  'Downloads/foo/bar' has depth 2.

Currently dedicated histograms are created to track downloads and drive
with all other volumes captured together.  Dedicated histograms
can be created in the future for other volume types.

Bug: 878324
Change-Id: I0feb6d2652cce023d33aad254726068ad8da7e32
Reviewed-on: https://chromium-review.googlesource.com/c/1273155
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598631}
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 4aef9c3..5e35352 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -21316,6 +21316,12 @@
   <int value="6" label="Error"/>
 </enum>
 
+<enum name="FileManagerCrostiniShareDialogType">
+  <int value="0" label="None"/>
+  <int value="1" label="Share before open"/>
+  <int value="2" label="Unable to open"/>
+</enum>
+
 <enum name="FileManagerListType">
   <int value="0" label="Uninitialized"/>
   <int value="1" label="List view (detail)"/>
@@ -21335,6 +21341,7 @@
   <int value="9" label="Toggle Show Google Docs files (off)"/>
   <int value="10" label="Toggle show hidden Android folders (on)"/>
   <int value="11" label="Toggle show hidden Android folders (off)"/>
+  <int value="12" label="Share with Linux"/>
 </enum>
 
 <enum name="FileManagerQuickViewWayToOpen">
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 67401be..7842bd8 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -32522,6 +32522,24 @@
   <summary>Chrome OS File Browser opening mode.</summary>
 </histogram>
 
+<histogram name="FileBrowser.CrostiniShareDialog"
+    enum="FileManagerCrostiniShareDialogType" expires_after="2019-10-01">
+  <owner>joelhockey@chromium.org</owner>
+  <owner>tbuckley@chromium.org</owner>
+  <summary>Dialog shown when using crostini app to open a file.</summary>
+</histogram>
+
+<histogram base="true" name="FileBrowser.CrostiniSharedPaths.Depth"
+    units="depth" expires_after="2019-10-01">
+  <owner>joelhockey@chromium.org</owner>
+  <owner>tbuckley@chromium.org</owner>
+  <summary>
+    The depth from volume root of the path being shared. This is equivalent to
+    how many '/' are in the path. E.g. 'Downloads' or 'My Drive' has depth 0.
+    'Downloads/foo/bar' has depth 2.
+  </summary>
+</histogram>
+
 <histogram name="FileBrowser.DirectoryScan" units="ms">
   <owner>sashab@chromium.org</owner>
   <owner>slangley@chromium.org</owner>
@@ -124575,6 +124593,13 @@
   <affected-histogram name="Autofill.Quality.ServerType.ByFieldType"/>
 </histogram_suffixes>
 
+<histogram_suffixes name="FileBrowserCrostiniSharedPathsDepth" separator=".">
+  <suffix name="downloads" label="Shared path in Downloads volume."/>
+  <suffix name="drive" label="Shared path in Drive volume."/>
+  <suffix name="other" label="Shared path in any other volume."/>
+  <affected-histogram name="FileBrowser.CrostiniSharedPaths.Depth"/>
+</histogram_suffixes>
+
 <histogram_suffixes name="FileBrowserLoad" separator=".">
   <suffix name="BackgroundLaunch"
       label="Time from onLaunched event is called to the window is created."/>
diff --git a/ui/file_manager/file_manager/foreground/js/crostini.js b/ui/file_manager/file_manager/foreground/js/crostini.js
index ec49d54..670519b97 100644
--- a/ui/file_manager/file_manager/foreground/js/crostini.js
+++ b/ui/file_manager/file_manager/foreground/js/crostini.js
@@ -11,6 +11,19 @@
 Crostini.IS_CROSTINI_FILES_ENABLED = false;
 
 /**
+ * Keep in sync with histograms.xml:FileBrowserCrostiniSharedPathsDepth
+ * histogram_suffix.
+ * @private {!Array<VolumeManagerCommon.RootType>}
+ */
+Crostini.UMA_VALID_ROOT_TYPES = [
+  VolumeManagerCommon.RootType.DOWNLOADS,
+  VolumeManagerCommon.RootType.DRIVE,
+];
+
+/** @private {string} */
+Crostini.UMA_ROOT_TYPE_OTHER = 'other';
+
+/**
  * Maintains a list of paths shared with the crostini container.
  * Keyed by VolumeManagerCommon.RootType, with boolean set values
  * of string paths.  e.g. {'Downloads': {'/foo': true, '/bar': true}}.
@@ -33,6 +46,14 @@
     Crostini.SHARED_PATHS_[info.rootType] = paths;
   }
   paths[entry.fullPath] = true;
+
+  // Record UMA.
+  let suffix = info.rootType;
+  if (!Crostini.UMA_VALID_ROOT_TYPES.includes(info.rootType))
+    suffix = Crostini.UMA_ROOT_TYPE_OTHER;
+  metrics.recordSmallCount(
+      'CrostiniSharedPaths.Depth.' + suffix,
+      entry.fullPath.split('/').length - 1);
 };
 
 /**
diff --git a/ui/file_manager/file_manager/foreground/js/crostini_unittest.js b/ui/file_manager/file_manager/foreground/js/crostini_unittest.js
index 6d8d893..374657f 100644
--- a/ui/file_manager/file_manager/foreground/js/crostini_unittest.js
+++ b/ui/file_manager/file_manager/foreground/js/crostini_unittest.js
@@ -2,6 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+window.metrics = {
+  recordSmallCount: function() {},
+};
+
 const volumeManagerTest = {
   getLocationInfo: (entry) => {
     return {rootType: 'testroot'};
diff --git a/ui/file_manager/file_manager/foreground/js/file_manager_commands.js b/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
index 5e1700b..f8ddc3a 100644
--- a/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
+++ b/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
@@ -394,6 +394,7 @@
   SHOW_GOOGLE_DOCS_FILES_ON: 'drive-hosted-settings-enabled',
   HIDDEN_ANDROID_FOLDERS_SHOW: 'toggle-hidden-android-folders-on',
   HIDDEN_ANDROID_FOLDERS_HIDE: 'toggle-hidden-android-folders-off',
+  SHARE_WITH_LINUX: 'share-with-linux',
 };
 
 /**
@@ -418,6 +419,7 @@
   CommandHandler.MenuCommandsForUMA.SHOW_GOOGLE_DOCS_FILES_OFF,
   CommandHandler.MenuCommandsForUMA.HIDDEN_ANDROID_FOLDERS_SHOW,
   CommandHandler.MenuCommandsForUMA.HIDDEN_ANDROID_FOLDERS_HIDE,
+  CommandHandler.MenuCommandsForUMA.SHARE_WITH_LINUX,
 ];
 console.assert(
     Object.keys(CommandHandler.MenuCommandsForUMA).length ===
@@ -1653,6 +1655,8 @@
               Crostini.registerSharedPath(dir, fileManager.volumeManager);
             }
           });
+      CommandHandler.recordMenuItemSelected_(
+          CommandHandler.MenuCommandsForUMA.SHARE_WITH_LINUX);
     }
   },
   /**
diff --git a/ui/file_manager/file_manager/foreground/js/file_tasks.js b/ui/file_manager/file_manager/foreground/js/file_tasks.js
index 8d8e34c..e99eacd 100644
--- a/ui/file_manager/file_manager/foreground/js/file_tasks.js
+++ b/ui/file_manager/file_manager/foreground/js/file_tasks.js
@@ -377,6 +377,39 @@
 };
 
 /**
+ * Crostini Share Dialog types.
+ * Keep in sync with enums.xml FileManagerCrostiniShareDialogType.
+ * @enum {string}
+ */
+FileTasks.CrostiniShareDialogType = {
+  None: 'None',
+  ShareBeforeOpen: 'ShareBeforeOpen',
+  UnableToOpen: 'UnableToOpen',
+};
+
+/**
+ * The indexes of these types must match with the values of
+ * FileManagerCrostiniShareDialogType in enums.xml, and should not change.
+ */
+FileTasks.UMA_CROSTINI_SHARE_DIALOG_TYPES_ = Object.freeze([
+  FileTasks.CrostiniShareDialogType.None,
+  FileTasks.CrostiniShareDialogType.ShareBeforeOpen,
+  FileTasks.CrostiniShareDialogType.UnableToOpen,
+]);
+
+
+/**
+ * Records the type of dialog shown when using a crostini app to open a file.
+ * @param {!FileTasks.CrostiniShareDialogType} dialogType
+ * @private
+ */
+FileTasks.recordCrostiniShareDialogTypeUMA_ = function(dialogType) {
+  metrics.recordEnum(
+      'CrostiniShareDialog', dialogType,
+      FileTasks.UMA_CROSTINI_SHARE_DIALOG_TYPES_);
+};
+
+/**
  * Returns true if the taskId is for an internal task.
  *
  * @param {string} taskId Task identifier.
@@ -556,6 +589,8 @@
     this.ui_.alertDialog.showHtml(
         strf('UNABLE_TO_OPEN_CROSTINI_TITLE', task.title),
         strf('UNABLE_TO_OPEN_CROSTINI', task.title));
+    FileTasks.recordCrostiniShareDialogTypeUMA_(
+        FileTasks.CrostiniShareDialogType.UnableToOpen);
     return true;
   }
 
@@ -571,10 +606,14 @@
                 'SHARE_BEFORE_OPEN_CROSTINI_SINGLE',
                 `<b>${firstEntryNotShared.name}</b>`),
         this.sharePathWithCrostiniAndExecute_.bind(this, task), () => {});
+    FileTasks.recordCrostiniShareDialogTypeUMA_(
+        FileTasks.CrostiniShareDialogType.ShareBeforeOpen);
     return true;
   }
 
   // No dialogs.
+  FileTasks.recordCrostiniShareDialogTypeUMA_(
+      FileTasks.CrostiniShareDialogType.None);
   return false;
 };
 
@@ -592,11 +631,7 @@
                 'Error sharing with linux to execute: ' +
                 chrome.runtime.lastError.message);
           } else {
-            // Register path as shared, and execute. This will be the 2nd
-            // time we have gone through executeInternal_().  The first time,
-            // we showed the share dialog, and did not execute, now we
-            // should detect that paths are already shared, and it is OK to
-            // execute.
+            // Register path as shared, and now we are ready to execute.
             Crostini.registerSharedPath(dir, this.volumeManager_);
             this.executeInternal_(task);
           }
@@ -633,6 +668,8 @@
   var callback = opt_callback || function(arg1, arg2) {};
 
   if (this.defaultTask_ !== null) {
+    if (this.maybeShowCrostiniShareDialog_(this.defaultTask_))
+      return;
     this.executeInternal_(this.defaultTask_);
     callback(true, this.entries_);
     return;
@@ -754,6 +791,8 @@
   FileTasks.recordViewingFileTypeUMA_(this.entries_);
   FileTasks.recordViewingRootTypeUMA_(
       this.directoryModel_.getCurrentRootType());
+  if (this.maybeShowCrostiniShareDialog_(task))
+    return;
   this.executeInternal_(task);
 };
 
@@ -768,8 +807,6 @@
     this.taskHistory_.recordTaskExecuted(task.taskId);
     if (FileTasks.isInternalTask_(task.taskId)) {
       this.executeInternalTask_(task.taskId);
-    } else if (this.maybeShowCrostiniShareDialog_(task)) {
-      // Nothing to do, dialog will be shown.
     } else {
       FileTasks.recordZipHandlerUMA_(task.taskId);
       chrome.fileManagerPrivate.executeTask(
diff --git a/ui/file_manager/file_manager/foreground/js/file_tasks_unittest.js b/ui/file_manager/file_manager/foreground/js/file_tasks_unittest.js
index 2fd804d..386962c 100644
--- a/ui/file_manager/file_manager/foreground/js/file_tasks_unittest.js
+++ b/ui/file_manager/file_manager/foreground/js/file_tasks_unittest.js
@@ -3,7 +3,8 @@
 // found in the LICENSE file.
 
 window.metrics = {
-  recordEnum: function() {}
+  recordEnum: function() {},
+  recordSmallCount: function() {},
 };
 
 var mockTaskHistory = {
diff --git a/ui/file_manager/file_manager/test/crostini.js b/ui/file_manager/file_manager/test/crostini.js
index cde9238..086946a 100644
--- a/ui/file_manager/file_manager/test/crostini.js
+++ b/ui/file_manager/file_manager/test/crostini.js
@@ -167,6 +167,7 @@
     executeTaskCalled = true;
     oldExecuteTask(taskId, entries, callback);
   };
+  chrome.metricsPrivate.values_ = [];
 
   test.setupAndWaitUntilReady([], [], [])
       .then(() => {
@@ -225,6 +226,12 @@
         });
       })
       .then(() => {
+        // Validate UMAs.
+        const lastEnumUma = chrome.metricsPrivate.values_.pop();
+        assertEquals(
+            'FileBrowser.CrostiniShareDialog', lastEnumUma[0].metricName);
+        assertEquals(1 /* ShareBeforeOpen */, lastEnumUma[1]);
+
         // Restore fmp.*.
         chrome.fileManagerPrivate.getFileTasks = oldGetFileTasks;
         chrome.fileManagerPrivate.sharePathWithCrostini = oldSharePath;
@@ -288,9 +295,14 @@
   const oldSharePath = chrome.fileManagerPrivate.sharePathWithCrostini;
   let sharePathCalled = false;
   chrome.fileManagerPrivate.sharePathWithCrostini = (entry, callback) => {
-    sharePathCalled = true;
-    oldSharePath(entry, callback);
+    oldSharePath(entry, () => {
+      sharePathCalled = true;
+      callback();
+    });
   };
+  chrome.metricsPrivate.smallCounts_ = [];
+  chrome.metricsPrivate.values_ = [];
+
   test.setupAndWaitUntilReady()
       .then(() => {
         // Right-click 'photos' directory.
@@ -308,11 +320,22 @@
             test.fakeMouseClick(
                 '#file-context-menu [command="#share-with-linux"]'),
             'Share with Linux');
-        return test.waitForElement('#file-context-menu[hidden]');
+        // Check sharePathWithCrostini is called.
+        return test.repeatUntil(() => {
+          return sharePathCalled || test.pending('wait for sharePathCalled');
+        });
       })
       .then(() => {
-        // Check sharePathWithCrostini is called.
-        assertTrue(sharePathCalled);
+        // Validate UMAs.
+        assertEquals(1, chrome.metricsPrivate.smallCounts_.length);
+        assertArrayEquals(
+            ['FileBrowser.CrostiniSharedPaths.Depth.downloads', 1],
+            chrome.metricsPrivate.smallCounts_[0]);
+        const lastEnumUma = chrome.metricsPrivate.values_.pop();
+        assertEquals('FileBrowser.MenuItemSelected', lastEnumUma[0].metricName);
+        assertEquals(12 /* Share with Linux */, lastEnumUma[1]);
+
+        // Restore fmp.*.
         chrome.fileManagerPrivate.sharePathWithCrostini = oldSharePath;
         done();
       });
diff --git a/ui/file_manager/file_manager/test/js/chrome_api_test_impl.js b/ui/file_manager/file_manager/test/js/chrome_api_test_impl.js
index 1ec1954..c71cf7c3 100644
--- a/ui/file_manager/file_manager/test/js/chrome_api_test_impl.js
+++ b/ui/file_manager/file_manager/test/js/chrome_api_test_impl.js
@@ -95,20 +95,26 @@
 
   metricsPrivate: {
     userActions_: [],
+    smallCounts_: [],
     times_: [],
+    values_: [],
     MetricTypeType: {
       HISTOGRAM_LINEAR: 'histogram-linear',
     },
     recordMediumCount: () => {},
     recordPercentage: () => {},
-    recordSmallCount: () => {},
+    recordSmallCount: (metricName, value) => {
+      chrome.metricsPrivate.smallCounts_.push([metricName, value]);
+    },
     recordTime: (metricName, value) => {
       chrome.metricsPrivate.times_.push([metricName, value]);
     },
     recordUserAction: (action) => {
       chrome.metricsPrivate.userActions_.push(action);
     },
-    recordValue: () => {},
+    recordValue: (metricName, value) => {
+      chrome.metricsPrivate.values_.push([metricName, value]);
+    },
   },
 
   notifications: {