Implement save/restore of smbfs shares

The existing SMB integration is based on file_system_provider, which
internally saves (if requested) mounts and restores them on login. Smbfs
is no longer based on FSP, and therefore needs to use its own mechanism
for saving and restoring mount on login.

Bug: 939235
Change-Id: I3c6c96cc8c733c74b5e071167d7d9ebf3b9eb12d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2071537Reviewed-by: Austin Tankiang <austinct@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745308}

chrome/browser/chromeos/BUILD.gn

@@ -2212,6 +2212,8 @@ source_set("chromeos") {
     "smb_client/smb_file_system_id.h",
     "smb_client/smb_kerberos_credentials_updater.cc",
     "smb_client/smb_kerberos_credentials_updater.h",
+    "smb_client/smb_persisted_share_registry.cc",
+    "smb_client/smb_persisted_share_registry.h",
     "smb_client/smb_provider.cc",
     "smb_client/smb_provider.h",
     "smb_client/smb_service.cc",
@@ -3077,6 +3079,7 @@ source_set("unit_tests") {
     "smb_client/smb_file_system_id_test.cc",
     "smb_client/smb_file_system_unittest.cc",
     "smb_client/smb_kerberos_credentials_updater_unittest.cc",
+    "smb_client/smb_persisted_share_registry_unittest.cc",
     "smb_client/smb_service_helper_unittest.cc",
     "smb_client/smb_service_unittest.cc",
     "smb_client/smb_share_finder_unittest.cc",

chrome/browser/chromeos/smb_client/smb_persisted_share_registry.cc

+// 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.
+
+#include "chrome/browser/chromeos/smb_client/smb_persisted_share_registry.h"
+
+#include <utility>
+
+#include "base/logging.h"
+#include "base/values.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+#include "components/pref_registry/pref_registry_syncable.h"
+#include "components/prefs/scoped_user_pref_update.h"
+
+namespace chromeos {
+namespace smb_client {
+namespace {
+
+constexpr char kShareUrlKey[] = "share_url";
+constexpr char kDisplayNameKey[] = "display_name";
+constexpr char kUsernameKey[] = "username";
+constexpr char kWorkgroupKey[] = "workgroup";
+constexpr char kUseKerberosKey[] = "use_kerberos";
+
+base::Value ShareToDict(const SmbShareInfo& share) {
+  base::Value dict(base::Value::Type::DICTIONARY);
+  dict.SetStringKey(kShareUrlKey, share.share_url().ToString());
+  dict.SetStringKey(kDisplayNameKey, share.display_name());
+  dict.SetBoolKey(kUseKerberosKey, share.use_kerberos());
+  if (!share.username().empty()) {
+    dict.SetStringKey(kUsernameKey, share.username());
+  }
+  if (!share.workgroup().empty()) {
+    dict.SetStringKey(kWorkgroupKey, share.workgroup());
+  }
+  return dict;
+}
+
+std::string GetStringValue(const base::Value& dict, const std::string& key) {
+  const std::string* value = dict.FindStringKey(key);
+  if (!value) {
+    return {};
+  }
+  return *value;
+}
+
+base::Optional<SmbShareInfo> DictToShare(const base::Value& dict) {
+  std::string share_url = GetStringValue(dict, kShareUrlKey);
+  if (share_url.empty()) {
+    return {};
+  }
+
+  SmbUrl url(share_url);
+  DCHECK(url.IsValid());
+  SmbShareInfo info(url, GetStringValue(dict, kDisplayNameKey),
+                    GetStringValue(dict, kUsernameKey),
+                    GetStringValue(dict, kWorkgroupKey),
+                    dict.FindBoolKey(kUseKerberosKey).value_or(false));
+  return base::make_optional(std::move(info));
+}
+
+}  // namespace
+
+// static
+void SmbPersistedShareRegistry::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterListPref(prefs::kNetworkFileSharesSavedShares);
+}
+
+SmbPersistedShareRegistry::SmbPersistedShareRegistry(Profile* profile)
+    : profile_(profile) {}
+
+void SmbPersistedShareRegistry::Save(const SmbShareInfo& share) {
+  ListPrefUpdate pref(profile_->GetPrefs(),
+                      prefs::kNetworkFileSharesSavedShares);
+
+  base::Value::ListView share_list = pref->GetList();
+  for (auto it = share_list.begin(); it != share_list.end(); ++it) {
+    if (GetStringValue(*it, kShareUrlKey) == share.share_url().ToString()) {
+      *it = ShareToDict(share);
+      return;
+    }
+  }
+
+  pref->Append(ShareToDict(share));
+  return;
+}
+
+void SmbPersistedShareRegistry::Delete(const SmbUrl& share_url) {
+  ListPrefUpdate pref(profile_->GetPrefs(),
+                      prefs::kNetworkFileSharesSavedShares);
+
+  base::Value::ListView share_list = pref->GetList();
+  for (auto it = share_list.begin(); it != share_list.end(); ++it) {
+    if (GetStringValue(*it, kShareUrlKey) == share_url.ToString()) {
+      bool result = pref->EraseListIter(it);
+      DCHECK(result);
+      return;
+    }
+  }
+}
+
+base::Optional<SmbShareInfo> SmbPersistedShareRegistry::Get(
+    const SmbUrl& share_url) const {
+  const base::Value* pref =
+      profile_->GetPrefs()->Get(prefs::kNetworkFileSharesSavedShares);
+  if (!pref) {
+    return {};
+  }
+
+  base::Value::ConstListView share_list = pref->GetList();
+  for (auto it = share_list.begin(); it != share_list.end(); ++it) {
+    if (GetStringValue(*it, kShareUrlKey) == share_url.ToString()) {
+      return DictToShare(*it);
+    }
+  }
+  return {};
+}
+
+std::vector<SmbShareInfo> SmbPersistedShareRegistry::GetAll() const {
+  const base::Value* pref =
+      profile_->GetPrefs()->Get(prefs::kNetworkFileSharesSavedShares);
+  if (!pref) {
+    return {};
+  }
+
+  std::vector<SmbShareInfo> shares;
+  base::Value::ConstListView share_list = pref->GetList();
+  for (auto it = share_list.begin(); it != share_list.end(); ++it) {
+    base::Optional<SmbShareInfo> info = DictToShare(*it);
+    if (!info) {
+      continue;
+    }
+    shares.push_back(std::move(*info));
+  }
+  return shares;
+}
+
+}  // namespace smb_client
+}  // namespace chromeos

chrome/browser/chromeos/smb_client/smb_persisted_share_registry.h

+// 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_SMB_CLIENT_SMB_PERSISTED_SHARE_REGISTRY_H_
+#define CHROME_BROWSER_CHROMEOS_SMB_CLIENT_SMB_PERSISTED_SHARE_REGISTRY_H_
+
+#include <string>
+#include <vector>
+
+#include "base/optional.h"
+#include "chrome/browser/chromeos/smb_client/smb_share_info.h"
+
+class Profile;
+
+namespace user_prefs {
+class PrefRegistrySyncable;
+}
+
+namespace chromeos {
+namespace smb_client {
+
+class SmbUrl;
+
+// Handles saving of SMB shares in the user's Profile.
+class SmbPersistedShareRegistry {
+ public:
+  static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+
+  explicit SmbPersistedShareRegistry(Profile* profile);
+
+  // Disallow copy/assign.
+  SmbPersistedShareRegistry() = delete;
+  SmbPersistedShareRegistry(const SmbPersistedShareRegistry&) = delete;
+  SmbPersistedShareRegistry& operator=(const SmbPersistedShareRegistry&) =
+      delete;
+
+  // Save |share| in the user's profile. If a saved share already exists with
+  // the url share.share_url(), that saved share will be overwritten.
+  void Save(const SmbShareInfo& share);
+
+  // Delete the saved share with the URL |share_url|.
+  void Delete(const SmbUrl& share_url);
+
+  // Return the saved share with URL |share_url|, or the empty Optional<> if no
+  // share is found.
+  base::Optional<SmbShareInfo> Get(const SmbUrl& share_url) const;
+
+  // Return a list of all saved shares.
+  std::vector<SmbShareInfo> GetAll() const;
+
+ private:
+  Profile* const profile_;
+};
+
+}  // namespace smb_client
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_SMB_CLIENT_SMB_PERSISTED_SHARE_REGISTRY_H_

chrome/browser/chromeos/smb_client/smb_persisted_share_registry_unittest.cc

+// 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.
+
+#include "chrome/browser/chromeos/smb_client/smb_persisted_share_registry.h"
+
+#include "chrome/browser/chromeos/smb_client/smb_url.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/browser_task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace smb_client {
+namespace {
+
+const char kShareUrl[] = "smb://server/share1";
+const char kShareUrl2[] = "smb://server/share2";
+const char kDisplayName[] = "My File Share";
+const char kUsername[] = "test-user";
+const char kUsername2[] = "test-user2";
+const char kWorkgroup[] = "test-workgroup.com";
+
+class SmbPersistedShareRegistryTest : public testing::Test {
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+  TestingProfile profile_;
+};
+
+TEST_F(SmbPersistedShareRegistryTest, Empty) {
+  SmbPersistedShareRegistry registry(&profile_);
+  base::Optional<SmbShareInfo> info = registry.Get(SmbUrl(kShareUrl));
+  EXPECT_FALSE(info);
+
+  std::vector<SmbShareInfo> all_info = registry.GetAll();
+  EXPECT_TRUE(all_info.empty());
+
+  // Should do nothing, not crash.
+  registry.Delete(SmbUrl(kShareUrl));
+}
+
+TEST_F(SmbPersistedShareRegistryTest, SaveGet) {
+  {
+    SmbPersistedShareRegistry registry(&profile_);
+    SmbShareInfo info1(SmbUrl(kShareUrl), kDisplayName, kUsername, kWorkgroup,
+                       false /* use_kerberos */);
+    registry.Save(info1);
+
+    SmbShareInfo info2(SmbUrl(kShareUrl2), kDisplayName, kUsername2, kWorkgroup,
+                       true /* use_kerberos */);
+    registry.Save(info2);
+  }
+  // Use scopes to simulate a logout/login so that the instances of
+  // SmbPersistedShareRegistry are not the same (and have no hidden state).
+  {
+    SmbPersistedShareRegistry registry(&profile_);
+    base::Optional<SmbShareInfo> info = registry.Get(SmbUrl(kShareUrl));
+    ASSERT_TRUE(info);
+    EXPECT_EQ(info->share_url().ToString(), kShareUrl);
+    EXPECT_EQ(info->display_name(), kDisplayName);
+    EXPECT_EQ(info->username(), kUsername);
+    EXPECT_EQ(info->workgroup(), kWorkgroup);
+    EXPECT_FALSE(info->use_kerberos());
+
+    base::Optional<SmbShareInfo> info2 = registry.Get(SmbUrl(kShareUrl2));
+    ASSERT_TRUE(info2);
+    EXPECT_EQ(info2->share_url().ToString(), kShareUrl2);
+    EXPECT_EQ(info2->display_name(), kDisplayName);
+    EXPECT_EQ(info2->username(), kUsername2);
+    EXPECT_EQ(info2->workgroup(), kWorkgroup);
+    EXPECT_TRUE(info2->use_kerberos());
+
+    std::vector<SmbShareInfo> all_info = registry.GetAll();
+    EXPECT_EQ(all_info.size(), 2u);
+    EXPECT_EQ(all_info[0].share_url().ToString(), kShareUrl);
+    EXPECT_EQ(all_info[1].share_url().ToString(), kShareUrl2);
+  }
+}
+
+TEST_F(SmbPersistedShareRegistryTest, Replace) {
+  {
+    SmbPersistedShareRegistry registry(&profile_);
+    SmbShareInfo info(SmbUrl(kShareUrl), kDisplayName, kUsername, kWorkgroup,
+                      false /* use_kerberos */);
+    registry.Save(info);
+  }
+  // Use scopes to simulate a logout/login so that the instances of
+  // SmbPersistedShareRegistry are not the same (and have no hidden state).
+  {
+    SmbPersistedShareRegistry registry(&profile_);
+    base::Optional<SmbShareInfo> info = registry.Get(SmbUrl(kShareUrl));
+    ASSERT_TRUE(info);
+    EXPECT_EQ(info->share_url().ToString(), kShareUrl);
+    EXPECT_EQ(info->display_name(), kDisplayName);
+    EXPECT_EQ(info->username(), kUsername);
+    EXPECT_EQ(info->workgroup(), kWorkgroup);
+    EXPECT_FALSE(info->use_kerberos());
+
+    std::vector<SmbShareInfo> all_info = registry.GetAll();
+    EXPECT_EQ(all_info.size(), 1u);
+
+    SmbShareInfo replace_info(SmbUrl(kShareUrl), kDisplayName, kUsername2,
+                              kWorkgroup, true /* use_kerberos */);
+    registry.Save(replace_info);
+  }
+  {
+    SmbPersistedShareRegistry registry(&profile_);
+    base::Optional<SmbShareInfo> info = registry.Get(SmbUrl(kShareUrl));
+    ASSERT_TRUE(info);
+    EXPECT_EQ(info->share_url().ToString(), kShareUrl);
+    EXPECT_EQ(info->display_name(), kDisplayName);
+    EXPECT_EQ(info->username(), kUsername2);
+    EXPECT_EQ(info->workgroup(), kWorkgroup);
+    EXPECT_TRUE(info->use_kerberos());
+
+    std::vector<SmbShareInfo> all_info = registry.GetAll();
+    EXPECT_EQ(all_info.size(), 1u);
+  }
+}
+
+TEST_F(SmbPersistedShareRegistryTest, Delete) {
+  {
+    SmbPersistedShareRegistry registry(&profile_);
+    SmbShareInfo info1(SmbUrl(kShareUrl), kDisplayName, kUsername, kWorkgroup,
+                       false /* use_kerberos */);
+    registry.Save(info1);
+
+    SmbShareInfo info2(SmbUrl(kShareUrl2), kDisplayName, kUsername2, kWorkgroup,
+                       true /* use_kerberos */);
+    registry.Save(info2);
+  }
+  // Use scopes to simulate a logout/login so that the instances of
+  // SmbPersistedShareRegistry are not the same (and have no hidden state).
+  {
+    SmbPersistedShareRegistry registry(&profile_);
+    registry.Delete(SmbUrl(kShareUrl2));
+
+    base::Optional<SmbShareInfo> info = registry.Get(SmbUrl(kShareUrl));
+    ASSERT_TRUE(info);
+    EXPECT_EQ(info->share_url().ToString(), kShareUrl);
+    EXPECT_EQ(info->display_name(), kDisplayName);
+    EXPECT_EQ(info->username(), kUsername);
+    EXPECT_EQ(info->workgroup(), kWorkgroup);
+    EXPECT_FALSE(info->use_kerberos());
+
+    base::Optional<SmbShareInfo> info2 = registry.Get(SmbUrl(kShareUrl2));
+    ASSERT_FALSE(info2);
+
+    std::vector<SmbShareInfo> all_info = registry.GetAll();
+    EXPECT_EQ(all_info.size(), 1u);
+  }
+}
+
+}  // namespace
+}  // namespace smb_client
+}  // namespace chromeos

