Revert "Enables the upgrade of Crostini container from Settings."

This reverts commit 97e6c2b3.

Reason for revert: suspect of causing unit test failures on
msan builds: 
https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests

Original change's description:
> Enables the upgrade of Crostini container from Settings.
> 
> We restart vm when upgrading, if it wasn't running already.
> 
> Bug: 1024693
> Change-Id: I00e093f7bbf83eb04a6adb4029a4612e99950ae4
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026559
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Reviewed-by: David Munro <davidmunro@google.com>
> Commit-Queue: Nicholas Verne <nverne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#738422}

TBR=khorimoto@chromium.org,nverne@chromium.org,hollingum@google.com,davidmunro@google.com

Change-Id: I41e8d20c971ac1392e998cc2d791a3053b7fa8e2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1024693
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036189Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738507}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -788,13 +788,15 @@ bool CrostiniManager::ShouldPromptContainerUpgrade(
     return false;
   }
   bool upgradable = IsContainerUpgradeable(container_id);
+  if (upgradable) {
+    // Currently a true return value from this function  implies the
+    // prompt must be shown. Storing this state in CrostiniManager implies that
+    // the prompt will only be shown once per session.
+    container_upgrade_prompt_shown_.insert(container_id);
+  }
   return upgradable;
 }
 
-void CrostiniManager::UpgradePromptShown(const ContainerId& container_id) {
-  container_upgrade_prompt_shown_.insert(container_id);
-}
-
 base::Optional<ContainerInfo> CrostiniManager::GetContainerInfo(
     std::string vm_name,
     std::string container_name) {
@@ -1543,23 +1545,6 @@ vm_tools::cicerone::UpgradeContainerRequest::Version ConvertVersion(
   }
 }
 
