Show retry dialog when crostini unsharing folder fails

Unsharing a crostini folder can fail if a user has a file
open in some application.  When this happens, we will show
a dialog to the user to inform them that unsharing has
failed and allow them to retry the unsharing presumably
after they have closed associated files.

Bug: 920416
Change-Id: I303200f7e0e40fe8c7e1382bf19878d7ed2b31fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037276Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Julian Watson <juwa@google.com>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738462}

chrome/app/settings_strings.grdp

@@ -632,6 +632,15 @@
     <message name="IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_SHARING" desc="Tooltip to show when hovering on the remove icon for a crostini shared folder.">
       Remove sharing
     </message>
+    <message name="IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_FAILURE_DIALOG_MESSAGE" desc="Message to show user when unsharing a crostini shared folder fails.">
+      Couldn't unshare because an application is using this folder. The folder will be unshared when Linux is next shut down.
+    </message>
+    <message name="IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_FAILURE_DIALOG_TITLE" desc="Title of the error dialog shown to a user when unsharing a crostini shared folder fails.">
+      Unshare failed
+    </message>
+    <message name="IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_FAILURE_TRY_AGAIN" desc="Button text in the dialog shown when unsharing a crostini shared folder fails.  Pressing this button will attempt to unshare the folder again.">
+      Try again
+    </message>
     <message name="IDS_SETTINGS_CROSTINI_EXPORT_IMPORT_TITLE" desc="Title for crostini container export and imoprt (backup and restore) section">
       Backup &amp; restore
     </message>

chrome/app/settings_strings_grdp/IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_FAILURE_DIALOG_MESSAGE.png.sha1

+f735083d7a1a1439fb455dbd6770cd16b5628b21
\ No newline at end of file

chrome/app/settings_strings_grdp/IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_FAILURE_DIALOG_TITLE.png.sha1

+f735083d7a1a1439fb455dbd6770cd16b5628b21
\ No newline at end of file

chrome/app/settings_strings_grdp/IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_FAILURE_TRY_AGAIN.png.sha1

+f735083d7a1a1439fb455dbd6770cd16b5628b21
\ No newline at end of file

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