chrome/browser/chromeos/smb_client/smb_service.cc

@@ -132,7 +132,8 @@ SmbService::SmbService(Profile* profile,
                        std::unique_ptr<base::TickClock> tick_clock)
     : provider_id_(ProviderId::CreateFromNativeId("smb")),
       profile_(profile),
-      tick_clock_(std::move(tick_clock)) {
+      tick_clock_(std::move(tick_clock)),
+      registry_(profile) {
   user_manager::User* user =
       chromeos::ProfileHelper::Get()->GetUserByProfile(profile_);
   DCHECK(user);
@@ -178,13 +179,18 @@ void SmbService::RegisterProfilePrefs(
   registry->RegisterBooleanPref(prefs::kNTLMShareAuthenticationEnabled, true);
   registry->RegisterListPref(prefs::kNetworkFileSharesPreconfiguredShares);
   registry->RegisterStringPref(prefs::kMostRecentlyUsedNetworkFileShareURL, "");
+  SmbPersistedShareRegistry::RegisterProfilePrefs(registry);
 }
 
 void SmbService::UnmountSmbFs(const base::FilePath& mount_path) {
   DCHECK(!mount_path.empty());
 
   for (auto it = smbfs_shares_.begin(); it != smbfs_shares_.end(); ++it) {
-    if (it->second->mount_path() == mount_path) {
+    SmbFsShare* share = it->second.get();
+    if (share->mount_path() == mount_path) {
+      // UnmountSmbFs() is called by an explicit unmount by the user. In this
+      // case, forget the share.
+      registry_.Delete(share->share_url());
       smbfs_shares_.erase(it);
       return;
     }
@@ -347,13 +353,14 @@ void SmbService::Mount(const file_system_provider::MountOptions& options,
                 false /* skip_connect */,
                 base::BindOnce(&SmbService::MountInternalDone,
                                base::Unretained(this), std::move(callback),
-                               should_open_file_manager_after_mount));
+                               info, should_open_file_manager_after_mount));
 
   profile_->GetPrefs()->SetString(prefs::kMostRecentlyUsedNetworkFileShareURL,
                                   share_path.value());
 }
 
 void SmbService::MountInternalDone(MountResponse callback,
+                                   const SmbShareInfo& info,
                                    bool should_open_file_manager_after_mount,
                                    SmbMountResult result,
                                    const base::FilePath& mount_path) {
@@ -367,6 +374,10 @@ void SmbService::MountInternalDone(MountResponse callback,
     platform_util::ShowItemInFolder(profile_, mount_path);
   }
 
+  if (IsSmbFsEnabled()) {
+    registry_.Save(info);
+  }
+
   RecordMountCount();
   std::move(callback).Run(SmbMountResult::kSuccess);
 }
@@ -535,25 +546,38 @@ SmbProviderClient* SmbService::GetSmbProviderClient() const {
 }
 
 void SmbService::RestoreMounts() {
-  std::vector<ProvidedFileSystemInfo> file_systems =
+  std::vector<ProvidedFileSystemInfo> provided_file_systems =
       GetProviderService()->GetProvidedFileSystemInfoList(provider_id_);
 
   std::vector<SmbUrl> preconfigured_shares =
       GetPreconfiguredSharePathsForPremount();
 
-  if (!file_systems.empty() || !preconfigured_shares.empty()) {
+  std::vector<SmbShareInfo> saved_smbfs_shares;
+  if (IsSmbFsEnabled()) {
+    // Restore smbfs shares.
+    // TODO(crbug.com/1055571): Migrate saved smbprovider shares to smbfs.
+    saved_smbfs_shares = registry_.GetAll();
+  }
+
+  if (!provided_file_systems.empty() || !saved_smbfs_shares.empty() ||
+      !preconfigured_shares.empty()) {
     share_finder_->DiscoverHostsInNetwork(base::BindOnce(
-        &SmbService::OnHostsDiscovered, AsWeakPtr(), std::move(file_systems),
+        &SmbService::OnHostsDiscovered, AsWeakPtr(),
+        std::move(provided_file_systems), std::move(saved_smbfs_shares),
         std::move(preconfigured_shares)));
   }
 }
 
 void SmbService::OnHostsDiscovered(
     const std::vector<ProvidedFileSystemInfo>& file_systems,
+    const std::vector<SmbShareInfo>& saved_smbfs_shares,
     const std::vector<SmbUrl>& preconfigured_shares) {
   for (const auto& file_system : file_systems) {
     Remount(file_system);
   }
+  for (const auto& smbfs_share : saved_smbfs_shares) {
+    MountSavedSmbfsShare(smbfs_share);
+  }
   for (const auto& url : preconfigured_shares) {
     MountPreconfiguredShare(url);
   }
@@ -647,6 +671,17 @@ void SmbService::OnRemountResponse(const std::string& file_system_id,
   mount_id_map_[file_system_id] = mount_id;
 }
 
+void SmbService::MountSavedSmbfsShare(const SmbShareInfo& info) {
+  MountInternal(
+      {} /* fsp::MountOptions, ignored by smbfs */, info, "" /* password */,
+      false /* save_credentials */, true /* skip_connect */,
+      base::BindOnce(
+          [](SmbMountResult result, const base::FilePath& mount_path) {
+            LOG_IF(ERROR, result != SmbMountResult::kSuccess)
+                << "Error restoring saved share: " << static_cast<int>(result);
+          }));
+}
+
 void SmbService::MountPreconfiguredShare(const SmbUrl& share_url) {
   file_system_provider::MountOptions mount_options;
   mount_options.display_name =

chrome/browser/chromeos/smb_client/smb_service.h

@@ -20,6 +20,7 @@
 #include "chrome/browser/chromeos/file_system_provider/provider_interface.h"
 #include "chrome/browser/chromeos/file_system_provider/service.h"
 #include "chrome/browser/chromeos/smb_client/smb_errors.h"
+#include "chrome/browser/chromeos/smb_client/smb_persisted_share_registry.h"
 #include "chrome/browser/chromeos/smb_client/smb_share_finder.h"
 #include "chrome/browser/chromeos/smb_client/smb_task_queue.h"
 #include "chrome/browser/chromeos/smb_client/smbfs_share.h"
@@ -132,6 +133,7 @@ class SmbService : public KeyedService,
 
   // Callback passed to MountInternal().
   void MountInternalDone(MountResponse callback,
+                         const SmbShareInfo& info,
                          bool should_open_file_manager_after_mount,
                          SmbMountResult result,
                          const base::FilePath& mount_path);
@@ -180,6 +182,7 @@ class SmbService : public KeyedService,
 
   void OnHostsDiscovered(
       const std::vector<ProvidedFileSystemInfo>& file_systems,
+      const std::vector<SmbShareInfo>& saved_smbfs_shares,
       const std::vector<SmbUrl>& preconfigured_shares);
 
   // Closure for OnHostDiscovered(). |reply| is passed down to
@@ -199,6 +202,9 @@ class SmbService : public KeyedService,
                          smbprovider::ErrorType error,
                          int32_t mount_id);
 
+  // Mounts a saved (smbfs) SMB share with details |info|.
+  void MountSavedSmbfsShare(const SmbShareInfo& info);
+
   // Mounts a preconfigured (by policy) SMB share with path |share_url|. The
   // share is mounted with empty credentials.
   void MountPreconfiguredShare(const SmbUrl& share_url);
@@ -310,6 +316,7 @@ class SmbService : public KeyedService,
   // Note, mount ID for smbfs is a randomly generated string. For smbprovider
   // shares, it is an integer.
   std::unordered_map<std::string, std::unique_ptr<SmbFsShare>> smbfs_shares_;
+  SmbPersistedShareRegistry registry_;
 
   std::unique_ptr<SmbKerberosCredentialsUpdater> smb_credentials_updater_;
 

chrome/browser/chromeos/smb_client/smb_service_unittest.cc

@@ -30,6 +30,7 @@
 #include "chrome/browser/chromeos/file_system_provider/service.h"
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/chromeos/smb_client/smb_file_system_id.h"
+#include "chrome/browser/chromeos/smb_client/smb_persisted_share_registry.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ -730,6 +731,16 @@ TEST_F(SmbServiceWithSmbfsTest, Mount) {
     }
   }
   EXPECT_TRUE(found);
+
+  // Check that the share was saved.
+  SmbPersistedShareRegistry registry(profile_);
+  base::Optional<SmbShareInfo> info = registry.Get(SmbUrl(kShareUrl));
+  ASSERT_TRUE(info);
+  EXPECT_EQ(info->share_url().ToString(), kShareUrl);
+  EXPECT_EQ(info->display_name(), kDisplayName);
+  EXPECT_EQ(info->username(), kTestUser);
+  EXPECT_TRUE(info->workgroup().empty());
+  EXPECT_FALSE(info->use_kerberos());
 }
 
 TEST_F(SmbServiceWithSmbfsTest, Mount_ActiveDirectory) {
@@ -790,6 +801,17 @@ TEST_F(SmbServiceWithSmbfsTest, Mount_ActiveDirectory) {
         run_loop.Quit();
       }));
   run_loop.Run();