-// Watches the Crostini restarter until the VM started phase, then aborts the
-// sequence.
-class AbortOnVmStartObserver : public CrostiniManager::RestartObserver {
- public:
-  explicit AbortOnVmStartObserver(
-      base::WeakPtr<CrostiniManager> crostini_manager)
-      : crostini_manager_(crostini_manager) {}
-  void OnVmStarted(bool success) override {
-    if (crostini_manager_) {
-      crostini_manager_->AbortRestartCrostini(restart_id_, base::DoNothing());
-    }
-  }
-
- private:
-  base::WeakPtr<CrostiniManager> crostini_manager_;
-};
-
 }  // namespace
 
 void CrostiniManager::UpgradeContainer(const ContainerId& key,
@@ -1592,34 +1577,10 @@ void CrostiniManager::UpgradeContainer(const ContainerId& key,
   request.set_container_name(container_name);
   request.set_source_version(ConvertVersion(source_version));
   request.set_target_version(ConvertVersion(target_version));
-
-  CrostiniResultCallback do_upgrade_container = base::BindOnce(
-      [](base::WeakPtr<CrostiniManager> crostini_manager,
-         vm_tools::cicerone::UpgradeContainerRequest request,
-         CrostiniResultCallback final_callback, CrostiniResult result) {
-        // When we fail to start the VM, we can't continue the upgrade.
-        if (result != CrostiniResult::SUCCESS &&
-            result != CrostiniResult::RESTART_ABORTED) {
-          LOG(ERROR) << "Failed to restart the vm before attempting container "
-                        "upgrade. Result code "
-                     << static_cast<int>(result);
-          std::move(final_callback)
-              .Run(CrostiniResult::UPGRADE_CONTAINER_FAILED);
-          return;
-        }
-        GetCiceroneClient()->UpgradeContainer(
-            std::move(request),
-            base::BindOnce(&CrostiniManager::OnUpgradeContainer,
-                           crostini_manager, std::move(final_callback)));
-      },
-      weak_ptr_factory_.GetWeakPtr(), std::move(request), std::move(callback));
-
-  if (!IsVmRunning(vm_name)) {
-    RestartCrostini(vm_name, container_name, std::move(do_upgrade_container),
-                    new AbortOnVmStartObserver(weak_ptr_factory_.GetWeakPtr()));
-  } else {
-    std::move(do_upgrade_container).Run(CrostiniResult::SUCCESS);
-  }
+  GetCiceroneClient()->UpgradeContainer(
+      std::move(request),
+      base::BindOnce(&CrostiniManager::OnUpgradeContainer,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void CrostiniManager::CancelUpgradeContainer(const ContainerId& key,

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -138,9 +138,6 @@ class CrostiniManager : public KeyedService,
   // Callback indicating success or failure
   using BoolCallback = base::OnceCallback<void(bool success)>;
 
-  using RestartId = int;
-  static const RestartId kUninitializedRestartId = -1;
-
   // Observer class for the Crostini restart flow.
   class RestartObserver {
    public:
@@ -158,10 +155,6 @@ class CrostiniManager : public KeyedService,
     virtual void OnContainerStarted(CrostiniResult result) {}
     virtual void OnSshKeysFetched(bool success) {}
     virtual void OnContainerMounted(bool success) {}
-
-   protected:
-    friend class CrostiniManager;
-    RestartId restart_id_;
   };
 
   struct RestartOptions {
@@ -412,6 +405,8 @@ class CrostiniManager : public KeyedService,
                        uint8_t guest_port,
                        BoolCallback callback);
 
+  using RestartId = int;
+  static const RestartId kUninitializedRestartId = -1;
   // Runs all the steps required to restart the given crostini vm and container.
   // The optional |observer| tracks progress. If provided, it must be alive
   // until the restart completes (i.e. when |callback| is called) or the restart
@@ -584,7 +579,6 @@ class CrostiniManager : public KeyedService,
 
   bool IsContainerUpgradeable(const ContainerId& container_id);
   bool ShouldPromptContainerUpgrade(const ContainerId& container_id);
-  void UpgradePromptShown(const ContainerId& container_id);
 
  private:
   class CrostiniRestarter;

chrome/browser/chromeos/crostini/crostini_util.cc

@@ -341,11 +341,9 @@ bool ShouldConfigureDefaultContainer(Profile* profile) {
          !default_container_configured && !ansible_playbook_file_path.empty();
 }
 
-bool ShouldAllowContainerUpgrade(Profile* profile) {
-  return CrostiniFeatures::Get()->IsContainerUpgradeUIAllowed(profile) &&
-         crostini::CrostiniManager::GetForProfile(profile)
-             ->IsContainerUpgradeable(ContainerId(
-                 kCrostiniDefaultVmName, kCrostiniDefaultContainerName));
+// TODO(davidmunro): Answer based on flag and current container version.
+bool ShouldAllowContainerUpgrade() {
+  return false;
 }
 
 void LaunchCrostiniApp(Profile* profile,

chrome/browser/chromeos/crostini/crostini_util.h

@@ -65,7 +65,7 @@ bool IsUninstallable(Profile* profile, const std::string& app_id);
 bool IsCrostiniRunning(Profile* profile);
 
 // Whether the user is able to perform a container upgrade.
-bool ShouldAllowContainerUpgrade(Profile* profile);
+bool ShouldAllowContainerUpgrade();
 
 // Returns whether default Crostini container should be configured according to
 // the configuration specified by CrostiniAnsiblePlaybook user policy.

chrome/browser/ui/webui/chromeos/crostini_upgrader/crostini_upgrader_dialog.cc

@@ -5,7 +5,6 @@
 #include "chrome/browser/ui/webui/chromeos/crostini_upgrader/crostini_upgrader_dialog.h"
 
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/chromeos/crostini_upgrader/crostini_upgrader_ui.h"
 #include "chrome/common/webui_url_constants.h"
 
@@ -63,10 +62,6 @@ bool CrostiniUpgraderDialog::CanCloseDialog() const {
 void CrostiniUpgraderDialog::OnDialogShown(content::WebUI* webui) {
   upgrader_ui_ = static_cast<CrostiniUpgraderUI*>(webui->GetController());
   upgrader_ui_->set_launch_closure(std::move(launch_closure_));
-  crostini::CrostiniManager::GetForProfile(Profile::FromWebUI(webui))
-      ->UpgradePromptShown(
-          crostini::ContainerId(crostini::kCrostiniDefaultVmName,
-                                crostini::kCrostiniDefaultContainerName));
   return SystemWebDialogDelegate::OnDialogShown(webui);
 }
 

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

@@ -27,7 +27,6 @@
 #include "components/prefs/pref_service.h"
 #include "components/user_manager/user_manager.h"
 #include "content/public/browser/browser_thread.h"
-#include "ui/display/screen.h"
 
 namespace chromeos {
 namespace settings {
@@ -299,6 +298,23 @@ void CrostiniHandler::OnCrostiniInstallerViewStatusChanged(bool status) {
   }
 }
 
+void CrostiniHandler::OnCrostiniExportImportOperationStatusChanged(
+    bool in_progress) {
+  // Other side listens with cr.addWebUIListener
+  FireWebUIListener("crostini-export-import-operation-status-changed",
+                    base::Value(in_progress));
+}
+
+void CrostiniHandler::HandleQueryArcAdbRequest(const base::ListValue* args) {
+  AllowJavascript();
+  CHECK_EQ(0U, args->GetSize());
+
+  chromeos::SessionManagerClient* client =
+      chromeos::SessionManagerClient::Get();
+  client->QueryAdbSideload(base::Bind(&CrostiniHandler::OnQueryAdbSideload,
+                                      weak_ptr_factory_.GetWeakPtr()));
+}
+
 void CrostiniHandler::OnQueryAdbSideload(
     SessionManagerClient::AdbSideloadResponseCode response_code,
     bool enabled) {
@@ -368,34 +384,10 @@ bool CrostiniHandler::CheckEligibilityToChangeArcAdbSideloading() const {
   return true;
 }
 
-void CrostiniHandler::LaunchTerminal() {
-  crostini::LaunchCrostiniApp(
-      profile_, crostini::GetTerminalId(),
-      display::Screen::GetScreen()->GetPrimaryDisplay().id());
-}
-
 void CrostiniHandler::HandleRequestContainerUpgradeView(
     const base::ListValue* args) {
   CHECK_EQ(0U, args->GetSize());
-  chromeos::CrostiniUpgraderDialog::Show(base::BindOnce(
-      &CrostiniHandler::LaunchTerminal, weak_ptr_factory_.GetWeakPtr()));
-}
-
-void CrostiniHandler::OnCrostiniExportImportOperationStatusChanged(
-    bool in_progress) {
-  // Other side listens with cr.addWebUIListener
-  FireWebUIListener("crostini-export-import-operation-status-changed",
-                    base::Value(in_progress));
-}
-
-void CrostiniHandler::HandleQueryArcAdbRequest(const base::ListValue* args) {
-  AllowJavascript();
-  CHECK_EQ(0U, args->GetSize());
-
-  chromeos::SessionManagerClient* client =
-      chromeos::SessionManagerClient::Get();
-  client->QueryAdbSideload(base::Bind(&CrostiniHandler::OnQueryAdbSideload,
-                                      weak_ptr_factory_.GetWeakPtr()));
+  chromeos::CrostiniUpgraderDialog::Show(base::DoNothing());
 }
 
 }  // namespace settings

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

@@ -74,8 +74,6 @@ class CrostiniHandler : public ::settings::SettingsPageUIHandler,
   void HandleEnableArcAdbRequest(const base::ListValue* args);
   // Handle a request for disabling adb sideloading in ARC.
   void HandleDisableArcAdbRequest(const base::ListValue* args);
-  // Launch the Crostini terminal.
-  void LaunchTerminal();
   // Handle a request for showing the container upgrade view.
   void HandleRequestContainerUpgradeView(const base::ListValue* args);
   // Callback of HandleQueryArcAdbRequest.

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

@@ -647,7 +647,7 @@ void AddCrostiniStrings(content::WebUIDataSource* html_source,
   html_source->AddBoolean("isEnterpriseManaged",
                           IsDeviceManaged() || IsProfileManaged(profile));
   html_source->AddBoolean("showCrostiniContainerUpgrade",
-                          crostini::ShouldAllowContainerUpgrade(profile));
+                          crostini::ShouldAllowContainerUpgrade());
 }
 
 void AddPluginVmStrings(content::WebUIDataSource* html_source,