@@ -48,6 +48,7 @@ cr.define('settings', function() {
     /**
      * @param {string} vmName VM to stop sharing path with.
      * @param {string} path Path to stop sharing.
+     * @return {!Promise<boolean>} Result of unsharing.
      */
     removeCrostiniSharedPath(vmName, path) {}
 
@@ -115,7 +116,7 @@ cr.define('settings', function() {
 
     /** @override */
     removeCrostiniSharedPath(vmName, path) {
-      chrome.send('removeCrostiniSharedPath', [vmName, path]);
+      return cr.sendWithPromise('removeCrostiniSharedPath', vmName, path);
     }
 
     /** @override */

chrome/browser/resources/settings/chromeos/crostini_page/crostini_shared_paths.html

@@ -47,6 +47,24 @@
         </template>
       </div>
     </div>
+    <template is="dom-if" if="[[sharedPathWhichFailedRemoval_]]" restamp>
+      <cr-dialog id="removeSharedPathFailedDialog" close-text="$i18n{close}">
+        <div slot="title">
+          $i18n{crostiniSharedPathsRemoveFailureDialogTitle}
+        </div>
+        <div slot="body">
+          $i18n{crostiniSharedPathsRemoveFailureDialogMessage}
+        </div>
+        <div slot="button-container">
+          <cr-button id="cancel" class="cancel-button"
+              on-click="onRemoveFailedDismissTap_">$i18n{ok}</cr-button>
+          </cr-button>
+          <cr-button id="retry" class="action-button"
+              on-click="onRemoveFailedRetryTap_">
+              $i18n{crostiniSharedPathsRemoveFailureTryAgain}
+        </div>
+      </cr-dialog>
+    </template>
   </template>
   <script src="crostini_shared_paths.js"></script>
 </dom-module>

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

@@ -33,6 +33,16 @@ Polymer({
      * @private {Array<!CrostiniSharedPath>}
      */
     sharedPaths_: Array,
+
+    /**
+     * The shared path which failed to be removed in the most recent attempt
+     * to remove a path. Null indicates that removal succeeded.
+     * @private {?string}
+     */
+    sharedPathWhichFailedRemoval_: {
+      type: String,
+      value: null,
+    },
   },
 
   observers: [
@@ -59,13 +69,46 @@ Polymer({
         });
   },
 
+  /**
+   * @param {string} path
+   * @private
+   */
+  removeSharedPath_(path) {
+    this.sharedPathWhichFailedRemoval_ = null;
+    settings.CrostiniBrowserProxyImpl.getInstance()
+        .removeCrostiniSharedPath(DEFAULT_CROSTINI_VM, path)
+        .then(result => {
+          if (!result) {
+            this.sharedPathWhichFailedRemoval_ = path;
+            // Flush to make sure that the retry dialog is attached.
+            Polymer.dom.flush();
+            this.$$('#removeSharedPathFailedDialog').showModal();
+          }
+        });
+  },
+
   /**
    * @param {!Event} event
    * @private
    */
   onRemoveSharedPathTap_(event) {
-    settings.CrostiniBrowserProxyImpl.getInstance().removeCrostiniSharedPath(
-        DEFAULT_CROSTINI_VM, event.model.item.path);
+    this.removeSharedPath_(event.model.item.path);
+  },
+
+  /**
+   * @param {!Event} event
+   * @private
+   */
+  onRemoveFailedRetryTap_(event) {
+    this.removeSharedPath_(assert(this.sharedPathWhichFailedRemoval_));
+  },
+
+  /**
+   * @param {!Event} event
+   * @private
+   */
+  onRemoveFailedDismissTap_(event) {
+    this.sharedPathWhichFailedRemoval_ = null;
   },
 
   /**

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

@@ -168,24 +168,31 @@ void CrostiniHandler::HandleGetCrostiniSharedPathsDisplayText(
 
 void CrostiniHandler::HandleRemoveCrostiniSharedPath(
     const base::ListValue* args) {
-  CHECK_EQ(2U, args->GetSize());
+  AllowJavascript();
+  CHECK_EQ(3U, args->GetSize());
+  std::string callback_id;
   std::string vm_name;
-  CHECK(args->GetString(0, &vm_name));
   std::string path;
-  CHECK(args->GetString(1, &path));
+  CHECK(args->GetString(0, &callback_id));
+  CHECK(args->GetString(1, &vm_name));
+  CHECK(args->GetString(2, &path));
 
   guest_os::GuestOsSharePath::GetForProfile(profile_)->UnsharePath(
       vm_name, base::FilePath(path),
       /*unpersist=*/true,
-      base::BindOnce(
-          [](const std::string& path, bool result,
+      base::BindOnce(&CrostiniHandler::OnCrostiniSharedPathRemoved,
+                     weak_ptr_factory_.GetWeakPtr(), callback_id, path));
+}
+
+void CrostiniHandler::OnCrostiniSharedPathRemoved(
+    const std::string& callback_id,
+    const std::string& path,
+    bool result,
     const std::string& failure_reason) {
   if (!result) {
-              LOG(ERROR) << "Error unsharing " << path << ": "
-                         << failure_reason;
+    LOG(ERROR) << "Error unsharing " << path << ": " << failure_reason;
   }
-          },
-          path));
+  ResolveJavascriptCallback(base::Value(callback_id), base::Value(result));
 }
 
 namespace {

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

@@ -45,6 +45,10 @@ class CrostiniHandler : public ::settings::SettingsPageUIHandler,
   void HandleGetCrostiniSharedPathsDisplayText(const base::ListValue* args);
   // Remove a specified path from being shared.
   void HandleRemoveCrostiniSharedPath(const base::ListValue* args);
+  void OnCrostiniSharedPathRemoved(const std::string& callback_id,
+                                   const std::string& path,
+                                   bool result,
+                                   const std::string& failure_reason);
   // Returns a list of available USB devices.
   void HandleGetCrostiniSharedUsbDevices(const base::ListValue* args);
   // Set the share state of a USB device.

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

@@ -561,6 +561,12 @@ void AddCrostiniStrings(content::WebUIDataSource* html_source,
        IDS_SETTINGS_CROSTINI_SHARED_PATHS_INSTRUCTIONS_REMOVE},
       {"crostiniSharedPathsRemoveSharing",
        IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_SHARING},
+      {"crostiniSharedPathsRemoveFailureDialogMessage",
+       IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_FAILURE_DIALOG_MESSAGE},
+      {"crostiniSharedPathsRemoveFailureDialogTitle",
+       IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_FAILURE_DIALOG_TITLE},
+      {"crostiniSharedPathsRemoveFailureTryAgain",
+       IDS_SETTINGS_CROSTINI_SHARED_PATHS_REMOVE_FAILURE_TRY_AGAIN},
       {"crostiniSharedPathsListEmptyMessage",
        IDS_SETTINGS_CROSTINI_SHARED_PATHS_LIST_EMPTY_MESSAGE},
       {"crostiniExportImportTitle", IDS_SETTINGS_CROSTINI_EXPORT_IMPORT_TITLE},

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

@@ -346,6 +346,28 @@ suite('CrostiniPageTests', function() {
             assertFalse(subpage.$.crostiniListEmpty.hidden);
           });
     });
+
+    test('RemoveFailedRetry', function() {
+      // Remove shared path fails.
+      crostiniBrowserProxy.removeSharedPathResult = false;
+      subpage.$$('.list-item cr-icon-button').click();
+      return crostiniBrowserProxy.whenCalled('removeCrostiniSharedPath')
+          .then(() => {
+            Polymer.dom.flush();
+            assertTrue(subpage.$$('#removeSharedPathFailedDialog').open);
+
+            // Click retry and make sure 'removeCrostiniSharedPath' is called
+            // and dialog is closed/removed.
+            crostiniBrowserProxy.removeSharedPathResult = true;
+            subpage.$$('#removeSharedPathFailedDialog')
+                .querySelector('.action-button')
+                .click();
+            return crostiniBrowserProxy.whenCalled('removeCrostiniSharedPath');
+          })
+          .then(() => {
+            assertFalse(!!subpage.$$('#removeSharedPathFailedDialog'));
+          });
+    });
   });
 
   suite('SubPageSharedUsbDevices', function() {

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

@@ -17,6 +17,7 @@ class TestCrostiniBrowserProxy extends TestBrowserProxy {
       'requestCrostiniContainerUpgradeView',
     ]);
     this.sharedUsbDevices = [];
+    this.removeSharedPathResult = true;
   }
 
   /** @override */
@@ -49,6 +50,7 @@ class TestCrostiniBrowserProxy extends TestBrowserProxy {
   /** override */
   removeCrostiniSharedPath(vmName, path) {
     this.methodCalled('removeCrostiniSharedPath', [vmName, path]);
+    return Promise.resolve(this.removeSharedPathResult);
   }
 
   /** @override */