crostini: OS settings page for Crostini disk resizing.

Javascript and testing improvements.
Addressing hsuregan and khorimoto's comments on previous CL (see bug).

Fixed: 1077314
Change-Id: I007ab73e223f57f1ab1115fd5ecfc97aaf4db91e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2178555
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Regan Hsu <hsuregan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#765392}

chrome/browser/resources/settings/chromeos/crostini_page/crostini_browser_proxy.js

@@ -152,11 +152,12 @@ cr.define('settings', function() {
 
     /**
      * @param {string} vmName Name of the VM to get disk info for.
-     * @param {boolean} fullInfo Whether to start the VM for full info. Not
-     *     required for the main Crostini pages.
+     * @param {boolean} requestFullInfo Whether to request full disk info, which
+     *     can take several seconds because it requires starting the VM. Set to
+     *     false for the main Crostini pages and true for the resize dialog.
      * @return {!Promise<CrostiniDiskInfo>} The requested information.
      */
-    getCrostiniDiskInfo(vmName, fullInfo) {}
+    getCrostiniDiskInfo(vmName, requestFullInfo) {}
 
     /**
      * Resizes a preallocated user-chosen-size Crostini VM disk to the requested

chrome/browser/resources/settings/chromeos/crostini_page/crostini_disk_resize_dialog.js

@@ -31,16 +31,6 @@ const ResizeState = {
   DONE: 'done',
 };
 
-/**
- * The current confirmation state.
- * @enum {string}
- */
-const ConfirmationState = {
-  NONE: 'none',
-  CONFIRMED: 'confirmed',
-  CANCELED: 'canceled',
-};
-
 Polymer({
   is: 'settings-crostini-disk-resize-dialog',
 
@@ -112,7 +102,7 @@ Polymer({
     // TODO(davidmunro): No magic 'termina' string.
     const vmName = 'termina';
     settings.CrostiniBrowserProxyImpl.getInstance()
-        .getCrostiniDiskInfo(vmName, /* fullInfo = */ true)
+        .getCrostiniDiskInfo(vmName, /*requestFullInfo=*/ true)
         .then(
             diskInfo => {
               if (!diskInfo.succeeded) {
@@ -127,7 +117,6 @@ Polymer({
                 this.minDiskSize_ = diskInfo.ticks[0].label;
                 this.maxDiskSize_ =
                     diskInfo.ticks[diskInfo.ticks.length - 1].label;
-                this.isUserChosenSize_ = diskInfo.isUserChosenSize;
               }
             },
             reason => {

chrome/browser/resources/settings/chromeos/crostini_page/crostini_subpage.js

@@ -12,9 +12,8 @@
  * @enum {string}
  */
 const ConfirmationState = {
-  NONE: 'none',
+  NOT_CONFIRMED: 'notConfirmed',
   CONFIRMED: 'confirmed',
-  CANCELED: 'canceled',
 };
 
 Polymer({
@@ -106,18 +105,6 @@ Polymer({
       value: false,
     },
 
-    /** @private */
-    isDiskUserChosenSize_: {
-      type: Boolean,
-      value: false,
-    },
-
-    /** @private {!ConfirmationState} */
-    diskResizeConfirmationState_: {
-      type: String,
-      value: ConfirmationState.NONE,
-    },
-
     /**
      * Whether the toggle to share the mic with Crostini should be shown.
      * @private {boolean}
@@ -174,6 +161,13 @@ Polymer({
   /** settings.RouteOriginBehavior override */
   route_: settings.routes.CROSTINI_DETAILS,
 
+
+  /** @private {boolean} */
+  isDiskUserChosenSize_: false,
+
+  /** @private {!ConfirmationState} */
+  diskResizeConfirmationState_: ConfirmationState.NOT_CONFIRMED,
+
   observers: [
     'onCrostiniEnabledChanged_(prefs.crostini.enabled.value)',
     'onArcEnabledChanged_(prefs.arc.enabled.value)'
@@ -241,7 +235,7 @@ Polymer({
     // TODO(davidmunro): No magic 'termina' string.
     const vmName = 'termina';
     settings.CrostiniBrowserProxyImpl.getInstance()
-        .getCrostiniDiskInfo(vmName, /*fullInfo = */ false)
+        .getCrostiniDiskInfo(vmName, /*requestFullInfo=*/ false)
         .then(
             diskInfo => {
               if (diskInfo.succeeded) {
@@ -266,17 +260,18 @@ Polymer({
   /** @private */
   onDiskResizeDialogClose_() {
     this.showDiskResizeDialog_ = false;
+    this.diskResizeConfirmationState_ = ConfirmationState.NOT_CONFIRMED;
     // DiskInfo could have changed.
     this.loadDiskInfo_();
   },
 
   /** @private */
   onDiskResizeConfirmationDialogClose_() {
-    this.showDiskResizeConfirmationDialog_ = false;
     // The on_cancel is followed by on_close, so check cancel didn't happen
     // first.
-    if (this.diskResizeConfirmationState_ !== ConfirmationState.CANCELED) {
+    if (this.showDiskResizeConfirmationDialog_) {
       this.diskResizeConfirmationState_ = ConfirmationState.CONFIRMED;
+      this.showDiskResizeConfirmationDialog_ = false;
       this.showDiskResizeDialog_ = true;
     }
   },
@@ -284,7 +279,6 @@ Polymer({
   /** @private */
   onDiskResizeConfirmationDialogCancel_() {
     this.showDiskResizeConfirmationDialog_ = false;
-    this.diskResizeConfirmationState_ = ConfirmationState.CANCELED;
   },
 
   /**

chrome/test/data/webui/settings/chromeos/crostini_page_test.js

@@ -686,34 +686,56 @@ suite('CrostiniPageTests', function() {
       });
 
       test('DiskResizeConfirmationDialogShownAndAccepted', async function() {
+        await crostiniBrowserProxy.resolvePromise(
+            'getCrostiniDiskInfo', sparseDiskData);
         await clickShowDiskResize(false);
         // Dismiss confirmation.
-        const confirmationDialog =
+        let confirmationDialog =
             subpage.$$('settings-crostini-disk-resize-confirmation-dialog');
         assertTrue(isVisible(confirmationDialog.$$('#continue')));
         assertTrue(isVisible(confirmationDialog.$$('#cancel')));
         confirmationDialog.$$('#continue').click();
+        await test_util.eventToPromise('close', confirmationDialog);
+        assertFalse(isVisible(confirmationDialog));
 
-        await crostiniBrowserProxy.resolvePromise(
-            'getCrostiniDiskInfo', sparseDiskData);
         dialog = subpage.$$('settings-crostini-disk-resize-dialog');
-        // TODO(nverne): Find out how to wait long enough for the dialog to
-        // show.
+        assertTrue(!!dialog);
+        assertTrue(isVisible(dialog.$$('#resize')));
+        assertTrue(isVisible(dialog.$$('#cancel')));
+
+        // Cancel main resize dialog.
+        dialog.$$('#cancel').click();
+        await test_util.eventToPromise('close', dialog);
+        assertFalse(isVisible(dialog));
+
+        // On another click, confirmation dialog should be shown again.
+        await clickShowDiskResize(false);
+        confirmationDialog =
+            subpage.$$('settings-crostini-disk-resize-confirmation-dialog');
+        assertTrue(isVisible(confirmationDialog.$$('#continue')));
+        confirmationDialog.$$('#continue').click();
+        await test_util.eventToPromise('close', confirmationDialog);
+
+        // Main dialog should show again.
+        dialog = subpage.$$('settings-crostini-disk-resize-dialog');
+        assertTrue(!!dialog);
+        assertTrue(isVisible(dialog.$$('#resize')));
+        assertTrue(isVisible(dialog.$$('#cancel')));
       });
 
       test('DiskResizeConfirmationDialogShownAndCanceled', async function() {
         await crostiniBrowserProxy.resolvePromise(
             'getCrostiniDiskInfo', sparseDiskData);
         await clickShowDiskResize(false);
+
         const confirmationDialog =
             subpage.$$('settings-crostini-disk-resize-confirmation-dialog');
         assertTrue(isVisible(confirmationDialog.$$('#continue')));
         assertTrue(isVisible(confirmationDialog.$$('#cancel')));
         confirmationDialog.$$('#cancel').click();
-        await flushAsync();
+        await test_util.eventToPromise('close', confirmationDialog);
 
-        dialog = subpage.$$('settings-crostini-disk-resize-dialog');
-        assertFalse(!!dialog);
+        assertFalse(!!subpage.$$('settings-crostini-disk-resize-dialog'));
       });
     });
   });

chrome/test/data/webui/settings/chromeos/test_crostini_browser_proxy.js

@@ -150,8 +150,8 @@ class TestCrostiniBrowserProxy extends TestBrowserProxy {
   }
 
   /** @override */
-  getCrostiniDiskInfo(vmName, fullInfo) {
-    this.methodCalled('getCrostiniDiskInfo', vmName, fullInfo);
+  getCrostiniDiskInfo(vmName, requestFullInfo) {
+    this.methodCalled('getCrostiniDiskInfo', vmName, requestFullInfo);
     return this.getNewPromiseFor('getCrostiniDiskInfo');
   }