Files app: Code refactoring

Moves static functions and enums associated with sharing action to
FileTasks. This is due to the fact that sharing with Android apps can
take a path through FileTasks without touching FileManagerCommands. This
CL just prepares the code for the additional UMA records, without
actually adding new recoding actions.

Bug: 1063169
Change-Id: Ic8833574241c0d9502b77356dbbb58c44211cf81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2198849Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Bo Majewski <majewski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769589}

ui/file_manager/file_manager/foreground/js/file_manager_commands.js

@@ -31,17 +31,6 @@ class Command {
  */
 const CommandUtil = {};
 
-/**
- * Possible share action sources for UMA.
- * @enum {string}
- * @const
- */
-CommandUtil.SharingActionSourceForUMA = {
-  UNKNOWN: 'Unknown',
-  CONTEXT_MENU: 'Context Menu',
-  SHARE_BUTTON: 'Share Button',
-};
-
 /**
  * The IDs of elements that can trigger share action.
  * @enum {string}
@@ -52,33 +41,23 @@ CommandUtil.SharingActionElementId = {
   SHARE_BUTTON: 'share-menu-button',
 };
 
-/**
- * A list of supported values for SharingActionSource enum. Keep this in sync
- * with SharingActionSource defined in //tools/metrics/histograms/enums.xml.
- */
-CommandUtil.ValidSharingActionSource = [
-  CommandUtil.SharingActionSourceForUMA.UNKNOWN,
-  CommandUtil.SharingActionSourceForUMA.CONTEXT_MENU,
-  CommandUtil.SharingActionSourceForUMA.SHARE_BUTTON,
-];
-
 /**
  * Helper function that for the given event returns the source of a share
  * action. If the source cannot be determined, this function returns
  * CommandUtil.SharingActionSourceForUMA.UNKNOWN.
  * @param {!Event} event The event that triggered share action.
- * @return {!CommandUtil.SharingActionSourceForUMA}
+ * @return {!FileTasks.SharingActionSourceForUMA}
  */
 CommandUtil.getSharingActionSource = event => {
   const id = event.target.id;
   switch (id) {
     case CommandUtil.SharingActionElementId.CONTEXT_MENU:
-      return CommandUtil.SharingActionSourceForUMA.CONTEXT_MENU;
+      return FileTasks.SharingActionSourceForUMA.CONTEXT_MENU;
     case CommandUtil.SharingActionElementId.SHARE_BUTTON:
-      return CommandUtil.SharingActionSourceForUMA.SHARE_BUTTON;
+      return FileTasks.SharingActionSourceForUMA.SHARE_BUTTON;
     default: {
       console.error('Unrecognized event.target.id for sharing action "%s"', id);
-      return CommandUtil.SharingActionSourceForUMA.UNKNOWN;
+      return FileTasks.SharingActionSourceForUMA.UNKNOWN;
     }
   }
 };
@@ -605,24 +584,6 @@ CommandHandler.recordMenuItemSelected = menuItem => {
       'MenuItemSelected', menuItem, CommandHandler.ValidMenuCommandsForUMA);
 };
 
-/**
- * Records UMA statistics about Share action.
- * @param {!Event} event The event that triggered sharing action.
- * @param {!Array<!FileEntry>} entries File entries to be shared.
- */
-CommandHandler.recordSharingAction = (event, entries) => {
-  const source = CommandUtil.getSharingActionSource(event);
-  metrics.recordEnum(
-      'Share.ActionSource', source, CommandUtil.ValidSharingActionSource);
-  metrics.recordSmallCount('Share.FileCount', entries.length);
-  for (const entry of entries) {
-    metrics.recordEnum(
-        'Share.FileType', FileTasks.getViewFileType(entry),
-        FileTasks.UMA_INDEX_KNOWN_EXTENSIONS);
-  }
-  // TODO(crbug.com/1063169): record Share.AppId AppID for each entity.
-};
-
 /**
  * Commands.
  * @private @const {Object<Command>}
@@ -1821,7 +1782,8 @@ CommandHandler.COMMANDS_['zip-selection'] = new class extends Command {
 CommandHandler.COMMANDS_['share'] = new class extends Command {
   execute(event, fileManager) {
     const entries = CommandUtil.getCommandEntries(fileManager, event.target);
-    CommandHandler.recordSharingAction(event, entries);
+    FileTasks.recordSharingAction(
+        CommandUtil.getSharingActionSource(event), entries);
     const actionsController = fileManager.actionsController;
 
     fileManager.actionsController.getActionsForEntries(entries).then(

ui/file_manager/file_manager/foreground/js/file_manager_commands_unittest.js

@@ -2,21 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-/**
- * Utility function that appends value under a given name in the store.
- * @param {!Map<string, !Array<string|number>>} store
- * @param {string} name
- * @param {string|number} value
- */
-function record(store, name, value) {
-  let recorded = store.get(name);
-  if (!recorded) {
-    recorded = [];
-    store.set(name, recorded);
-  }
-  recorded.push(value);
-}
-
 /**
  * Checks that a correct sharing action source is extracted from an event.
  */
