[Reland] Change ActionsController to only update DOM when displaying a menu

Previously ActionsController would calculate the additional actions for
Drive (share, create-shortcut, etc) and FSP [1] like change directory,
selection changed, and metadata updated. When these events arrive in
FilesApp, they update the <command> and <cr-menu> DOM state.

However, that can cause flickering in visible menus and also flakes in
browser tests based on menus, since these events can happen while the
menu is displayed for an entry or set of entries (selection) or affect
entries other than those associated with the menu.

This CL changes ActionsController to store state in an internal map,
rather than in the DOM, and to only update the menu DOM state when a
user-action shows the menu.

Change CommandHandler to set the visibility for Drive actions/commands
synchronously to avoid flickering the size of the menu - it still can
flicker the disabled state and pinned check-mark, but these are softer
to human eyes than the change in size.

Change ToolbarController to update its refresh button disabled state.
The button relied on FileManagerCommands to update the |refresh| state
command, which was not correct because the refresh button always acts
on the current directory, whereas the command was being evaluated for
different entries.

Change Action models and derived classes to have a |getEntries| method
so that ActionsController can get the entries that it is acting upon
to invalidate the state prior to executation.

Change ActionsSubMenu.setActionsModel to forward the target element,
so the <command> can be updated to the correct target element.

[1]
https://developer.chrome.com/apps/fileSystemProvider#event-onGetActionsRequested

Bug: 889153, 875446, 961244
Change-Id: I059f4516f0b64426a7570a88d8eff7e48b435454
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1605383Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658458}

ui/file_manager/file_manager/common/js/BUILD.gn

@@ -179,6 +179,8 @@ js_library("util") {
     "../../../externs/entry_location.js",
     "../../../externs/platform.js",
     "../../../externs/volume_info.js",
+    "../../../externs/volume_info_list.js",
+    "../../../externs/volume_manager.js",
   ]
 }
 

ui/file_manager/file_manager/common/js/util.js

@@ -1549,3 +1549,36 @@ util.entryDebugString = (entry) => {
   }
   return entryDescription;
 };
+
+/**
+ * Returns true if all entries belong to the same volume. If there are no
+ * entries it also returns false.
+ *
+ * @param {!Array<Entry|FilesAppEntry>} entries
+ * @param {!VolumeManager} volumeManager
+ * @return boolean
+ */
+util.isSameVolume = (entries, volumeManager) => {
+  if (!entries.length) {
+    return false;
+  }
+
+  const firstEntry = entries[0];
+  if (!firstEntry) {
+    return false;
+  }
+  const volumeInfo = volumeManager.getVolumeInfo(firstEntry);
+
+  for (let i = 1; i < entries.length; i++) {
+    if (!entries[i]) {
+      return false;
+    }
+    const volumeInfoToCompare = volumeManager.getVolumeInfo(assert(entries[i]));
+    if (!volumeInfoToCompare ||
+        volumeInfoToCompare.volumeId !== volumeInfo.volumeId) {
+      return false;
+    }
+  }
+
+  return true;
+};

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

@@ -23,6 +23,12 @@ class Action {
    * @return {?string}
    */
   getTitle() {}
+
+  /**
+   * Entries that this Action will execute upon.
+   * @return {!Array<!Entry|!FileEntry>}
+   */
+  getEntries() {}
 }
 
 /**
@@ -127,6 +133,11 @@ class DriveShareAction {
   getTitle() {
     return null;
   }
+
+  /** @override */
+  getEntries() {
+    return [this.entry_];
+  }
 }
 
 
@@ -296,6 +307,11 @@ class DriveToggleOfflineAction {
   getTitle() {
     return null;
   }
+
+  /** @override */
+  getEntries() {
+    return this.entries_;
+  }
 }
 
 
@@ -367,6 +383,11 @@ class DriveCreateFolderShortcutAction {
   getTitle() {
     return null;
   }
+
+  /** @override */
+  getEntries() {
+    return [this.entry_];
+  }
 }
 
 
@@ -433,6 +454,11 @@ class DriveRemoveFolderShortcutAction {
   getTitle() {
     return null;
   }
+
+  /** @override */
+  getEntries() {
+    return [this.entry_];
+  }
 }
 
 
@@ -526,6 +552,11 @@ class DriveManageAction {
   getTitle() {
     return null;
   }
+
+  /** @override */
+  getEntries() {
+    return [this.entry_];
+  }
 }
 
 
