Move "Add new services" button from directory tree

Add "Add new services" menu item to Files app "gear menu" and change
the behavior to:

1. Go directly to Webstore in case there isn't any provider/FSP
installed.
2. Open "providers menu" when there is at least one provider/FSP
installed, which is the old behaviour.

Add a new command "new-service" to display the providers menu to be
able open it from the gear menu.

Change the gear menu item label to be "Add new service".

Fix My files tests because "Add new services" isn't displayed on
directory tree anymore.

Bug: 682356, 869240
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ie9bf211c42a18b3717a72f40e57ceaade5958281
Reviewed-on: https://chromium-review.googlesource.com/1139944Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579639}

chrome/app/chromeos_strings.grdp

@@ -434,10 +434,10 @@
     Close
   </message>
   <message name="IDS_FILE_BROWSER_ADD_NEW_SERVICES_BUTTON_LABEL" desc="Title of the button in the left nav to add new services (file systems) to the Files app.">
-    Add new services
+    Add new service
   </message>
   <message name="IDS_FILE_BROWSER_INSTALL_NEW_EXTENSION_LABEL" desc="Title of the menu item for installing new extensions from the web store.">
-    Install new from the webstore
+    Install new service
   </message>
   <message name="IDS_FILE_BROWSER_TASK_VIEW" desc="Title of the action to view (no edit) a file.">
     View

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

@@ -590,7 +590,8 @@ FileManager.prototype = /** @struct */ {
         this.ui_.gearButtonToggleRipple,
         this.ui_.gearMenu,
         this.directoryModel_,
-        this.commandHandler_);
+        this.commandHandler_,
+        assert(this.providersModel_));
     this.selectionMenuController_ = new SelectionMenuController(
         this.ui_.selectionMenuButton,
         util.queryDecoratedElement('#file-context-menu', cr.ui.Menu));
@@ -1191,9 +1192,6 @@ FileManager.prototype = /** @struct */ {
         (this.dialogDom_.querySelector('#directory-tree'));
     var fakeEntriesVisible =
         this.dialogType !== DialogType.SELECT_SAVEAS_FILE;
-    var addNewServicesVisible =
-        this.dialogType === DialogType.FULL_PAGE &&
-        !chrome.extension.inIncognitoContext;
     this.navigationUma_ = new NavigationUma(assert(this.volumeManager_));
     DirectoryTree.decorate(directoryTree,
                            assert(this.directoryModel_),
@@ -1215,11 +1213,7 @@ FileManager.prototype = /** @struct */ {
                   sourceRestriction: this.getSourceRestriction_()
                 }) :
             null,
-        addNewServicesVisible ?
-            new NavigationModelMenuItem(
-                str('ADD_NEW_SERVICES_BUTTON_LABEL'), '#add-new-services-menu',
-                'add-new-services') :
-            null,
+        null,  // TODO(crbug.com/869252) remove this null.
         this.commandLineFlags_['disable-my-files-navigation']);
     this.setupCrostini_();
     this.ui_.initDirectoryTree(directoryTree);

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

@@ -2048,3 +2048,43 @@ CommandHandler.COMMANDS_['volume-storage'] = /** @type {Command} */ ({
   },
   canExecute: CommandUtil.canExecuteAlways
 });
+
+/**
+ * Opens "providers menu" to allow users to install new providers/FSPs.
+ * @type {Command}
+ */
+CommandHandler.COMMANDS_['new-service'] = /** @type {Command} */ ({
+  /**
+   * @param {!Event} event Command event.
+   * @param {!CommandHandlerDeps} fileManager CommandHandlerDeps to use.
+   */
+  execute: function(event, fileManager) {
+    const menuButton = fileManager.ui.newServiceButton;
+    // Make the button visible because showMenu positions the menu relative to
+    // to this button.
+    menuButton.hidden = false;
+
+    // Fire update event to display services that are already installed.
+    const updateEvent =
+        /** @type {MenuItemUpdateEvent} */ (new Event('update'));
+
+    // Display the menu near gearMenu, since menuButton will be hidden.
+    // The event listener (ProvidersMenu) only uses this menuButton for
+    // positioning.
+    updateEvent.menuButton = fileManager.ui.gearButton;
+    menuButton.menu.dispatchEvent(updateEvent);
+    menuButton.showMenu(false);
+
+    // Hide it back.
+    menuButton.hidden = true;
+  },
+  /**
+   * @param {!Event} event Command event.
+   * @param {!CommandHandlerDeps} fileManager CommandHandlerDeps to use.
+   */
+  canExecute: function(event, fileManager) {
+    event.canExecute =
+        (fileManager.dialogType === DialogType.FULL_PAGE &&
+         !chrome.extension.inIncognitoContext);
+  }
+});