+
+  // Check that the share was saved.
+  SmbPersistedShareRegistry registry(ad_profile_);
+  base::Optional<SmbShareInfo> info = registry.Get(SmbUrl(kShareUrl));
+  ASSERT_TRUE(info);
+  EXPECT_EQ(info->share_url().ToString(), kShareUrl);
+  EXPECT_EQ(info->display_name(), kDisplayName);
+  EXPECT_EQ(info->username(), kTestADUser);
+  // Workgroup/domain is converted to upper-case.
+  EXPECT_EQ(info->workgroup(), base::ToUpperASCII(kTestADDomain));
+  EXPECT_TRUE(info->use_kerberos());
 }
 
 TEST_F(SmbServiceWithSmbfsTest, PreconfiguredMount) {
@@ -843,5 +865,64 @@ TEST_F(SmbServiceWithSmbfsTest, PreconfiguredMount) {
   run_loop.Run();
 }
 
+TEST_F(SmbServiceWithSmbfsTest, MountSaved) {
+  // Save share in profile.
+  {
+    SmbPersistedShareRegistry registry(profile_);
+    SmbShareInfo info(SmbUrl(kShareUrl), kDisplayName, kTestUser, kTestDomain,
+                      false /* use_kerberos */);
+    registry.Save(info);
+  }
+
+  CreateService(profile_);
+
+  mojo::Remote<smbfs::mojom::SmbFs> smbfs_remote;
+  MockSmbFsImpl smbfs_impl(smbfs_remote.BindNewPipeAndPassReceiver());
+  mojo::Remote<smbfs::mojom::SmbFsDelegate> smbfs_delegate_remote;
+
+  smbfs::SmbFsHost::Delegate* smbfs_host_delegate = nullptr;
+  std::unique_ptr<MockSmbFsMounter> mock_mounter =
+      std::make_unique<MockSmbFsMounter>();
+  smb_service_->SetSmbFsMounterCreationCallbackForTesting(
+      base::BindLambdaForTesting([&mock_mounter, &smbfs_host_delegate](
+                                     const std::string& share_path,
+                                     const std::string& mount_dir_name,
+                                     const SmbFsShare::MountOptions& options,
+                                     smbfs::SmbFsHost::Delegate* delegate)
+                                     -> std::unique_ptr<smbfs::SmbFsMounter> {
+        EXPECT_EQ(share_path, kShareUrl);
+        EXPECT_EQ(options.username, kTestUser);
+        EXPECT_EQ(options.workgroup, kTestDomain);
+        EXPECT_TRUE(options.password.empty());
+        EXPECT_EQ(options.allow_ntlm, true);
+        EXPECT_FALSE(options.kerberos_options);
+        smbfs_host_delegate = delegate;
+        return std::move(mock_mounter);
+      }));
+
+  base::RunLoop run_loop;
+  EXPECT_CALL(*mock_mounter, Mount(_))
+      .WillOnce([this, &smbfs_host_delegate, &smbfs_remote,
+                 &smbfs_delegate_remote,
+                 &run_loop](smbfs::SmbFsMounter::DoneCallback callback) {
+        std::move(callback).Run(
+            smbfs::mojom::MountError::kOk,
+            std::make_unique<smbfs::SmbFsHost>(
+                MakeMountPoint(base::FilePath(kMountPath)), smbfs_host_delegate,
+                std::move(smbfs_remote),
+                smbfs_delegate_remote.BindNewPipeAndPassReceiver()));
+        run_loop.Quit();
+      });
+
+  run_loop.Run();
+
+  // Unmounting should remove the saved share.
+  smb_service_->UnmountSmbFs(base::FilePath(kMountPath));
+  SmbPersistedShareRegistry registry(profile_);
+  base::Optional<SmbShareInfo> info = registry.Get(SmbUrl(kShareUrl));
+  EXPECT_FALSE(info);
+  EXPECT_TRUE(registry.GetAll().empty());
+}
+
 }  // namespace smb_client
 }  // namespace chromeos

chrome/common/pref_names.cc

@@ -970,6 +970,9 @@ const char kNetworkFileSharesPreconfiguredShares[] =
 const char kMostRecentlyUsedNetworkFileShareURL[] =
     "network_file_shares.most_recently_used_url";
 
+// List of network files shares added by the user.
+const char kNetworkFileSharesSavedShares[] = "network_file_shares.saved_shares";
+
 // A string pref storing the path of device wallpaper image file.
 const char kDeviceWallpaperImageFilePath[] =
     "policy.device_wallpaper_image_file_path";

chrome/common/pref_names.h

@@ -306,6 +306,7 @@ extern const char kReleaseNotesSuggestionChipTimesLeftToShow[];
 extern const char kNTLMShareAuthenticationEnabled[];
 extern const char kNetworkFileSharesPreconfiguredShares[];
 extern const char kMostRecentlyUsedNetworkFileShareURL[];
+extern const char kNetworkFileSharesSavedShares[];
 extern const char kParentAccessCodeConfig[];
 extern const char kPerAppTimeLimitsAppActivities[];
 extern const char kPerAppTimeLimitsLastResetTime[];