@@ -595,6 +626,11 @@ class CustomAction {
   getTitle() {
     return this.title_;
   }
+
+  /** @override */
+  getEntries() {
+    return this.entries_;
+  }
 }
 
 /**
@@ -692,21 +728,12 @@ class ActionsModel extends cr.EventTarget {
 
           const volumeInfo = this.entries_.length >= 1 &&
               this.volumeManager_.getVolumeInfo(this.entries_[0]);
-          if (!volumeInfo) {
-            fulfill({});
-            return;
-          }
           // All entries need to be on the same volume to execute ActionsModel
           // commands.
-          // TODO(sashab): Move this to util.js.
-          for (let i = 1; i < this.entries_.length; i++) {
-            const volumeInfoToCompare =
-                this.volumeManager_.getVolumeInfo(this.entries_[i]);
-            if (!volumeInfoToCompare ||
-                volumeInfoToCompare.volumeId != volumeInfo.volumeId) {
-              fulfill({});
-              return;
-            }
+          if (!volumeInfo ||
+              !util.isSameVolume(this.entries_, this.volumeManager_)) {
+            fulfill({});
+            return;
           }
 
           const actions = {};
@@ -837,6 +864,14 @@ class ActionsModel extends cr.EventTarget {
     }
     cr.dispatchSimpleEvent(this, 'invalidated', true);
   }
+
+  /**
+   * @return {!Array<!Entry>}
+   * @public
+   */
+  getEntries() {
+    return this.entries_;
+  }
 }
 
 /**

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

@@ -73,10 +73,6 @@ function GearMenuController(
 GearMenuController.prototype.onShowGearMenu_ = function() {
   this.toggleRipple_.activated = true;
   this.refreshRemainingSpace_(false); /* Without loading caption. */
-
-  // Update view of drive-related settings.
-  this.commandHandler_.updateAvailability();
-
   this.updateNewServiceItem();
 };
 

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

@@ -62,6 +62,14 @@ class ToolbarController {
         queryRequiredElement('#delete', assert(this.toolbar_.ownerDocument)),
         cr.ui.Command);
 
+    /**
+     * @private {!cr.ui.Command}
+     * @const
+     */
+    this.refreshCommand_ = assertInstanceof(
+        queryRequiredElement('#refresh', assert(this.toolbar_.ownerDocument)),
+        cr.ui.Command);
+
     /**
      * @private {!HTMLElement}
      * @const
@@ -105,6 +113,9 @@ class ToolbarController {
     this.navigationList_.addEventListener(
         'relayout', this.onNavigationListRelayout_.bind(this));
 
+    this.directoryModel_.addEventListener(
+        'directory-change', this.updateCurrentDirectoryButtons_.bind(this));
+
     // Watch visibility of toolbar buttons to update the width of location line.
     const observer =
         new MutationObserver(this.onToolbarButtonsMutated_.bind(this));
@@ -117,6 +128,20 @@ class ToolbarController {
     }
   }
 
+  /**
+   * Updates buttons that act on current directory.
+   * @private
+   */
+  updateCurrentDirectoryButtons_() {
+    const volumeInfo = this.directoryModel_.getCurrentVolumeInfo();
+    this.refreshCommand_.disabled = !!volumeInfo && volumeInfo.watchable;
+    console.log(
+        '****** toolbar controller.disabled: ' + this.refreshCommand_.disabled);
+    this.refreshCommand_.setHidden(
+        volumeInfo && volumeInfo.watchable ||
+        this.directoryModel_.getFileListSelection().getCheckSelectMode());
+  }
+
   /**
    * Handles selection's change event to update the UI.
    * @private

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

@@ -38,8 +38,9 @@ class ActionsSubmenu {
 
   /**
    * @param {ActionsModel} actionsModel
+   * @param {Element} element The target element.
    */
-  setActionsModel(actionsModel) {
+  setActionsModel(actionsModel, element) {
     this.items_.forEach(item => {
       item.parentNode.removeChild(item);
     });
@@ -61,7 +62,8 @@ class ActionsSubmenu {
       menuItem.classList.toggle('hide-on-toolbar', true);
       delete remainingActions[ActionsModel.CommonActionId.SHARE];
     }
-    util.queryDecoratedElement('#share', cr.ui.Command).canExecuteChange();
+    util.queryDecoratedElement('#share', cr.ui.Command)
+        .canExecuteChange(element);
 
     // Then add the Manage in Drive item (if available).
     const manageInDriveAction =
@@ -73,13 +75,13 @@ class ActionsSubmenu {
       delete remainingActions[ActionsModel.InternalActionId.MANAGE_IN_DRIVE];
     }
     util.queryDecoratedElement('#manage-in-drive', cr.ui.Command)
-        .canExecuteChange();
+        .canExecuteChange(element);
 
     // Removing shortcuts is not rendered in the submenu to keep the previous
     // behavior. Shortcuts can be removed in the left nav using the roots menu.
     // TODO(mtomasz): Consider rendering the menu item here for consistency.
     util.queryDecoratedElement('#remove-folder-shortcut', cr.ui.Command)
-        .canExecuteChange();
+        .canExecuteChange(element);
 
     // Both save-for-offline and offline-not-necessary are handled by the single
     // #toggle-pinned command.
@@ -99,7 +101,7 @@ class ActionsSubmenu {
       }
     }
     util.queryDecoratedElement('#toggle-pinned', cr.ui.Command)
-        .canExecuteChange();
+        .canExecuteChange(element);
 
     // Process all the rest as custom actions.
     Object.keys(remainingActions).forEach(key => {