Add CrostiniManager::RestartCrostiniWithOptions()

This is the first step to allow users to install Crostini with a custom
username.

Bug: 1016195
Change-Id: I18b6bd3c99198ff733c987dd25ae2d46029a6c9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1880411Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Jason Lin <lxj@google.com>
Cr-Commit-Position: refs/heads/master@{#712047}

chrome/browser/chromeos/crostini/crostini_installer.cc

@@ -176,7 +176,8 @@ void CrostiniInstaller::Shutdown() {
   }
 }
 
-void CrostiniInstaller::Install(ProgressCallback progress_callback,
+void CrostiniInstaller::Install(CrostiniManager::RestartOptions options,
+                                ProgressCallback progress_callback,
                                 ResultCallback result_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
@@ -187,6 +188,7 @@ void CrostiniInstaller::Install(ProgressCallback progress_callback,
     return;
   }
 
+  restart_options_ = std::move(options);
   progress_callback_ = std::move(progress_callback);
   result_callback_ = std::move(result_callback);
 
@@ -608,12 +610,14 @@ void CrostiniInstaller::OnAvailableDiskSpace(int64_t bytes) {
 
   // Kick off the Crostini Restart sequence. We will be added as an observer.
   restart_id_ =
-      crostini::CrostiniManager::GetForProfile(profile_)->RestartCrostini(
-          crostini::kCrostiniDefaultVmName,
-          crostini::kCrostiniDefaultContainerName,
-          base::BindOnce(&CrostiniInstaller::OnCrostiniRestartFinished,
-                         weak_ptr_factory_.GetWeakPtr()),
-          this);
+      crostini::CrostiniManager::GetForProfile(profile_)
+          ->RestartCrostiniWithOptions(
+              crostini::kCrostiniDefaultVmName,
+              crostini::kCrostiniDefaultContainerName,
+              std::move(restart_options_),
+              base::BindOnce(&CrostiniInstaller::OnCrostiniRestartFinished,
+                             weak_ptr_factory_.GetWeakPtr()),
+              this);
 
   // |restart_id| will be invalid when |CrostiniManager::RestartCrostini()|
   // decides to fail immediately and calls |OnCrostiniRestartFinished()|, which

chrome/browser/chromeos/crostini/crostini_installer.h

@@ -63,7 +63,8 @@ class CrostiniInstaller : public KeyedService,
   void Shutdown() override;
 
   // CrostiniInstallerUIDelegate:
-  void Install(ProgressCallback progress_callback,
+  void Install(CrostiniManager::RestartOptions options,
+               ProgressCallback progress_callback,
                ResultCallback result_callback) override;
   void Cancel(base::OnceClosure callback) override;
   void CancelBeforeStart() override;
@@ -126,6 +127,7 @@ class CrostiniInstaller : public KeyedService,
   int32_t container_download_percent_;
   crostini::CrostiniManager::RestartId restart_id_ =
       crostini::CrostiniManager::kUninitializedRestartId;
+  CrostiniManager::RestartOptions restart_options_;
 
   bool skip_launching_terminal_for_testing_ = false;
 

chrome/browser/chromeos/crostini/crostini_installer_ui_delegate.h

@@ -8,6 +8,7 @@
 #include "base/callback_forward.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/chromeos/crostini/crostini_installer_types.mojom-forward.h"
+#include "chrome/browser/chromeos/crostini/crostini_manager.h"
 
 namespace crostini {
 
@@ -34,7 +35,8 @@ class CrostiniInstallerUIDelegate {
   // Start the installation. |progress_callback| will be called multiple times
   // until |result_callback| is called. The crostini terminal will be launched
   // when the installation succeeds.
-  virtual void Install(ProgressCallback progress_callback,
+  virtual void Install(CrostiniManager::RestartOptions options,
+                       ProgressCallback progress_callback,
                        ResultCallback result_callback) = 0;
 
   // Cancel the ongoing installation. |callback| will be called when it

chrome/browser/chromeos/crostini/crostini_installer_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -140,6 +141,7 @@ class CrostiniInstallerTest : public testing::Test {
 
   void Install() {
     crostini_installer_->Install(
+        CrostiniManager::RestartOptions{},
         base::BindRepeating(&MockCallbacks::OnProgress,
                             base::Unretained(&mock_callbacks_)),
         base::BindRepeating(&MockCallbacks::OnFinished,
@@ -256,7 +258,8 @@ TEST_F(CrostiniInstallerTest, CancelAfterStart) {
 TEST_F(CrostiniInstallerTest, CancelAfterStartBeforeCheckDisk) {
   EXPECT_CALL(mock_callbacks_, OnCanceled());
 
-  crostini_installer_->Install(base::DoNothing(), base::DoNothing());
+  crostini_installer_->Install(CrostiniManager::RestartOptions{},
+                               base::DoNothing(), base::DoNothing());
   Cancel();  // Cancel immediately
 
   task_environment_.RunUntilIdle();

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -110,6 +110,12 @@ void InvokeAndErasePendingContainerCallbacks(
 
 }  // namespace
 
+CrostiniManager::RestartOptions::RestartOptions() = default;
+CrostiniManager::RestartOptions::RestartOptions(RestartOptions&&) = default;
+CrostiniManager::RestartOptions::~RestartOptions() = default;
+CrostiniManager::RestartOptions& CrostiniManager::RestartOptions::operator=(
+    RestartOptions&&) = default;
+
 class CrostiniManager::CrostiniRestarter
     : public base::RefCountedThreadSafe<CrostiniRestarter>,
       public crostini::VmShutdownObserver,
@@ -119,11 +125,13 @@ class CrostiniManager::CrostiniRestarter
                     CrostiniManager* crostini_manager,
                     std::string vm_name,
                     std::string container_name,
+                    RestartOptions options,
                     CrostiniManager::CrostiniResultCallback callback)
       : profile_(profile),
         crostini_manager_(crostini_manager),
         vm_name_(std::move(vm_name)),
         container_name_(std::move(container_name)),
+        options_(std::move(options)),
         completed_callback_(std::move(callback)),
         restart_id_(next_restart_id_++) {
     crostini_manager_->AddVmShutdownObserver(this);
@@ -363,7 +371,9 @@ class CrostiniManager::CrostiniRestarter
     }
     StartStage(mojom::InstallerState::kSetupContainer);
     crostini_manager_->SetUpLxdContainerUser(
-        vm_name_, container_name_, DefaultContainerUserNameForProfile(profile_),
+        vm_name_, container_name_,
+        options_.container_username.value_or(
+            DefaultContainerUserNameForProfile(profile_)),
         base::BindOnce(&CrostiniRestarter::SetUpLxdContainerUserFinished,
                        this));
   }
@@ -523,6 +533,7 @@ class CrostiniManager::CrostiniRestarter
 
   std::string vm_name_;
   std::string container_name_;
+  RestartOptions options_;
   std::string source_path_;
   bool is_initial_install_ = true;
   CrostiniManager::CrostiniResultCallback completed_callback_;
@@ -1721,6 +1732,17 @@ CrostiniManager::RestartId CrostiniManager::RestartCrostini(
     std::string container_name,
     CrostiniResultCallback callback,
     RestartObserver* observer) {
+  return RestartCrostiniWithOptions(std::move(vm_name),
+                                    std::move(container_name), RestartOptions{},
+                                    std::move(callback), observer);
+}
+
+CrostiniManager::RestartId CrostiniManager::RestartCrostiniWithOptions(
+    std::string vm_name,
+    std::string container_name,
+    RestartOptions options,
+    CrostiniResultCallback callback,
+    RestartObserver* observer) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // Currently, |remove_crostini_callbacks_| is only used just before running
   // CrostiniRemover. If that changes, then we should check for a currently
@@ -1734,7 +1756,7 @@ CrostiniManager::RestartId CrostiniManager::RestartCrostini(
 
   auto restarter = base::MakeRefCounted<CrostiniRestarter>(
       profile_, this, std::move(vm_name), std::move(container_name),
-      std::move(callback));
+      std::move(options), std::move(callback));
   if (observer)
     restarter->AddObserver(observer);
   auto key = ContainerId(restarter->vm_name(), restarter->container_name());

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -151,6 +151,17 @@ class CrostiniManager : public KeyedService,
     virtual void OnContainerMounted(bool success) = 0;
   };
 
+  struct RestartOptions {
+    // This normally will not have effect on existing container.
+    base::Optional<std::string> container_username;
+
+    RestartOptions();
+    ~RestartOptions();
+    // Add copy version if necessary.
+    RestartOptions(RestartOptions&&);
+    RestartOptions& operator=(RestartOptions&&);
+  };
+
   static CrostiniManager* GetForProfile(Profile* profile);
 
   explicit CrostiniManager(Profile* profile);
@@ -404,6 +415,12 @@ class CrostiniManager : public KeyedService,
                             CrostiniResultCallback callback,
                             RestartObserver* observer = nullptr);
 
+  RestartId RestartCrostiniWithOptions(std::string vm_name,
+                                       std::string container_name,
+                                       RestartOptions options,
+                                       CrostiniResultCallback callback,
+                                       RestartObserver* observer = nullptr);
+
   // Aborts a restart. A "next" restarter with the same <vm_name,
   // container_name> will run, if there is one. |callback| will be called once
   // the restart has finished aborting

chrome/browser/chromeos/crostini/crostini_manager_unittest.cc

@@ -734,6 +734,31 @@ TEST_F(CrostiniManagerRestartTest, RestartSuccess) {
   // Mount only performed for termina/penguin.
   EXPECT_FALSE(fake_concierge_client_->get_container_ssh_keys_called());
   EXPECT_EQ(1, restart_crostini_callback_count_);
+
+  base::Optional<ContainerInfo> container_info =
+      crostini_manager()->GetContainerInfo(kVmName, kContainerName);
+  EXPECT_EQ(container_info.value().username,
+            DefaultContainerUserNameForProfile(profile()));
+}
+
+TEST_F(CrostiniManagerRestartTest, RestartSuccessWithOptions) {
+  CrostiniManager::RestartOptions options;
+  options.container_username = "helloworld";
+  restart_id_ = crostini_manager()->RestartCrostiniWithOptions(
+      kVmName, kContainerName, std::move(options),
+      base::BindOnce(&CrostiniManagerRestartTest::RestartCrostiniCallback,
+                     base::Unretained(this), run_loop()->QuitClosure()),
+      this);
+  run_loop()->Run();
+  EXPECT_TRUE(fake_concierge_client_->create_disk_image_called());
+  EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());
+  // Mount only performed for termina/penguin.
+  EXPECT_FALSE(fake_concierge_client_->get_container_ssh_keys_called());
+  EXPECT_EQ(1, restart_crostini_callback_count_);
+
+  base::Optional<ContainerInfo> container_info =
+      crostini_manager()->GetContainerInfo(kVmName, kContainerName);
+  EXPECT_EQ(container_info.value().username, "helloworld");
 }
 
 TEST_F(CrostiniManagerRestartTest, AbortOnComponentLoaded) {

chrome/browser/chromeos/crostini/fake_crostini_installer_ui_delegate.cc

@@ -10,8 +10,10 @@ FakeCrostiniInstallerUIDelegate::FakeCrostiniInstallerUIDelegate() = default;
 FakeCrostiniInstallerUIDelegate::~FakeCrostiniInstallerUIDelegate() = default;
 
 void FakeCrostiniInstallerUIDelegate::Install(
+    CrostiniManager::RestartOptions options,
     ProgressCallback progress_callback,
     ResultCallback result_callback) {
+  restart_options_ = std::move(options);
   progress_callback_ = std::move(progress_callback);
   result_callback_ = std::move(result_callback);
 }

chrome/browser/chromeos/crostini/fake_crostini_installer_ui_delegate.h

@@ -7,6 +7,7 @@
 
 #include "base/callback.h"
 #include "chrome/browser/chromeos/crostini/crostini_installer_ui_delegate.h"
+#include "chrome/browser/chromeos/crostini/crostini_manager.h"
 
 namespace crostini {
 
@@ -15,11 +16,13 @@ class FakeCrostiniInstallerUIDelegate : public CrostiniInstallerUIDelegate {
   FakeCrostiniInstallerUIDelegate();
   ~FakeCrostiniInstallerUIDelegate();
 
-  void Install(ProgressCallback progress_callback,
+  void Install(CrostiniManager::RestartOptions options,
+               ProgressCallback progress_callback,
                ResultCallback result_callback) override;
   void Cancel(base::OnceClosure callback) override;
   void CancelBeforeStart() override;
 
+  CrostiniManager::RestartOptions restart_options_;
   ProgressCallback progress_callback_;
   ResultCallback result_callback_;
   base::OnceClosure cancel_callback_;

chrome/browser/ui/views/crostini/crostini_installer_view.cc

@@ -226,6 +226,7 @@ bool CrostiniInstallerView::Accept() {
 
   VLOG(1) << "delegate_->Install()";
   delegate_->Install(
+      crostini::CrostiniManager::RestartOptions{},
       base::BindRepeating(&CrostiniInstallerView::OnProgressUpdate,
                           weak_ptr_factory_.GetWeakPtr()),
       base::BindOnce(&CrostiniInstallerView::OnInstallFinished,

chrome/browser/ui/webui/chromeos/crostini_installer/crostini_installer_page_handler.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/optional.h"
 #include "chrome/browser/chromeos/crostini/crostini_installer_ui_delegate.h"
 
 namespace chromeos {
@@ -25,7 +26,10 @@ CrostiniInstallerPageHandler::CrostiniInstallerPageHandler(
 CrostiniInstallerPageHandler::~CrostiniInstallerPageHandler() = default;
 
 void CrostiniInstallerPageHandler::Install() {
+  // TODO(crbug.com/1016195): Web page should allow input container username,
+  // and here we will pass that to Install().
   installer_ui_delegate_->Install(
+      crostini::CrostiniManager::RestartOptions{},
       base::BindRepeating(&CrostiniInstallerPageHandler::OnProgressUpdate,
                           weak_ptr_factory_.GetWeakPtr()),
       base::BindOnce(&CrostiniInstallerPageHandler::OnInstallFinished,