ui/file_manager/file_manager/foreground/js/gear_menu_controller.js

@@ -8,11 +8,13 @@
  * @param {!GearMenu} gearMenu
  * @param {!DirectoryModel} directoryModel
  * @param {!CommandHandler} commandHandler
+ * @param {!ProvidersModel} providersModel
  * @constructor
  * @struct
  */
 function GearMenuController(
-    gearButton, toggleRipple, gearMenu, directoryModel, commandHandler) {
+    gearButton, toggleRipple, gearMenu, directoryModel, commandHandler,
+    providersModel) {
   /**
    * @type {!FilesToggleRipple}
    * @const
@@ -41,6 +43,13 @@ function GearMenuController(
    */
   this.commandHandler_ = commandHandler;
 
+  /**
+   * @type {!ProvidersModel}
+   * @const
+   * @private
+   */
+  this.providersModel_ = providersModel;
+
   gearButton.addEventListener('menushow', this.onShowGearMenu_.bind(this));
   gearButton.addEventListener('menuhide', this.onHideGearMenu_.bind(this));
   directoryModel.addEventListener(
@@ -59,6 +68,35 @@ GearMenuController.prototype.onShowGearMenu_ = function() {
 
   // Update view of drive-related settings.
   this.commandHandler_.updateAvailability();
+
+  this.updateNewServiceItem();
+};
+
+/**
+ * Update "New service" menu item to either directly show the Webstore dialog
+ * when there isn't any service/FSP extension installed, or display the
+ * providers menu with the currently installed extensions and also install new
+ * service.
+ *
+ * @private
+ */
+GearMenuController.prototype.updateNewServiceItem = function() {
+  this.providersModel_.getMountableProviders().then(providers => {
+    // Go straight to webstore to install the first provider.
+    let desiredMenu = '#install-new-extension';
+    let label = str('INSTALL_NEW_EXTENSION_LABEL');
+
+    const shouldDisplayProvidersMenu = providers.length > 0;
+    if (shouldDisplayProvidersMenu) {
+      // Open the providers menu with an installed provider and an install new
+      // provider option.
+      desiredMenu = '#new-service';
+      label = str('ADD_NEW_SERVICES_BUTTON_LABEL');
+    }
+
+    this.gearMenu_.setNewServiceCommand(desiredMenu, label);
+  });
+
 };
 
 /**

ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js

@@ -219,6 +219,14 @@ function FileManagerUI(providersModel, element, launchParam) {
   this.gearButton = util.queryDecoratedElement(
       '#gear-button', cr.ui.MenuButton);
 
+  /**
+   * The button to add new service (file system providers).
+   * @type {!cr.ui.MenuButton}
+   * @const
+   */
+  this.newServiceButton =
+      util.queryDecoratedElement('#new-service-button', cr.ui.MenuButton);
+
   /**
    * Ripple effect of gear button.
    * @type {!FilesToggleRipple}

ui/file_manager/file_manager/foreground/js/ui/gear_menu.js

@@ -61,6 +61,14 @@ function GearMenu(element) {
       this.volumeSpaceInnerBar_.parentElement,
       HTMLElement);
 
+  /**
+   * @type {!HTMLElement}
+   * @const
+   * @private
+   */
+  this.newServiceMenuItem_ =
+      queryRequiredElement('#gear-menu-newservice', element);
+
   /**
    * Volume space info.
    * @type {Promise<chrome.fileManagerPrivate.MountPointSizeStats>}
@@ -73,6 +81,20 @@ function GearMenu(element) {
   this.hostedButton.checkable = true;
 }
 
+/**
+ * @param {!string} commandId Element id of the command that new service menu
+ *     should trigger.
+ * @param {!string} label Text that should be displayed to user in the menu.
+ */
+GearMenu.prototype.setNewServiceCommand = function(commandId, label) {
+  this.newServiceMenuItem_.textContent = label;
+  // Only change command if needed because it does some parsing when setting.
+  if ('#' + this.newServiceMenuItem_.command.id === commandId) {
+    return;
+  }
+  this.newServiceMenuItem_.command = commandId;
+};
+
 /**
  * @param {Promise<chrome.fileManagerPrivate.MountPointSizeStats>}
  * spaceInfoPromise Promise to be fulfilled with space info.

ui/file_manager/file_manager/main.html

@@ -111,6 +111,8 @@
                shortcut="e|Ctrl">
       <command id="new-window" i18n-values="label:NEW_WINDOW_BUTTON_LABEL"
                shortcut="n|Ctrl">
+      <command id="new-service" i18n-values="label:INSTALL_NEW_EXTENSION_LABEL">
+      </command>
       <!-- Note that actual shortcut Ctrl/Meta+A is handled in
                ListSelectionController.handleKeyDown -->
       <command id="select-all"
@@ -274,6 +276,7 @@
       <cr-menu-item id="gear-menu-volume-help"
                     command="#volume-help"></cr-menu-item>
       <hr id="volume-space-info-separator">
+      <cr-menu-item id="gear-menu-newservice" command="#new-service"></cr-menu-item>
       <cr-menu-item id="volume-space-info" command="#volume-storage">
         <div id="volume-space-info-contents">
           <span id="volume-space-info-label"></span>
@@ -402,6 +405,9 @@
           <files-toggle-ripple></files-toggle-ripple>
           <div class="icon"></div>
         </button>
+        <button id="new-service-button" class="icon-button menu-button"
+                menu="#add-new-services-menu" hidden>
+        </button>
         <button id="selection-menu-button" class="icon-button menu-button" tabindex="19"
                 menu="#file-context-menu"
                 i18n-values="aria-label:SELECTION_MENU_BUTTON_TOOLTIP"

ui/file_manager/file_manager/test/js/strings.js

@@ -58,7 +58,7 @@ loadTimeData.data = new Proxy(
           'devices/goodies.html?utm_source=filesapp&utm_medium=banner&' +
           'utm_campaign=gsg',
       IMAGE_FILE_TYPE: '$1 image',
-      INSTALL_NEW_EXTENSION_LABEL: 'Install new from the webstore',
+      INSTALL_NEW_EXTENSION_LABEL: 'Install new service',
       LINUX_FILES_ROOT_LABEL: 'Linux files',
       MANY_ENTRIES_SELECTED: '$1 items selected',
       MANY_FILES_SELECTED: '$1 files selected',

ui/file_manager/integration_tests/file_manager/my_files.js

@@ -17,7 +17,6 @@ testcase.showMyFiles = function() {
     'My Drive: SubDirectoryItem',
     'Shared with me: SubDirectoryItem',
     'Offline: SubDirectoryItem',
-    'Add new services: MenuItem',
   ];
 
   StepsRunner.run([

ui/file_manager/integration_tests/file_manager/providers.js

@@ -52,19 +52,52 @@ function getSetupSteps(manifest) {
   ];
 }
 
+/**
+ * Returns steps for clicking on the "gear menu".
+ * @return {!Array<function>}
+ */
+function clickGearMenu() {
+  const newServiceMenuItem = '#gear-menu-newservice:not([hidden])';
+  return [
+    // Open the gear menu by clicking the gear button.
+    function() {
+      remoteCall.callRemoteTestUtil(
+          'fakeMouseClick', appId, ['#gear-button'], this.next);
+    },
+    // Wait for Add new service menu item to appear in the gear menu.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'fakeMouseClick failed');
+      remoteCall.waitForElement(appId, newServiceMenuItem).then(this.next);
+    },
+  ];
+}
+
 /**
  * Returns steps for clicking on the "Add new services" menu button.
  * @return {!Array<function>}
  */
-function getClickMenuSteps() {
+function showProvidersMenuSteps() {
+  const newServiceMenuItem = '#gear-menu-newservice:not([hidden])';
   return [
+    // Open the gear menu by clicking the gear button.
     function() {
       remoteCall.callRemoteTestUtil(
-          'fakeMouseClick',
-          appId,
-          ['div[menu=\'#add-new-services-menu\']'],
-          this.next);
-    }
+          'fakeMouseClick', appId, ['#gear-button'], this.next);
+    },
+    // Wait for Add new service menu item to appear.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'fakeMouseClick failed');
+      remoteCall.waitForElement(appId, newServiceMenuItem).then(this.next);
+    },
+    // Click the menu item.
+    function(result) {
+      remoteCall.callRemoteTestUtil(
+          'fakeMouseClick', appId, [newServiceMenuItem], this.next);
+    },
+    function(result) {
+      chrome.test.assertTrue(!!result, 'fakeMouseClick failed');
+      this.next();
+    },
   ];
 }
 
