Move enums from CrostiniInstallerUIDelegate to a mojom file

Enums |InstallationState| and |Error| in class |CrostiniInstallerUIDelegate|
are moved to file crostini_installer_types.mojom and renamed to
`InstallerState` and `InstallerError`. All other changes to .h and .cc files
are just simple replacement to migrate to the new types.

The purpose of this CL is to allow the js code of the new Crostini WebUI
installer to use the same enums.

Bug: 929571
Change-Id: I185bb94b79187db98850dba4d23a7ba221e1d1e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815797Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Jason Lin <lxj@google.com>
Cr-Commit-Position: refs/heads/master@{#698836}

chrome/browser/chromeos/BUILD.gn

@@ -54,6 +54,7 @@ source_set("chromeos") {
     ":print_job_info_proto",
     ":screen_brightness_event_proto",
     ":user_activity_event_proto",
+    "crostini:crostini_installer_types_mojom",
     "//apps",
     "//ash",
     "//ash/public/cpp",

chrome/browser/chromeos/crostini/BUILD.gn

+# Copyright 2019 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.
+
+import("//mojo/public/tools/bindings/mojom.gni")
+
+mojom("crostini_installer_types_mojom") {
+  sources = [
+    "crostini_installer_types.mojom",
+  ]
+}

chrome/browser/chromeos/crostini/OWNERS

@@ -3,4 +3,7 @@ joelhockey@chromium.org
 nverne@chromium.org
 timloh@chromium.org
 
+per-file *.mojom=set noparent
+per-file *.mojom=file://ipc/SECURITY_OWNERS
+
 # COMPONENT: UI>Shell>Containers

chrome/browser/chromeos/crostini/crostini_installer.h

@@ -101,9 +101,10 @@ class CrostiniInstaller : public KeyedService,
 
   void RunProgressCallback();
   void UpdateState(State new_state);
-  void UpdateInstallingState(InstallationState new_installing_state,
-                             bool run_callback = true);
-  void HandleError(Error error);
+  void UpdateInstallingState(
+      crostini::mojom::InstallerState new_installing_state,
+      bool run_callback = true);
+  void HandleError(crostini::mojom::InstallerError error);
   void FinishCleanup(crostini::CrostiniResult result);
   void RecordSetupResult(SetupResult result);
 
@@ -113,7 +114,7 @@ class CrostiniInstaller : public KeyedService,
   Profile* profile_;
 
   State state_ = State::IDLE;
-  InstallationState installing_state_;
+  crostini::mojom::InstallerState installing_state_;
   base::TimeTicks install_start_time_;
   base::Time installing_state_start_time_;
   base::RepeatingTimer state_progress_timer_;

chrome/browser/chromeos/crostini/crostini_installer_types.mojom

+// Copyright 2019 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.
+
+module crostini.mojom;
+
+enum InstallerState {
+  kStart,               // Just started installation
+  kInstallImageLoader,  // Loading the Termina VM component.
+  kStartConcierge,      // Starting the Concierge D-Bus client.
+  kCreateDiskImage,     // Creating the image for the Termina VM.
+  kStartTerminaVm,      // Starting the Termina VM.
+  kCreateContainer,     // Creating the container inside the Termina VM.
+  kSetupContainer,      // Setting up the container inside the Termina VM.
+  kStartContainer,      // Starting the container inside the Termina VM.
+  kFetchSshKeys,        // Fetch ssh keys from concierge.
+  kMountContainer,      // Do sshfs mount of container.
+};
+
+enum InstallerError {
+  kNone,
+  kErrorLoadingTermina,
+  kErrorStartingConcierge,
+  kErrorCreatingDiskImage,
+  kErrorStartingTermina,
+  kErrorStartingContainer,
+  kErrorOffline,
+  kErrorFetchingSshKeys,
+  kErrorMountingContainer,
+  kErrorSettingUpContainer,
+  kErrorInsufficientDiskSpace,
+};

chrome/browser/chromeos/crostini/crostini_installer_ui_delegate.h

@@ -7,6 +7,7 @@
 
 #include "base/callback_forward.h"
 #include "base/strings/string16.h"
+#include "chrome/browser/chromeos/crostini/crostini_installer_types.mojom-forward.h"
 
 namespace crostini {
 
@@ -23,38 +24,12 @@ class CrostiniInstallerUIDelegate {
       crostini::CrostiniInstallerUIDelegate::kDownloadSizeInBytes +
       kMinimumDiskSize;
 
-  enum class InstallationState {
-    START,                 // Just started installation
-    INSTALL_IMAGE_LOADER,  // Loading the Termina VM component.
-    START_CONCIERGE,       // Starting the Concierge D-Bus client.
-    CREATE_DISK_IMAGE,     // Creating the image for the Termina VM.
-    START_TERMINA_VM,      // Starting the Termina VM.
-    CREATE_CONTAINER,      // Creating the container inside the Termina VM.
-    SETUP_CONTAINER,       // Setting up the container inside the Termina VM.
-    START_CONTAINER,       // Starting the container inside the Termina VM.
-    FETCH_SSH_KEYS,        // Fetch ssh keys from concierge.
-    MOUNT_CONTAINER,       // Do sshfs mount of container.
-  };
-
-  enum class Error {
-    NONE,
-    ERROR_LOADING_TERMINA,
-    ERROR_STARTING_CONCIERGE,
-    ERROR_CREATING_DISK_IMAGE,
-    ERROR_STARTING_TERMINA,
-    ERROR_STARTING_CONTAINER,
-    ERROR_OFFLINE,
-    ERROR_FETCHING_SSH_KEYS,
-    ERROR_MOUNTING_CONTAINER,
-    ERROR_SETTING_UP_CONTAINER,
-    ERROR_INSUFFICIENT_DISK_SPACE,
-  };
-
   // |progress_fraction| ranges from 0.0 to 1.0.
   using ProgressCallback =
-      base::RepeatingCallback<void(InstallationState state,
+      base::RepeatingCallback<void(crostini::mojom::InstallerState state,
                                    double progress_fraction)>;
-  using ResultCallback = base::OnceCallback<void(Error error)>;
+  using ResultCallback =
+      base::OnceCallback<void(crostini::mojom::InstallerError error)>;
 
   // Start the installation. |progress_callback| will be called multiple times
   // until |result_callback| is called. The crostini terminal will be launched

chrome/browser/chromeos/crostini/crostini_installer_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "chrome/browser/chromeos/crostini/crostini_installer_types.mojom.h"
 #include "chrome/browser/chromeos/crostini/crostini_installer_ui_delegate.h"
 #include "chrome/browser/chromeos/crostini/crostini_test_helper.h"
 #include "chrome/test/base/testing_profile.h"
@@ -22,6 +23,8 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using crostini::mojom::InstallerError;
+using crostini::mojom::InstallerState;
 using testing::_;
 using testing::AnyNumber;
 using testing::Expectation;
@@ -38,14 +41,12 @@ class CrostiniInstallerTest : public testing::Test {
  public:
   using ResultCallback = CrostiniInstallerUIDelegate::ResultCallback;
   using ProgressCallback = CrostiniInstallerUIDelegate::ProgressCallback;
-  using Error = CrostiniInstallerUIDelegate::Error;
-  using InstallationState = CrostiniInstallerUIDelegate::InstallationState;
 
   class MockCallbacks {
    public:
     MOCK_METHOD2(OnProgress,
-                 void(InstallationState state, double progress_fraction));
-    MOCK_METHOD1(OnFinished, void(Error error));
+                 void(InstallerState state, double progress_fraction));
+    MOCK_METHOD1(OnFinished, void(InstallerError error));
     MOCK_METHOD0(OnCanceled, void());
   };
 
@@ -184,7 +185,8 @@ TEST_F(CrostiniInstallerTest, InstallFlow) {
                   MountPath("sshfs://testing_profile@hostname:", _, _, _, _, _))
           .WillOnce(Invoke(&mount_path_waiter_, &MountPathWaiter::MountPath));
   // |OnProgress()| should not happens after |OnFinished()|
-  EXPECT_CALL(mock_callbacks_, OnFinished(Error::NONE)).After(expectation_set);
+  EXPECT_CALL(mock_callbacks_, OnFinished(InstallerError::kNone))
+      .After(expectation_set);
 
   Install();
   mount_path_waiter_.WaitForMountPathCalled();
@@ -268,11 +270,12 @@ TEST_F(CrostiniInstallerTest, CancelAfterStartBeforeCheckDisk) {
       << "Installer should recover to installable state";
 }
 
-TEST_F(CrostiniInstallerTest, Error) {
+TEST_F(CrostiniInstallerTest, InstallerError) {
   Expectation expect_progresses =
       EXPECT_CALL(mock_callbacks_, OnProgress(_, _)).Times(AnyNumber());
   // |OnProgress()| should not happens after |OnFinished()|
-  EXPECT_CALL(mock_callbacks_, OnFinished(Error::ERROR_STARTING_TERMINA))
+  EXPECT_CALL(mock_callbacks_,
+              OnFinished(InstallerError::kErrorStartingTermina))
       .After(expect_progresses);
 
   // Fake a failure for starting vm.

chrome/browser/ui/BUILD.gn

@@ -3709,6 +3709,7 @@ jumbo_split_static_library("ui") {
         "views/plugin_vm/plugin_vm_launcher_view.h",
       ]
       deps += [
+        "//chrome/browser/chromeos/crostini:crostini_installer_types_mojom",
         "//chrome/services/app_service/public/cpp:app_update",
         "//chrome/services/app_service/public/cpp:icon_loader",
         "//components/services/app_service/public/cpp:app_file_handling",

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

@@ -12,6 +12,7 @@
 #include "base/metrics/histogram_functions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/crostini/crostini_installer.h"
+#include "chrome/browser/chromeos/crostini/crostini_installer_types.mojom.h"
 #include "chrome/browser/chromeos/crostini/crostini_installer_ui_delegate.h"
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
 #include "chrome/browser/ui/browser_dialogs.h"
@@ -39,11 +40,11 @@
 #include "ui/views/window/dialog_client_view.h"
 
 using crostini::CrostiniResult;
+using crostini::mojom::InstallerError;
+using crostini::mojom::InstallerState;
 
 namespace {
 
-using Error = crostini::CrostiniInstallerUIDelegate::Error;
-
 CrostiniInstallerView* g_crostini_installer_view = nullptr;
 
 constexpr gfx::Insets kOOBEButtonRowInsets(32, 64, 32, 64);
@@ -65,36 +66,36 @@ base::string16 GetHelpUrlWithBoard(const std::string& original_url) {
                             "&b=" + base::SysInfo::GetLsbReleaseBoard());
 }
 
-base::string16 GetErrorMessage(Error error) {
+base::string16 GetErrorMessage(InstallerError error) {
   switch (error) {
-    case Error::ERROR_LOADING_TERMINA:
+    case InstallerError::kErrorLoadingTermina:
       return l10n_util::GetStringUTF16(
           IDS_CROSTINI_INSTALLER_LOAD_TERMINA_ERROR);
-    case Error::ERROR_STARTING_CONCIERGE:
+    case InstallerError::kErrorStartingConcierge:
       return l10n_util::GetStringUTF16(
           IDS_CROSTINI_INSTALLER_START_CONCIERGE_ERROR);
-    case Error::ERROR_CREATING_DISK_IMAGE:
+    case InstallerError::kErrorCreatingDiskImage:
       return l10n_util::GetStringUTF16(
           IDS_CROSTINI_INSTALLER_CREATE_DISK_IMAGE_ERROR);
-    case Error::ERROR_STARTING_TERMINA:
+    case InstallerError::kErrorStartingTermina:
       return l10n_util::GetStringUTF16(
           IDS_CROSTINI_INSTALLER_START_TERMINA_VM_ERROR);
-    case Error::ERROR_STARTING_CONTAINER:
+    case InstallerError::kErrorStartingContainer:
       return l10n_util::GetStringUTF16(
           IDS_CROSTINI_INSTALLER_START_CONTAINER_ERROR);
-    case Error::ERROR_OFFLINE:
+    case InstallerError::kErrorOffline:
       return l10n_util::GetStringFUTF16(IDS_CROSTINI_INSTALLER_OFFLINE_ERROR,
                                         ui::GetChromeOSDeviceName());
-    case Error::ERROR_FETCHING_SSH_KEYS:
+    case InstallerError::kErrorFetchingSshKeys:
       return l10n_util::GetStringUTF16(
           IDS_CROSTINI_INSTALLER_FETCH_SSH_KEYS_ERROR);
-    case Error::ERROR_MOUNTING_CONTAINER:
+    case InstallerError::kErrorMountingContainer:
       return l10n_util::GetStringUTF16(
           IDS_CROSTINI_INSTALLER_MOUNT_CONTAINER_ERROR);
-    case Error::ERROR_SETTING_UP_CONTAINER:
+    case InstallerError::kErrorSettingUpContainer:
       return l10n_util::GetStringUTF16(
           IDS_CROSTINI_INSTALLER_SETUP_CONTAINER_ERROR);
-    case Error::ERROR_INSUFFICIENT_DISK_SPACE:
+    case InstallerError::kErrorInsufficientDiskSpace:
       return l10n_util::GetStringFUTF16(
           IDS_CROSTINI_INSTALLER_INSUFFICIENT_DISK,
           ui::FormatBytesWithUnits(
@@ -212,7 +213,7 @@ bool CrostiniInstallerView::Accept() {
   learn_more_link_ = nullptr;
 
   state_ = State::INSTALLING;
-  installing_state_ = InstallationState::START;
+  installing_state_ = InstallerState::kStart;
   progress_bar_->SetValue(0);
   progress_bar_->SetVisible(true);
   SetMessageLabel();
@@ -367,7 +368,7 @@ CrostiniInstallerView::~CrostiniInstallerView() {
   g_crostini_installer_view = nullptr;
 }
 
-void CrostiniInstallerView::OnProgressUpdate(InstallationState installing_state,
+void CrostiniInstallerView::OnProgressUpdate(InstallerState installing_state,
                                              double progress_fraction) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK_EQ(state_, State::INSTALLING);
@@ -382,10 +383,10 @@ void CrostiniInstallerView::OnProgressUpdate(InstallationState installing_state,
   GetWidget()->GetRootView()->Layout();
 }
 
-void CrostiniInstallerView::OnInstallFinished(Error error) {
+void CrostiniInstallerView::OnInstallFinished(InstallerError error) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  if (error == Error::NONE) {
+  if (error == InstallerError::kNone) {
     state_ = State::SUCCESS;
     GetWidget()->Close();
     return;
@@ -417,34 +418,34 @@ void CrostiniInstallerView::SetMessageLabel() {
   int message_id = 0;
 
   switch (installing_state_) {
-    case InstallationState::INSTALL_IMAGE_LOADER:
+    case InstallerState::kInstallImageLoader:
       message_id = IDS_CROSTINI_INSTALLER_LOAD_TERMINA_MESSAGE;
       break;
-    case InstallationState::START_CONCIERGE:
+    case InstallerState::kStartConcierge:
       message_id = IDS_CROSTINI_INSTALLER_START_CONCIERGE_MESSAGE;
       break;
-    case InstallationState::CREATE_DISK_IMAGE:
+    case InstallerState::kCreateDiskImage:
       message_id = IDS_CROSTINI_INSTALLER_CREATE_DISK_IMAGE_MESSAGE;
       break;
-    case InstallationState::START_TERMINA_VM:
+    case InstallerState::kStartTerminaVm:
       message_id = IDS_CROSTINI_INSTALLER_START_TERMINA_VM_MESSAGE;
       break;
-    case InstallationState::CREATE_CONTAINER:
+    case InstallerState::kCreateContainer:
       // TODO(lxj): we are using the same message as for |START_CONTAINER|,
       // which is weird because user is going to see message "start container"
       // then "setup container" and then "start container" again.
       message_id = IDS_CROSTINI_INSTALLER_START_CONTAINER_MESSAGE;
       break;
-    case InstallationState::SETUP_CONTAINER:
+    case InstallerState::kSetupContainer:
       message_id = IDS_CROSTINI_INSTALLER_SETUP_CONTAINER_MESSAGE;
       break;
-    case InstallationState::START_CONTAINER:
+    case InstallerState::kStartContainer:
       message_id = IDS_CROSTINI_INSTALLER_START_CONTAINER_MESSAGE;
       break;
-    case InstallationState::FETCH_SSH_KEYS:
+    case InstallerState::kFetchSshKeys:
       message_id = IDS_CROSTINI_INSTALLER_FETCH_SSH_KEYS_MESSAGE;
       break;
-    case InstallationState::MOUNT_CONTAINER:
+    case InstallerState::kMountContainer:
       message_id = IDS_CROSTINI_INSTALLER_MOUNT_CONTAINER_MESSAGE;
       break;
     default:

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

@@ -43,10 +43,6 @@ class CrostiniInstallerView : public views::DialogDelegateView,
   void LinkClicked(views::Link* source, int event_flags) override;
 
  private:
-  using InstallationState =
-      crostini::CrostiniInstallerUIDelegate::InstallationState;
-  using Error = crostini::CrostiniInstallerUIDelegate::Error;
-
   enum class State {
     PROMPT,
     INSTALLING,
@@ -61,14 +57,14 @@ class CrostiniInstallerView : public views::DialogDelegateView,
       crostini::CrostiniInstallerUIDelegate* delegate);
   ~CrostiniInstallerView() override;
 
-  void OnProgressUpdate(InstallationState installing_state,
+  void OnProgressUpdate(crostini::mojom::InstallerState installing_state,
                         double progress_fraction);
-  void OnInstallFinished(Error error);
+  void OnInstallFinished(crostini::mojom::InstallerError error);
   void OnCanceled();
   void SetMessageLabel();
 
   State state_ = State::PROMPT;
-  InstallationState installing_state_;
+  crostini::mojom::InstallerState installing_state_;
   Profile* profile_;
   crostini::CrostiniInstallerUIDelegate* delegate_;
 

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

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/chromeos/crostini/crostini_installer_types.mojom.h"
 #include "chrome/browser/ui/views/crostini/crostini_installer_view.h"
 
 #include "base/bind.h"
@@ -22,9 +23,8 @@
 #include "ui/base/ui_base_types.h"
 #include "ui/views/window/dialog_client_view.h"
 
-using Error = crostini::CrostiniInstallerUIDelegate::Error;
-using InstallationState =
-    crostini::CrostiniInstallerUIDelegate::InstallationState;
+using crostini::mojom::InstallerError;
+using crostini::mojom::InstallerState;
 
 class CrostiniInstallerViewBrowserTest : public CrostiniDialogBrowserTest {
  public:
@@ -82,11 +82,9 @@ IN_PROC_BROWSER_TEST_F(CrostiniInstallerViewBrowserTest, InstallFlow) {
       << "Install() should be called";
   EXPECT_TRUE(fake_delegate_.result_callback_);
 
-  fake_delegate_.progress_callback_.Run(InstallationState::CREATE_CONTAINER,
-                                        0.4);
-  fake_delegate_.progress_callback_.Run(InstallationState::MOUNT_CONTAINER,
-                                        0.8);
-  std::move(fake_delegate_.result_callback_).Run(Error::NONE);
+  fake_delegate_.progress_callback_.Run(InstallerState::kCreateContainer, 0.4);
+  fake_delegate_.progress_callback_.Run(InstallerState::kMountContainer, 0.8);
+  std::move(fake_delegate_.result_callback_).Run(InstallerError::kNone);
 
   // This allow the dialog to be destructed.
   base::RunLoop().RunUntilIdle();
@@ -104,7 +102,7 @@ IN_PROC_BROWSER_TEST_F(CrostiniInstallerViewBrowserTest, ErrorThenCancel) {
 
   ASSERT_TRUE(fake_delegate_.result_callback_);
   std::move(fake_delegate_.result_callback_)
-      .Run(Error::ERROR_CREATING_DISK_IMAGE);
+      .Run(InstallerError::kErrorCreatingDiskImage);
 
   EXPECT_FALSE(ActiveView()->GetWidget()->IsClosed());
   EXPECT_TRUE(HasEnabledAcceptButton());
@@ -129,7 +127,7 @@ IN_PROC_BROWSER_TEST_F(CrostiniInstallerViewBrowserTest, ErrorThenRetry) {
 
   ASSERT_TRUE(fake_delegate_.result_callback_);
   std::move(fake_delegate_.result_callback_)
-      .Run(Error::ERROR_CREATING_DISK_IMAGE);
+      .Run(InstallerError::kErrorCreatingDiskImage);
 
   EXPECT_FALSE(ActiveView()->GetWidget()->IsClosed());
   EXPECT_TRUE(HasEnabledAcceptButton());
@@ -140,7 +138,7 @@ IN_PROC_BROWSER_TEST_F(CrostiniInstallerViewBrowserTest, ErrorThenRetry) {
   EXPECT_TRUE(fake_delegate_.result_callback_)
       << "Install() should be called again";
 
-  std::move(fake_delegate_.result_callback_).Run(Error::NONE);
+  std::move(fake_delegate_.result_callback_).Run(InstallerError::kNone);
 
   // This allow the dialog to be destructed.
   base::RunLoop().RunUntilIdle();