Add ShouldOpenFileManagerAfterMount attribute to add-smb-share-dialog

This change adds an InSettings attribute that is used to determine whether
the File Manager should be automatically opened on a successful mount call.
When mounting from the free-form dialog, the File Manager will not be
automatically opened.

Bug: chromium:887135
Test: browser_tests --gtest_filter=CrSettingsSmb*
Change-Id: I4e681c1ffe104f234784c2f958d24b2d257bc33c
Reviewed-on: https://chromium-review.googlesource.com/c/1336911Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613197}

chrome/browser/chromeos/smb_client/smb_service.cc

@@ -135,11 +135,12 @@ void SmbService::Mount(const file_system_provider::MountOptions& options,
                        const std::string& username,
                        const std::string& password,
                        bool use_chromad_kerberos,
+                       bool should_open_file_manager_after_mount,
                        MountResponse callback) {
   DCHECK(temp_file_manager_);
 
   CallMount(options, share_path, username, password, use_chromad_kerberos,
-            std::move(callback));
+            should_open_file_manager_after_mount, std::move(callback));
 }
 
 void SmbService::GatherSharesInNetwork(HostDiscoveryResponse discovery_callback,
@@ -154,6 +155,7 @@ void SmbService::CallMount(const file_system_provider::MountOptions& options,
                            const std::string& username_input,
                            const std::string& password_input,
                            bool use_chromad_kerberos,
+                           bool should_open_file_manager_after_mount,
                            MountResponse callback) {
   std::string username;
   std::string password;
@@ -208,7 +210,8 @@ void SmbService::CallMount(const file_system_provider::MountOptions& options,
       temp_file_manager_->WritePasswordToFile(password),
       base::BindOnce(&SmbService::OnMountResponse, AsWeakPtr(),
                      base::Passed(&callback), options, share_path,
-                     use_chromad_kerberos));
+                     use_chromad_kerberos,
+                     should_open_file_manager_after_mount));
 
   profile_->GetPrefs()->SetString(prefs::kMostRecentlyUsedNetworkFileShareURL,
                                   share_path.value());