@@ -88,7 +121,7 @@ function getConfirmVolumeSteps(ejectExpected) {
           this.next);
     },
     function(result) {
-      chrome.test.assertTrue(result);
+      chrome.test.assertTrue(!!result, 'fakeMouseClick failed');
       remoteCall.waitForElement(
           appId,
           '.tree-row[selected] .icon[volume-type-icon="provided"]')
@@ -124,16 +157,17 @@ function requestMountInternal(multipleMounts, manifest) {
 
   StepsRunner.runGroups([
     getSetupSteps(manifest),
-    getClickMenuSteps(),
+    showProvidersMenuSteps(),
     [
-      function(result) {
-        chrome.test.assertTrue(result);
+      // Wait for providers menu and new service menu item to appear.
+      function() {
         remoteCall.waitForElement(
             appId,
             '#add-new-services-menu:not([hidden]) cr-menu-item:first-child ' +
                 'span')
             .then(this.next);
       },
+      // Click to install test provider.
       function(result) {
         chrome.test.assertEq(providerName, result.text);
         remoteCall.callRemoteTestUtil(
@@ -142,18 +176,26 @@ function requestMountInternal(multipleMounts, manifest) {
             ['#add-new-services-menu cr-menu-item:first-child span'],
             this.next);
       },
+      function(result) {
+        chrome.test.assertTrue(!!result, 'fakeMouseClick failed');
+        this.next();
+      },
     ],
     getConfirmVolumeSteps(false /* ejectExpected */),
-    getClickMenuSteps(),
+    // If multipleMounts we display the providers menu, otherwise we display the
+    // gear menu and check the "add new service" menu item.
+    multipleMounts ? showProvidersMenuSteps() : clickGearMenu(),
     [
       function() {
-        // When multiple mounts are supported, then the first element of the
-        // menu should stay the same. Otherwise it should disappear from the
-        // list.
+        // When multiple mounts are supported, then the "new service" menu item
+        // should open the providers menu. Otherwise it should go directly to
+        // install-new-extension, however install-new-service command uses
+        // webview which doesn't work in the integration tests.
         var selector = multipleMounts ?
             '#add-new-services-menu:not([hidden]) cr-menu-item:first-child ' +
                 'span' :
-            '#add-new-services-menu:not([hidden]) hr:first-child';
+            '#gear-menu:not([hidden]) ' +
+                'cr-menu-item[command="#install-new-extension"]';
         remoteCall.waitForElement(
             appId,
             selector)
@@ -179,19 +221,18 @@ function requestMountNotInMenuInternal(manifest) {
   StepsRunner.runGroups([
     getSetupSteps(manifest),
     getConfirmVolumeSteps(true /* ejectExpected */),
-    getClickMenuSteps(),
+    clickGearMenu(),
     [
-      function(result) {
-        chrome.test.assertTrue(result);
-        // Confirm that it doesn't show up in the menu.
-        remoteCall.waitForElement(
-            appId,
-            '#add-new-services-menu:not([hidden]) hr:first-child')
-            .then(this.next);
+      function(element) {
+        // clickGearMenu returns the "Add new service" menu item.
+        // Here we only check these attributes because the menu item calls
+        // Webstore using webview which doesn't work in the integration test.
+        chrome.test.assertEq('Install new service', element.text);
+        chrome.test.assertEq(
+            '#install-new-extension', element.attributes.command);
+        this.next();
       },
-    ],
-    [
-      function(result) {
+      function() {
         checkIfNoErrorsOccured(this.next);
       }
     ]