@@ -24,19 +9,19 @@ function testGetSharingActionSource() {
   const testData = [
     {
       event: {target: {id: CommandUtil.SharingActionElementId.CONTEXT_MENU}},
-      expected: CommandUtil.SharingActionSourceForUMA.CONTEXT_MENU,
+      expected: FileTasks.SharingActionSourceForUMA.CONTEXT_MENU,
     },
     {
       event: {target: {id: CommandUtil.SharingActionElementId.SHARE_BUTTON}},
-      expected: CommandUtil.SharingActionSourceForUMA.SHARE_BUTTON,
+      expected: FileTasks.SharingActionSourceForUMA.SHARE_BUTTON,
     },
     {
       event: {target: {id: '__no_such_id__'}},
-      expected: CommandUtil.SharingActionSourceForUMA.UNKNOWN,
+      expected: FileTasks.SharingActionSourceForUMA.UNKNOWN,
     },
     {
       event: {target: {id: null}},
-      expected: CommandUtil.SharingActionSourceForUMA.UNKNOWN,
+      expected: FileTasks.SharingActionSourceForUMA.UNKNOWN,
     },
   ];
   for (const data of testData) {
@@ -45,50 +30,3 @@ function testGetSharingActionSource() {
   }
 }
 
-/**
- * Checks that we are correctly recording UMA about Share action.
- */
-function testReportSharingAction() {
-  // Setup: create a fake metrics object that can be examined for content.
-  const enumMap = new Map();
-  const countMap = new Map();
-  window.metrics = {
-    recordEnum: (name, value, valid) => {
-      assertTrue(valid.includes(value));
-      record(enumMap, name, value);
-    },
-    recordSmallCount: (name, value) => {
-      record(countMap, name, value);
-    },
-  };
-  const mockFileSystem = new MockFileSystem('volumeId');
-
-  // Actual tests.
-  CommandHandler.recordSharingAction(
-      /** @type {!Event} */ (
-          {target: {id: CommandUtil.SharingActionElementId.CONTEXT_MENU}}),
-      [
-        MockFileEntry.create(mockFileSystem, '/test.log'),
-        MockFileEntry.create(mockFileSystem, '/test.doc'),
-        MockFileEntry.create(mockFileSystem, '/test.__no_such_extension__'),
-      ]);
-  assertArrayEquals(
-      enumMap.get('Share.ActionSource'),
-      [CommandUtil.SharingActionSourceForUMA.CONTEXT_MENU]);
-  assertArrayEquals(countMap.get('Share.FileCount'), [3]);
-  assertArrayEquals(enumMap.get('Share.FileType'), ['.log', '.doc', 'other']);
-
-  CommandHandler.recordSharingAction(
-      /** @type {!Event} */ (
-          {target: {id: CommandUtil.SharingActionElementId.SHARE_BUTTON}}),
-      [
-        MockFileEntry.create(mockFileSystem, '/test.log'),
-      ]);
-  assertArrayEquals(enumMap.get('Share.ActionSource'), [
-    CommandUtil.SharingActionSourceForUMA.CONTEXT_MENU,
-    CommandUtil.SharingActionSourceForUMA.SHARE_BUTTON,
-  ]);
-  assertArrayEquals(countMap.get('Share.FileCount'), [3, 1]);
-  assertArrayEquals(
-      enumMap.get('Share.FileType'), ['.log', '.doc', 'other', '.log']);
-}

ui/file_manager/file_manager/foreground/js/file_tasks.js

@@ -252,6 +252,24 @@ class FileTasks {
     return extension;
   }
 
+  /**
+   * Records UMA statistics about Share action.
+   * @param {!FileTasks.SharingActionSourceForUMA} source The enum representing
+   *     the UI element that triggered the share action.
+   * @param {!Array<!FileEntry>} entries File entries to be shared.
+   */
+  static recordSharingAction(source, entries) {
+    metrics.recordEnum(
+        'Share.ActionSource', source, FileTasks.ValidSharingActionSource);
+    metrics.recordSmallCount('Share.FileCount', entries.length);
+    for (const entry of entries) {
+      metrics.recordEnum(
+          'Share.FileType', FileTasks.getViewFileType(entry),
+          FileTasks.UMA_INDEX_KNOWN_EXTENSIONS);
+    }
+    // TODO(crbug.com/1063169): record Share.AppId AppID for each entity.
+  }
+
   /**
    * Records trial of opening file grouped by extensions.
    *
@@ -1297,6 +1315,27 @@ FileTasks.UMA_CROSTINI_SHARE_DIALOG_TYPES_ = Object.freeze([
   FileTasks.CrostiniShareDialogType.UnableToOpen,
 ]);
 
+/**
+ * Possible share action sources for UMA.
+ * @enum {string}
+ * @const
+ */
+FileTasks.SharingActionSourceForUMA = {
+  UNKNOWN: 'Unknown',
+  CONTEXT_MENU: 'Context Menu',
+  SHARE_BUTTON: 'Share Button',
+};
+
+/**
+ * A list of supported values for SharingActionSource enum. Keep this in sync
+ * with SharingActionSource defined in //tools/metrics/histograms/enums.xml.
+ */
+FileTasks.ValidSharingActionSource = Object.freeze([
+  FileTasks.SharingActionSourceForUMA.UNKNOWN,
+  FileTasks.SharingActionSourceForUMA.CONTEXT_MENU,
+  FileTasks.SharingActionSourceForUMA.SHARE_BUTTON,
+]);
+
 /**
  * The number of menu-item entries in the top level menu
  * before we split and show the 'More actions' option

ui/file_manager/file_manager/foreground/js/file_tasks_unittest.js

@@ -241,6 +241,21 @@ function showImportCrostiniImageDialogIsCalled(entries) {
   });
 }
 
+/**
+ * Utility function that appends value under a given name in the store.
+ * @param {!Map<string, !Array<string|number>>} store
+ * @param {string} name
+ * @param {string|number} value
+ */
+function record(store, name, value) {
+  let recorded = store.get(name);
+  if (!recorded) {
+    recorded = [];
+    store.set(name, recorded);
+  }
+  recorded.push(value);
+}
+
 /**
  * Tests opening a .exe file.
  */
@@ -629,3 +644,47 @@ function testGetViewFileType() {
     assertEquals(data.expected, type);
   }
 }
