Reland "Add CrostiniController class to manage foreground tasks"

This reverts commit 159ca7c4.

Reason for revert: fixed build script error

Original change's description:
> Revert "Add CrostiniController class to manage foreground tasks"
>
> This reverts commit b2e67312.
>
> Reason for revert: Caused compile failure on Linux ChromiumOS MSan Builder: https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Builder/19874
>
> Original change's description:
> > Add CrostiniController class to manage foreground tasks
> >
> > This is a refactor to move code out of file_manager.js and
> > into crostini_controller.js in preparation to add more features
> > for showing messages.
> >
> > Updated UI tests to use FILES_NG_ENABLED and work with all files
> > that import from chrome://resources such as files_message.html.
> >
> > Bug: 1095889
> > Change-Id: I9c9f34304e26dca8df4c0aa64a51c124650385b9
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2259873
> > Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
> > Reviewed-by: Joel Hockey <joelhockey@chromium.org>
> > Commit-Queue: Joel Hockey <joelhockey@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#781773}
>
> TBR=joelhockey@chromium.org,lucmult@chromium.org
>
> Change-Id: I91a7bd3837cf2ab2108ba672c36b3e5a5d5c743c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1095889
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2263833
> Reviewed-by: Bret Sepulveda <bsep@chromium.org>
> Commit-Queue: Bret Sepulveda <bsep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#781810}

TBR=bsep@chromium.org,joelhockey@chromium.org,lucmult@chromium.org

Bug: 1095889
Change-Id: I83b5dd3369e186ccffa29a63f25de370b73d7e12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264086Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782098}

ui/file_manager/file_manager/foreground/js/BUILD.gn

