crostini: disable export import buttons when operation in progress

BUG=985192

Change-Id: I15da8fd02164d3c2089c4dddc24b48171577cde8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1728171Reviewed-by: Dan Beam <dbeam@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Julian Watson <juwa@google.com>
Cr-Commit-Position: refs/heads/master@{#683408}

chrome/browser/chromeos/crostini/crostini_export_import.cc

@@ -175,6 +175,9 @@ void CrostiniExportImport::Start(
     return;
   } else {
     notifications_.emplace_hint(it, container_id, notification);
+    for (auto& observer : observers_) {
+      observer.OnCrostiniExportImportOperationStatusChanged(true);
+    }
   }
 
   switch (type) {
@@ -504,6 +507,9 @@ CrostiniExportImportNotification& CrostiniExportImport::RemoveNotification(
   DCHECK(it != notifications_.end());
   auto& notification = *it->second;
   notifications_.erase(it);
+  for (auto& observer : observers_) {
+    observer.OnCrostiniExportImportOperationStatusChanged(false);
+  }
   return notification;
 }
 
@@ -532,6 +538,11 @@ void CrostiniExportImport::CancelOperation(ExportImportType type,
   }
 }
 
+bool CrostiniExportImport::GetExportImportOperationStatus() const {
+  ContainerId id(kCrostiniDefaultVmName, kCrostiniDefaultContainerName);
+  return notifications_.find(id) != notifications_.end();
+}
+
 CrostiniExportImportNotification*
 CrostiniExportImport::GetNotificationForTesting(ContainerId container_id) {
   auto it = notifications_.find(container_id);

chrome/browser/chromeos/crostini/crostini_export_import.h

@@ -60,11 +60,25 @@ class CrostiniExportImport : public KeyedService,
                              public crostini::ExportContainerProgressObserver,
                              public crostini::ImportContainerProgressObserver {
  public:
+  class Observer : public base::CheckedObserver {
+   public:
+    // Called immediately before operation begins with |in_progress|=true, and
+    // again immediately after the operation completes with |in_progress|=false.
+    virtual void OnCrostiniExportImportOperationStatusChanged(
+        bool in_progress) = 0;
+  };
+
   static CrostiniExportImport* GetForProfile(Profile* profile);
 
   explicit CrostiniExportImport(Profile* profile);
   ~CrostiniExportImport() override;
 
+  void AddObserver(Observer* observer) { observers_.AddObserver(observer); }
+
+  void RemoveObserver(Observer* observer) {
+    observers_.RemoveObserver(observer);
+  }
+
   // KeyedService:
   void Shutdown() override;
 
@@ -82,9 +96,12 @@ class CrostiniExportImport : public KeyedService,
                        base::FilePath path,
                        CrostiniManager::CrostiniResultCallback callback);
 
-  // Cancel currently running export/import
+  // Cancel currently running export/import.
   void CancelOperation(ExportImportType type, ContainerId id);
 
+  // Whether an export or import is currently in progress.
+  bool GetExportImportOperationStatus() const;
+
   CrostiniExportImportNotification* GetNotificationForTesting(
       ContainerId container_id);
 
@@ -173,6 +190,7 @@ class CrostiniExportImport : public KeyedService,
   // Notifications must have unique-per-profile identifiers.
   // A non-static member on a profile-keyed-service will suffice.
   int next_notification_id_;
+  base::ObserverList<Observer> observers_;
   // weak_ptr_factory_ should always be last member.
   base::WeakPtrFactory<CrostiniExportImport> weak_ptr_factory_;
 

chrome/browser/resources/settings/crostini_page/BUILD.gn

@@ -24,6 +24,7 @@ js_library("crostini_browser_proxy") {
 js_library("crostini_export_import") {
   deps = [
     ":crostini_browser_proxy",
+    "//ui/webui/resources/js:web_ui_listener_behavior",
   ]
 }
 

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

@@ -51,14 +51,26 @@ cr.define('settings', function() {
      */
     removeCrostiniSharedPath(vmName, path) {}
 
-    /* Request chrome send a crostini-installer-status-changed event with the
-    current installer status */
+    /**
+     * Request chrome send a crostini-installer-status-changed event with the
+     * current installer status
+     */
     requestCrostiniInstallerStatus() {}
 
-    /* Export crostini container. */
+    /**
+     * Request chrome send a crostini-export-import-operation-status-changed
+     * event with the current operation status
+     */
+    requestCrostiniExportImportOperationStatus() {}
+
+    /**
+     * Export crostini container.
+     */
     exportCrostiniContainer() {}
 
-    /* Import crostini container. */
+    /**
+     * Import crostini container.
+     */
     importCrostiniContainer() {}
   }
 
@@ -99,6 +111,11 @@ cr.define('settings', function() {
       chrome.send('requestCrostiniInstallerStatus');
     }
 
+    /** @override */
+    requestCrostiniExportImportOperationStatus() {
+      chrome.send('requestCrostiniExportImportOperationStatus');
+    }
+
     /** @override */
     exportCrostiniContainer() {
       chrome.send('exportCrostiniContainer');

chrome/browser/resources/settings/crostini_page/crostini_export_import.html

@@ -2,6 +2,7 @@
 
 <link rel="import" href="chrome://resources/cr_elements/cr_button/cr_button.html">
 <link rel="import" href="chrome://resources/html/i18n_behavior.html">
+<link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
 <link rel="import" href="crostini_browser_proxy.html">
 <link rel="import" href="crostini_import_confirmation_dialog.html">
 <link rel="import" href="../i18n_setup.html">
@@ -15,18 +16,18 @@
         <div id="exportCrostiniLabel" class="start secondary">
           $i18n{crostiniExportLabel}
         </div>
-        <cr-button on-click="onExportClick_"
-          aria-labelledby="exportCrostiniLabel">
-           $i18n{crostiniExport}
+        <cr-button on-click="onExportClick_" disabled="[[!enableButtons_]]"
+            aria-labelledby="exportCrostiniLabel">
+          $i18n{crostiniExport}
         </cr-button>
       </div>
       <div id="import" class="list-item">
         <div id="importCrostiniLabel" class="start secondary">
           $i18n{crostiniImportLabel}
         </div>
-        <cr-button on-click="onImportClick_"
-          aria-labelledby="importCrostiniLabel">
-           $i18n{crostiniImport}
+        <cr-button on-click="onImportClick_" disabled="[[!enableButtons_]]"
+            aria-labelledby="importCrostiniLabel">
+          $i18n{crostiniImport}
         </cr-button>
       </div>
     </div>

chrome/browser/resources/settings/crostini_page/crostini_export_import.js

@@ -11,12 +11,33 @@
 Polymer({
   is: 'settings-crostini-export-import',
 
+  behaviors: [WebUIListenerBehavior],
+
   properties: {
     /** @private */
     showImportConfirmationDialog_: {
       type: Boolean,
       value: false,
     },
+
+    /**
+     * Whether the export import buttons should be enabled. Initially false
+     * until status has been confirmed.
+     * @private {boolean}
+     */
+    enableButtons_: {
+      type: Boolean,
+      value: false,
+    },
+  },
+
+  attached: function() {
+    this.addWebUIListener(
+        'crostini-export-import-operation-status-changed', inProgress => {
+          this.enableButtons_ = !inProgress;
+        });
+    settings.CrostiniBrowserProxyImpl.getInstance()
+        .requestCrostiniExportImportOperationStatus();
   },
 
   /** @private */

chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc

@@ -6,12 +6,9 @@
 
 #include <string>
 #include <utility>
-#include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "chrome/browser/chromeos/crostini/crostini_export_import.h"
-#include "chrome/browser/chromeos/crostini/crostini_manager.h"
 #include "chrome/browser/chromeos/crostini/crostini_util.h"
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/chromeos/guest_os/guest_os_share_path.h"
@@ -67,6 +64,11 @@ void CrostiniHandler::RegisterMessages() {
       base::BindRepeating(
           &CrostiniHandler::HandleCrostiniInstallerStatusRequest,
           weak_ptr_factory_.GetWeakPtr()));
+  web_ui()->RegisterMessageCallback(
+      "requestCrostiniExportImportOperationStatus",
+      base::BindRepeating(
+          &CrostiniHandler::HandleCrostiniExportImportOperationStatusRequest,
+          weak_ptr_factory_.GetWeakPtr()));
 }
 
 void CrostiniHandler::OnJavascriptAllowed() {
@@ -75,6 +77,7 @@ void CrostiniHandler::OnJavascriptAllowed() {
   if (chromeos::CrosUsbDetector::Get()) {
     chromeos::CrosUsbDetector::Get()->AddUsbDeviceObserver(this);
   }
+  crostini::CrostiniExportImport::GetForProfile(profile_)->AddObserver(this);
 }
 
 void CrostiniHandler::OnJavascriptDisallowed() {
@@ -86,6 +89,7 @@ void CrostiniHandler::OnJavascriptDisallowed() {
   if (chromeos::CrosUsbDetector::Get()) {
     chromeos::CrosUsbDetector::Get()->RemoveUsbDeviceObserver(this);
   }
+  crostini::CrostiniExportImport::GetForProfile(profile_)->RemoveObserver(this);
 }
 
 void CrostiniHandler::HandleRequestCrostiniInstallerView(
@@ -225,6 +229,15 @@ void CrostiniHandler::HandleCrostiniInstallerStatusRequest(
   OnCrostiniInstallerViewStatusChanged(status);
 }
 
+void CrostiniHandler::HandleCrostiniExportImportOperationStatusRequest(
+    const base::ListValue* args) {
+  AllowJavascript();
+  CHECK_EQ(0U, args->GetSize());
+  bool in_progress = crostini::CrostiniExportImport::GetForProfile(profile_)
+                         ->GetExportImportOperationStatus();
+  OnCrostiniExportImportOperationStatusChanged(in_progress);
+}
+
 void CrostiniHandler::OnCrostiniInstallerViewStatusChanged(bool status) {
   // It's technically possible for this to be called before Javascript is
   // enabled, in which case we must not call FireWebUIListener
@@ -234,5 +247,12 @@ void CrostiniHandler::OnCrostiniInstallerViewStatusChanged(bool status) {
   }
 }
 
+void CrostiniHandler::OnCrostiniExportImportOperationStatusChanged(
+    bool in_progress) {
+  // Other side listens with cr.addWebUIListener
+  FireWebUIListener("crostini-export-import-operation-status-changed",
+                    base::Value(in_progress));
+}
+
 }  // namespace settings
 }  // namespace chromeos

chrome/browser/ui/webui/settings/chromeos/crostini_handler.h

@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/chromeos/crostini/crostini_export_import.h"
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
 #include "chrome/browser/chromeos/usb/cros_usb_detector.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
@@ -23,6 +24,7 @@ namespace settings {
 
 class CrostiniHandler : public ::settings::SettingsPageUIHandler,
                         public crostini::InstallerViewStatusObserver,
+                        public crostini::CrostiniExportImport::Observer,
                         public chromeos::CrosUsbDeviceObserver {
  public:
   explicit CrostiniHandler(Profile* profile);
@@ -56,6 +58,11 @@ class CrostiniHandler : public ::settings::SettingsPageUIHandler,
   void HandleCrostiniInstallerStatusRequest(const base::ListValue* args);
   // Handle the CrostiniInstallerView opening/closing.
   void OnCrostiniInstallerViewStatusChanged(bool open) override;
+  // Handle a request for the CrostiniExportImport operation status.
+  void HandleCrostiniExportImportOperationStatusRequest(
+      const base::ListValue* args);
+  // CrostiniExportImport::Observer:
+  void OnCrostiniExportImportOperationStatusChanged(bool in_progress) override;
 
   Profile* profile_;
   // weak_ptr_factory_ should always be last member.

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

@@ -151,6 +151,33 @@ suite('CrostiniPageTests', function() {
           });
     });
 
+    test('ExportImportButtonsGetDisabledOnOperationStatus', function() {
+      assertTrue(!!subpage.$$('#crostini-export-import'));
+      subpage.$$('#crostini-export-import').click();
+      return flushAsync()
+          .then(() => {
+            subpage = crostiniPage.$$('settings-crostini-export-import');
+            assertFalse(subpage.$$('#export cr-button').disabled);
+            assertFalse(subpage.$$('#import cr-button').disabled);
+            cr.webUIListenerCallback(
+                'crostini-export-import-operation-status-changed', true);
+            return flushAsync();
+          })
+          .then(() => {
+            subpage = crostiniPage.$$('settings-crostini-export-import');
+            assertTrue(subpage.$$('#export cr-button').disabled);
+            assertTrue(subpage.$$('#import cr-button').disabled);
+            cr.webUIListenerCallback(
+                'crostini-export-import-operation-status-changed', false);
+            return flushAsync();
+          })
+          .then(() => {
+            subpage = crostiniPage.$$('settings-crostini-export-import');
+            assertFalse(subpage.$$('#export cr-button').disabled);
+            assertFalse(subpage.$$('#import cr-button').disabled);
+          });
+    });
+
     test('Remove', function() {
       assertTrue(!!subpage.$$('#remove cr-button'));
       subpage.$$('#remove cr-button').click();

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

@@ -55,6 +55,12 @@ class TestCrostiniBrowserProxy extends TestBrowserProxy {
     cr.webUIListenerCallback('crostini-installer-status-changed', false);
   }
 
+  /** @override */
+  requestCrostiniExportImportOperationStatus() {
+    cr.webUIListenerCallback(
+        'crostini-export-import-operation-status-changed', false);
+  }
+
   /** override */
   exportCrostiniContainer() {
     this.methodCalled('exportCrostiniContainer');