@@ -219,6 +222,7 @@ void SmbService::OnMountResponse(
     const file_system_provider::MountOptions& options,
     const base::FilePath& share_path,
     bool is_kerberos_chromad,
+    bool should_open_file_manager_after_mount,
     smbprovider::ErrorType error,
     int32_t mount_id) {
   if (error != smbprovider::ERROR_OK) {
@@ -235,7 +239,7 @@ void SmbService::OnMountResponse(
   base::File::Error result =
       GetProviderService()->MountFileSystem(provider_id_, mount_options);
 
-  if (result == base::File::FILE_OK) {
+  if (result == base::File::FILE_OK && should_open_file_manager_after_mount) {
     OpenFileManager(mount_options.file_system_id);
   }
 

chrome/browser/chromeos/smb_client/smb_service.h

@@ -62,6 +62,7 @@ class SmbService : public KeyedService,
              const std::string& username,
              const std::string& password,
              bool use_chromad_kerberos,
+             bool should_open_file_manager_after_mount,
              MountResponse callback);
 
   // Completes the mounting of an SMB file system, passing |options| on to
@@ -71,6 +72,7 @@ class SmbService : public KeyedService,
                        const file_system_provider::MountOptions& options,
                        const base::FilePath& share_path,
                        bool is_kerberos_chromad,
+                       bool should_open_file_manager_after_mount,
                        smbprovider::ErrorType error,
                        int32_t mount_id);
 
@@ -89,6 +91,7 @@ class SmbService : public KeyedService,
                  const std::string& username,
                  const std::string& password,
                  bool use_chromad_kerberos,
+                 bool should_open_file_manager_after_mount,
                  MountResponse callback);
 
   // Calls file_system_provider::Service::UnmountFileSystem().

chrome/browser/resources/settings/downloads_page/smb_shares_page.html

@@ -39,6 +39,7 @@
     <template is="dom-if" if="[[showAddSmbDialog_]]" restamp>
       <add-smb-share-dialog on-close="onAddSmbDialogClosed_"
           last-url="[[prefs.network_file_shares.most_recently_used_url.value]]"
+          should-open-file-manager-after-mount
       </add-smb-share-dialog>
     </template>
   </template>

chrome/browser/ui/webui/chromeos/smb_shares/smb_handler.cc

@@ -48,7 +48,7 @@ void SmbHandler::RegisterMessages() {
 }
 
 void SmbHandler::HandleSmbMount(const base::ListValue* args) {
-  CHECK_EQ(6U, args->GetSize());
+  CHECK_EQ(7U, args->GetSize());
   std::string callback_id;
   CHECK(args->GetString(0, &callback_id));
 
@@ -57,11 +57,13 @@ void SmbHandler::HandleSmbMount(const base::ListValue* args) {
   std::string username;
   std::string password;
   bool use_kerberos;
+  bool should_open_file_manager_after_mount;
   CHECK(args->GetString(1, &mount_url));
   CHECK(args->GetString(2, &mount_name));
   CHECK(args->GetString(3, &username));
   CHECK(args->GetString(4, &password));
   CHECK(args->GetBoolean(5, &use_kerberos));
+  CHECK(args->GetBoolean(6, &should_open_file_manager_after_mount));
 
   smb_client::SmbService* const service = GetSmbService(profile_);
   if (!service) {
@@ -75,10 +77,10 @@ void SmbHandler::HandleSmbMount(const base::ListValue* args) {
   auto mount_response =
       base::BindOnce(&SmbHandler::HandleSmbMountResponse,
                      weak_ptr_factory_.GetWeakPtr(), callback_id);
-  auto mount_call =
-      base::BindOnce(&smb_client::SmbService::Mount, base::Unretained(service),
-                     mo, base::FilePath(mount_url), username, password,
-                     use_kerberos, std::move(mount_response));
+  auto mount_call = base::BindOnce(
+      &smb_client::SmbService::Mount, base::Unretained(service), mo,
+      base::FilePath(mount_url), username, password, use_kerberos,
+      should_open_file_manager_after_mount, std::move(mount_response));
 
   if (host_discovery_done_) {
     std::move(mount_call).Run();

chrome/test/data/webui/settings/smb_shares_page_tests.js

@@ -12,9 +12,10 @@ class TestSmbBrowserProxy extends TestBrowserProxy {
   }
 
   /** @override */
-  smbMount(smbUrl, smbName, username, password, authMethod) {
+  smbMount(smbUrl, smbName, username, password, authMethod, inSettings) {
     this.methodCalled(
-        'smbMount', [smbUrl, smbName, username, password, authMethod]);
+        'smbMount',
+        [smbUrl, smbName, username, password, authMethod, inSettings]);
     return Promise.resolve(SmbMountResult.SUCCESS);
   }
 
@@ -74,6 +75,7 @@ suite('AddSmbShareDialogTests', function() {
     const expectedUsername = 'username';
     const expectedPassword = 'password';
     const expectedAuthMethod = 'credentials';
+    const expectedShouldOpenFileManager = false;
 
     const url = addDialog.$$('#address');
     expectTrue(!!url);
@@ -95,6 +97,7 @@ suite('AddSmbShareDialogTests', function() {
     expectTrue(!!addButton);
 
     addDialog.authenticationMethod_ = expectedAuthMethod;
+    addDialog.shouldOpenFileManagerAfterMount = expectedShouldOpenFileManager;
 
     addButton.click();
     return smbBrowserProxy.whenCalled('smbMount').then(function(args) {
@@ -103,6 +106,7 @@ suite('AddSmbShareDialogTests', function() {
       expectEquals(expectedUsername, args[2]);
       expectEquals(expectedPassword, args[3]);
       expectEquals(expectedAuthMethod, args[4]);
+      expectEquals(expectedShouldOpenFileManager, args[5]);
     });
   });
 

ui/webui/resources/cr_components/chromeos/smb_shares/add_smb_share_dialog.js

@@ -17,6 +17,11 @@ Polymer({
       value: '',
     },
 
+    shouldOpenFileManagerAfterMount: {
+      type: Boolean,
+      value: false,
+    },
+
     /** @private {string} */
     mountUrl_: {
       type: String,
@@ -104,7 +109,8 @@ Polymer({
     this.browserProxy_
         .smbMount(
             this.mountUrl_, this.mountName_.trim(), this.username_,
-            this.password_, this.authenticationMethod_)
+            this.password_, this.authenticationMethod_,
+            this.shouldOpenFileManagerAfterMount)
         .then(result => {
           this.onAddShare_(result);
         });

ui/webui/resources/cr_components/chromeos/smb_shares/smb_browser_proxy.js

@@ -44,9 +44,12 @@ cr.define('smb_shares', function() {
      * @param {string} username
      * @param {string} password
      * @param {string} authMethod
+     * @param {boolean} shouldOpenFileManagerAfterMount
      * @return {!Promise<SmbMountResult>}
      */
-    smbMount(smbUrl, smbName, username, password, authMethod) {}
+    smbMount(
+        smbUrl, smbName, username, password, authMethod,
+        shouldOpenFileManagerAfterMount) {}
 
     /**
      * Starts the file share discovery process.
@@ -57,10 +60,13 @@ cr.define('smb_shares', function() {
   /** @implements {smb_shares.SmbBrowserProxy} */
   class SmbBrowserProxyImpl {
     /** @override */
-    smbMount(smbUrl, smbName, username, password, authMethod) {
+    smbMount(
+        smbUrl, smbName, username, password, authMethod,
+        shouldOpenFileManagerAfterMount) {
       return cr.sendWithPromise(
           'smbMount', smbUrl, smbName, username, password,
-          authMethod == SmbAuthMethod.KERBEROS);
+          authMethod == SmbAuthMethod.KERBEROS,
+          shouldOpenFileManagerAfterMount);
     }
 
     /** @override */