@@ -30,6 +30,7 @@ js_type_check("closure_compile_module") {
     ":closure_compile_externs",
     ":column_visibility_controller",
     ":constants",
+    ":crostini_controller",
     ":dialog_action_controller",
     ":dialog_type",
     ":directory_contents",
@@ -233,6 +234,10 @@ js_library("column_visibility_controller") {
 js_library("constants") {
 }
 
+js_library("crostini_controller") {
+  deps = [ "ui:directory_tree" ]
+}
+
 js_library("dialog_action_controller") {
   deps = [
     ":dialog_type",
@@ -329,6 +334,7 @@ js_library("file_manager") {
     ":android_app_list_model",
     ":app_state_controller",
     ":column_visibility_controller",
+    ":crostini_controller",
     ":dialog_action_controller",
     ":dialog_type",
     ":directory_model",

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

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/**
+ * CrostiniController handles the foreground UI relating to crostini.
+ */
+class CrostiniController {
+  /**
+   * @param {!Crostini} crostini Crostini background object.
+   * @param {!DirectoryTree} directoryTree DirectoryTree.
+   */
+  constructor(crostini, directoryTree) {
+    /** @private @const */
+    this.crostini_ = crostini;
+
+    /** @private @const */
+    this.directoryTree_ = directoryTree;
+
+    /** @private */
+    this.entrySharedWithCrostini_ = false;
+
+    /** @private */
+    this.entrySharedWithPluginVm_ = false;
+  }
+
+  /**
+   * Refresh the Linux files item at startup and when crostini enabled changes.
+   */
+  async redraw() {
+    // Setup Linux files fake root.
+    this.directoryTree_.dataModel.linuxFilesItem =
+        this.crostini_.isEnabled(constants.DEFAULT_CROSTINI_VM) ?
+        new NavigationModelFakeItem(
+            str('LINUX_FILES_ROOT_LABEL'), NavigationModelItemType.CROSTINI,
+            new FakeEntry(
+                str('LINUX_FILES_ROOT_LABEL'),
+                VolumeManagerCommon.RootType.CROSTINI)) :
+        null;
+    // Redraw the tree to ensure 'Linux files' is added/removed.
+    this.directoryTree_.redraw(false);
+  }
+
+  /**
+   * Load the list of shared paths and show a toast if this is the first time
+   * that FilesApp is loaded since login.
+   *
+   * @param {boolean} maybeShowToast if true, show toast if this is the first
+   *     time FilesApp is opened since login.
+   * @param {!FilesToast} filesToast
+   */
+  async loadSharedPaths(maybeShowToast, filesToast) {
+    let showToast = maybeShowToast;
+    const getSharedPaths = async (vmName) => {
+      if (!this.crostini_.isEnabled(vmName)) {
+        return 0;
+      }
+
+      return new Promise(resolve => {
+        chrome.fileManagerPrivate.getCrostiniSharedPaths(
+            maybeShowToast, vmName, (entries, firstForSession) => {
+              showToast = showToast && firstForSession;
+              for (const entry of entries) {
+                this.crostini_.registerSharedPath(vmName, assert(entry));
+              }
+              resolve(entries.length);
+            });
+      });
+    };
+
+    const toast = (count, msgSingle, msgPlural, action, subPage, umaItem) => {
+      if (!showToast || count == 0) {
+        return;
+      }
+      filesToast.show(count == 1 ? str(msgSingle) : strf(msgPlural, count), {
+        text: str(action),
+        callback: () => {
+          chrome.fileManagerPrivate.openSettingsSubpage(subPage);
+          CommandHandler.recordMenuItemSelected(umaItem);
+        }
+      });
+    };
+
+    const [crostiniShareCount, pluginVmShareCount] = await Promise.all([
+      getSharedPaths(constants.DEFAULT_CROSTINI_VM),
+      getSharedPaths(constants.PLUGIN_VM)
+    ]);
+
+    toast(
+        crostiniShareCount, 'FOLDER_SHARED_WITH_CROSTINI',
+        'FOLDER_SHARED_WITH_CROSTINI_PLURAL', 'MANAGE_TOAST_BUTTON_LABEL',
+        'crostini/sharedPaths',
+        CommandHandler.MenuCommandsForUMA.MANAGE_LINUX_SHARING_TOAST_STARTUP);
+    // TODO(crbug.com/949356): UX to provide guidance for what to do
+    // when we have shared paths with both Linux and Plugin VM.
+    toast(
+        pluginVmShareCount, 'FOLDER_SHARED_WITH_PLUGIN_VM',
+        'FOLDER_SHARED_WITH_PLUGIN_VM_PLURAL', 'MANAGE_TOAST_BUTTON_LABEL',
+        'app-management/pluginVm/sharedPaths',
+        CommandHandler.MenuCommandsForUMA
+            .MANAGE_PLUGIN_VM_SHARING_TOAST_STARTUP);
+  }
+}

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

@@ -29,6 +29,9 @@ class FileManager extends cr.EventTarget {
     /** @private {?Crostini} */
     this.crostini_ = null;
 
+    /** @private {?CrostiniController} */
+    this.crostiniController_ = null;
+
     /**
      * ImportHistory. Non-null only once history observer is added in
      * {@code addHistoryObserver}.
@@ -1161,81 +1164,13 @@ class FileManager extends cr.EventTarget {
     // multiple VMs.
     chrome.fileManagerPrivate.onCrostiniChanged.addListener(
         this.onCrostiniChanged_.bind(this));
-    return this.setupCrostini_();
-  }
-
-  /**
-   * Sets up Crostini 'Linux files'.
-   * @return {!Promise<void>}
-   * @private
-   */
-  async setupCrostini_() {
-    // Setup Linux files fake root.
-    this.directoryTree.dataModel.linuxFilesItem =
-        this.crostini_.isEnabled(constants.DEFAULT_CROSTINI_VM) ?
-        new NavigationModelFakeItem(
-            str('LINUX_FILES_ROOT_LABEL'), NavigationModelItemType.CROSTINI,
-            new FakeEntry(
-                str('LINUX_FILES_ROOT_LABEL'),
-                VolumeManagerCommon.RootType.CROSTINI)) :
-        null;
-    // Redraw the tree to ensure 'Linux files' is added/removed.
-    this.directoryTree.redraw(false);
-
-    // Load any existing shared paths.
-    // Only observe firstForSession when using full-page FilesApp.
-    // I.e., don't show toast in a dialog.
-    let showToast = false;
-    const getSharedPaths = async (vmName) => {
-      if (!this.crostini_.isEnabled(vmName)) {
-        return 0;
-      }
-
-      return new Promise(resolve => {
-        chrome.fileManagerPrivate.getCrostiniSharedPaths(
-            this.dialogType === DialogType.FULL_PAGE, vmName,
-            (entries, firstForSession) => {
-              showToast = showToast || firstForSession;
-              for (const entry of entries) {
-                this.crostini_.registerSharedPath(vmName, assert(entry));
-              }
-              resolve(entries.length);
-            });
-      });
-    };
-
-    const toast = (count, msgSingle, msgPlural, action, subPage, umaItem) => {
-      if (!showToast || count == 0) {
-        return;
-      }
-      this.ui_.toast.show(
-          count == 1 ? str(msgSingle) : strf(msgPlural, count), {
-            text: str(action),
-            callback: () => {
-              chrome.fileManagerPrivate.openSettingsSubpage(subPage);
-              CommandHandler.recordMenuItemSelected(umaItem);
-            }
-          });
-    };
-
-    const [crostiniShareCount, pluginVmShareCount] = await Promise.all([
-      getSharedPaths(constants.DEFAULT_CROSTINI_VM),
-      getSharedPaths(constants.PLUGIN_VM)
-    ]);
-
-    toast(
-        crostiniShareCount, 'FOLDER_SHARED_WITH_CROSTINI',
-        'FOLDER_SHARED_WITH_CROSTINI_PLURAL', 'MANAGE_TOAST_BUTTON_LABEL',
-        'crostini/sharedPaths',
-        CommandHandler.MenuCommandsForUMA.MANAGE_LINUX_SHARING_TOAST_STARTUP);
-    // TODO(crbug.com/949356): UX to provide guidance for what to do
-    // when we have shared paths with both Linux and Plugin VM.
-    toast(
-        pluginVmShareCount, 'FOLDER_SHARED_WITH_PLUGIN_VM',
-        'FOLDER_SHARED_WITH_PLUGIN_VM_PLURAL', 'MANAGE_TOAST_BUTTON_LABEL',
-        'app-management/pluginVm/sharedPaths',
-        CommandHandler.MenuCommandsForUMA
-            .MANAGE_PLUGIN_VM_SHARING_TOAST_STARTUP);
+    this.crostiniController_ = new CrostiniController(
+        assert(this.crostini_), assert(this.directoryTree));
+    await this.crostiniController_.redraw();
+    // Never show toast in an open-file dialog.
+    const maybeShowToast = this.dialogType === DialogType.FULL_PAGE;
+    return this.crostiniController_.loadSharedPaths(
+        maybeShowToast, this.ui_.toast);
   }
 
   /**
@@ -1255,11 +1190,11 @@ class FileManager extends cr.EventTarget {
     switch (event.eventType) {
       case chrome.fileManagerPrivate.CrostiniEventType.ENABLE:
         this.crostini_.setEnabled(event.vmName, true);
-        return this.setupCrostini_();
+        return this.crostiniController_.redraw();
 
       case chrome.fileManagerPrivate.CrostiniEventType.DISABLE:
         this.crostini_.setEnabled(event.vmName, false);
-        return this.setupCrostini_();
+        return this.crostiniController_.redraw();
     }
   }
 

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

@@ -108,6 +108,7 @@
 // <include src="android_app_list_model.js">
 // <include src="app_state_controller.js">
 // <include src="column_visibility_controller.js">
+// <include src="crostini_controller.js">
 // <include src="dialog_action_controller.js">
 // <include src="dialog_type.js">
 // <include src="directory_contents.js">

ui/file_manager/file_manager/test/BUILD.gn

@@ -19,7 +19,6 @@ action("create_test_main") {
     "//ui/file_manager/file_manager/foreground/js/elements_importer.js",
     "//ui/file_manager/file_manager/foreground/js/main_scripts.js",
     "//ui/file_manager/file_manager/main.html",
-    "//ui/webui/resources/css/text_defaults.css",
     "check_select.js",
     "crostini_mount.js",
     "crostini_share.js",

ui/file_manager/file_manager/test/crostini_mount.js

@@ -32,7 +32,7 @@ crostiniMount.testMountCrostiniSuccess = async (done) => {
 
   // Click on Linux files.
   assertTrue(test.fakeMouseClick(fakeRoot, 'click linux files'));
-  await test.waitForElement('paper-progress:not([hidden])');
+  await test.waitForElement('files-spinner:not([hidden])');
 
   // Ensure mountCrostini is called.
   await test.repeatUntil(() => {

ui/file_manager/file_manager/test/js/chrome_file_manager_private_test_impl.js

@@ -22,17 +22,10 @@ chrome.fileManagerPrivate = {
     SHARE: 'share',
     UNSHARE: 'unshare',
   },
-  Verb: {
-    OPEN_WITH: 'open_with',
-    ADD_TO: 'add_to',
-    PACK_WITH: 'pack_with',
-    SHARE_WITH: 'share_with',
-  },
-  SearchType: {
-    ALL: 'ALL',
-    SHARED_WITH_ME: 'SHARED_WITH_ME',
-    EXCLUDE_DIRECTORIES: 'EXCLUDE_DIRECTORIES',
-    OFFLINE: 'OFFLINE',
+  FormatFileSystemType: {
+    VFAT: 'vfat',
+    EXFAT: 'exfat',
+    NTFS: 'ntfs',
   },
   DriveConnectionStateType: {
     ONLINE: 'ONLINE',
@@ -44,6 +37,18 @@ chrome.fileManagerPrivate = {
     NO_NETWORK: 'NO_NETWORK',
     NO_SERVICE: 'NO_SERVICE',
   },
+  SearchType: {
+    ALL: 'ALL',
+    SHARED_WITH_ME: 'SHARED_WITH_ME',
+    EXCLUDE_DIRECTORIES: 'EXCLUDE_DIRECTORIES',
+    OFFLINE: 'OFFLINE',
+  },
+  Verb: {
+    OPEN_WITH: 'open_with',
+    ADD_TO: 'add_to',
+    PACK_WITH: 'pack_with',
+    SHARE_WITH: 'share_with',
+  },
   currentId_: 'test@example.com',
   displayedId_: 'test@example.com',
   preferences_: {

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

@@ -10,7 +10,7 @@ loadTimeData.data = $GRDP;
 // Extend with additional fields not found in grdp files.
 loadTimeData.overrideValues({
   'CROSTINI_ENABLED': true,
-  'FILES_NG_ENABLED': false,
+  'FILES_NG_ENABLED': true,
   'FILES_TRANSFER_DETAILS_ENABLED': false,
   'FEEDBACK_PANEL_ENABLED': false,
   'GOOGLE_DRIVE_REDEEM_URL': 'http://www.google.com/intl/en/chrome/devices' +

ui/file_manager/file_manager/test/scripts/create_test_main.py

@@ -12,6 +12,7 @@ as a regular web page in a single renderer.
 
 
 import argparse
+import glob
 import json
 import os
 import re
@@ -34,13 +35,11 @@ output = args.output or os.path.abspath(
 # SRC   : Absolute path to //src/.
 # GEN   : Absolute path to $target_gen_dir.
 # ROOT  : Relative path from GEN to //src/ui/file_manager/file_manager.
-# CC_SRC: Source directory for components-chromium.
-# CC_GEN: Directory where components-chromium is copied to.
+# R_GEN : Directory where chrome://resources/ is copied to.
 SRC = os.path.abspath(os.path.join(sys.path[0], '../../../../..')) + '/'
 GEN = os.path.dirname(os.path.abspath(args.output)) + '/'
 ROOT = os.path.relpath(SRC, GEN) + '/ui/file_manager/file_manager/'
-CC_SRC = 'third_party/polymer/v1_0/components-chromium/'
-CC_GEN = 'test/gen/cc/'
+R_GEN = 'test/gen/resources/'
 scripts = []
 GENERATED = 'Generated at %s by: %s' % (time.ctime(), sys.path[0])
 GENERATED_HTML = '<!-- %s -->\n\n' % GENERATED
@@ -122,25 +121,33 @@ def i18n(template):
   repl = lambda x: strings.get(x.group(1), x.group())
   return re.sub(r'\$i18n(?:Raw)?\{(.*?)\}', repl, template)
 
+
+# Copy tree from src_dir to dst_dir with substitutions.
+def copytree(src_dir, dst_dir):
+  for root, _, files in os.walk(SRC + src_dir):
+    for f in files:
+      srcf = os.path.join(root[len(SRC):], f)
+      dstf = dst_dir + srcf[len(src_dir):]
+      write(dstf, i18n(read(srcf).replace('chrome://resources/', GEN + R_GEN)))
+
+# Copy any files required in chrome://resources/... into test/gen/resources.
+copytree('ui/webui/resources/', R_GEN)
+copytree('third_party/polymer/v1_0/components-chromium/',
+         R_GEN + 'polymer/v1_0/')
+shutil.rmtree(GEN + R_GEN + 'polymer/v1_0/polymer')
+os.rename(GEN + R_GEN + 'polymer/v1_0/polymer2',
+          GEN + R_GEN + 'polymer/v1_0/polymer')
+for css in glob.glob(GEN + '../../webui/resources/css/*.css'):
+  shutil.copy(css, GEN + R_GEN + 'css/')
+
 # Substitute $i18n{}.
-# Update relative paths.
-# Fix link to action_link.css and text_defaults.css.
-# Fix stylesheet from extension.
+# Update relative paths, and paths to chrome://resources/.
 main_html = (i18n(read('ui/file_manager/file_manager/main.html'))
-             .replace('chrome://resources/polymer/v1_0/', '../../../' + CC_SRC)
-             .replace('chrome://resources/css/action_link.css',
-                      '../../webui/resources/css/action_link.css')
              .replace('href="', 'href="' + ROOT)
              .replace('src="', 'src="' + ROOT)
-             .replace(ROOT + 'chrome://resources/html/', CC_GEN + 'polymer/')
-             .replace(ROOT + 'chrome://resources/css/text_defaults.css',
-                      'test/gen/css/text_defaults.css')
+             .replace(ROOT + 'chrome://resources/', R_GEN)
              .split('\n'))
 
-# Fix text_defaults.css.  Copy and replace placeholders.
-text_defaults = i18n(read('ui/webui/resources/css/text_defaults.css'))
-write('test/gen/css/text_defaults.css', text_defaults)
-
 # Add scripts required for testing, and the test files (test/*.js).
 src = [
     'test/js/chrome_api_test_impl.js',
@@ -193,15 +200,6 @@ main_html = replaceline(main_html, 'foreground/js/main_scripts.js', [
 def elements_path(elements_filename):
   return '../../../../%sforeground/elements/%s' % (ROOT, elements_filename)
 
-# Copy all files from //third_party/polymer/v1_0/components-chromium/ into
-# test/gen/cc.  Rename polymer2 to polymer.
-gen_cc = GEN + CC_GEN
-if os.path.exists(gen_cc):
-  shutil.rmtree(gen_cc)
-shutil.copytree(SRC + CC_SRC, gen_cc)
-shutil.rmtree(gen_cc + 'polymer')
-os.rename(gen_cc + 'polymer2', gen_cc + 'polymer')
-
 # Generate strings.js
 # Copy all html files in foreground/elements and fix references to polymer
 # Load QuickView in iframe rather than webview.
@@ -209,13 +207,13 @@ for filename, substitutions in (
     ('test/js/strings.js', (
         ('$GRDP', json.dumps(strings, sort_keys=True, indent=2)),
     )),
-    ('foreground/elements/elements_bundle.html', (
-        ('="files_xf', '="' + elements_path('files_xf')),
-    )),
+    ('foreground/elements/elements_bundle.html', ()),
     ('foreground/js/elements_importer.js', (
         ("= 'foreground", "= 'test/gen/foreground"),
     )),
+    ('foreground/elements/files_format_dialog.html', ()),
     ('foreground/elements/files_icon_button.html', ()),
+    ('foreground/elements/files_message.html', ()),
     ('foreground/elements/files_metadata_box.html', ()),
     ('foreground/elements/files_metadata_entry.html', ()),
     ('foreground/elements/files_quick_view.html', (
@@ -235,23 +233,19 @@ for filename, substitutions in (
          ('this.webview_.contentWindow.content.type = this.type;'
           'this.webview_.contentWindow.content.src = this.src;')),
     )),
+    ('foreground/elements/files_spinner.html', ()),
     ('foreground/elements/files_toast.html', ()),
     ('foreground/elements/files_toggle_ripple.html', ()),
     ('foreground/elements/files_tooltip.html', ()),
+    ('foreground/elements/files_xf_elements.html', (
+        ('src="xf_',
+         'src="%sui/file_manager/file_manager/foreground/elements/xf_' % SRC),
+    )),
     ('foreground/elements/icons.html', ()),
     ):
   buf = i18n(read('ui/file_manager/file_manager/' + filename))
-  buf = buf.replace('chrome://resources/html/', '../../cc/polymer/')
-  buf = buf.replace('chrome://resources/polymer/v1_0/', '../../cc/')
-  buf = buf.replace('<link rel="import" href="chrome://resources/cr_elements/'
-                    'cr_input/cr_input.html">', '')
-  buf = buf.replace('<link rel="import" href="chrome://resources/cr_elements/'
-                    'cr_button/cr_button.html">', '')
+  buf = buf.replace('chrome://resources/', '../../resources/')
   buf = buf.replace('src="files_', 'src="' + elements_path('files_'))
-  # The files_format_dialog and files_message import various files that are
-  # not available in a ui test, just ignore them completely.
-  buf = buf.replace('<link rel="import" href="files_format_dialog.html">', '')
-  buf = buf.replace('<link rel="import" href="files_message.html">', '')
   for old, new in substitutions:
     buf = buf.replace(old, new)
   write('test/gen/' + filename, buf)