+
+/**
+ * Checks that we are correctly recording UMA about Share action.
+ */
+function testRecordSharingAction() {
+  // Setup: create a fake metrics object that can be examined for content.
+  const enumMap = new Map();
+  const countMap = new Map();
+  window.metrics = {
+    recordEnum: (name, value, valid) => {
+      assertTrue(valid.includes(value));
+      record(enumMap, name, value);
+    },
+    recordSmallCount: (name, value) => {
+      record(countMap, name, value);
+    },
+  };
+  const mockFileSystem = new MockFileSystem('volumeId');
+
+  // Actual tests.
+  FileTasks.recordSharingAction(
+      FileTasks.SharingActionSourceForUMA.CONTEXT_MENU, [
+        MockFileEntry.create(mockFileSystem, '/test.log'),
+        MockFileEntry.create(mockFileSystem, '/test.doc'),
+        MockFileEntry.create(mockFileSystem, '/test.__no_such_extension__'),
+      ]);
+  assertArrayEquals(
+      enumMap.get('Share.ActionSource'),
+      [FileTasks.SharingActionSourceForUMA.CONTEXT_MENU]);
+  assertArrayEquals(countMap.get('Share.FileCount'), [3]);
+  assertArrayEquals(enumMap.get('Share.FileType'), ['.log', '.doc', 'other']);
+
+  FileTasks.recordSharingAction(
+      FileTasks.SharingActionSourceForUMA.SHARE_BUTTON, [
+        MockFileEntry.create(mockFileSystem, '/test.log'),
+      ]);
+  assertArrayEquals(enumMap.get('Share.ActionSource'), [
+    FileTasks.SharingActionSourceForUMA.CONTEXT_MENU,
+    FileTasks.SharingActionSourceForUMA.SHARE_BUTTON,
+  ]);
+  assertArrayEquals(countMap.get('Share.FileCount'), [3, 1]);
+  assertArrayEquals(
+      enumMap.get('Share.FileType'), ['.log', '.doc', 'other', '.log']);
+}