Validate crostini container architecture when importing

Modified export/import handling from CrostiniManager to
take a CrostiniResultMessageCallback which receives both
a CrostiniResult and a base::string16 error message which
can be displayed in the notification.

Strings from https://docs.google.com/spreadsheets/d/1paVaVV2azeuAHnxHT4jQ402VvaZ7Z8btZ5e_y7rPRnE/edit#gid=0

Bug: 933948
Change-Id: I2451492d85cd8263a8914a31b82162d4e93a77de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1558559
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649780}

chrome/app/chromeos_strings.grdp

@@ -3578,6 +3578,9 @@
   <message name="IDS_CROSTINI_EXPORT_NOTIFICATION_MESSAGE_FAILED" desc="Message displayed in the notification when export (backup) of the Linux container fails.">
     Backup couldn't be completed due to an error
   </message>
+  <message name="IDS_CROSTINI_EXPORT_NOTIFICATION_MESSAGE_FAILED_IN_PROGRESS" desc="Message displayed in the notification when export (backup) of the Linux container fails due to another being in progress.">
+    Backup currently in progress for <ph name="CONTAINER_ID">$1<ex>(termina, penguin)</ex></ph>
+  </message>
   <message name="IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_RUNNING" desc="Title displayed in the notification while import (restore) of the Linux container is in progress.">
     Restoring Linux apps &amp; files
   </message>
@@ -3593,6 +3596,12 @@
   <message name="IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED" desc="Message displayed in the notification when import (restore) of the Linux container fails.">
     Restoring couldn't be completed due to an error
   </message>
+  <message name="IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED_IN_PROGRESS" desc="Message displayed in the notification when import (restore) of the Linux container fails due to another being in progress.">
+    Restore currently in progress for <ph name="CONTAINER_ID">$1<ex>(termina, penguin)</ex></ph>
+  </message>
+  <message name="IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED_ARCHITECTURE" desc="Message displayed in the notification when import (restore) of the Linux container fails due to architecture mismatch.">
+    Cannot import container architecture type <ph name="ARCHITECTURE_CONTAINER">$1<ex>arm64</ex></ph> with this device which is <ph name="ARCHITECTURE_DEVICE">$2<ex>x86_64</ex></ph>. You can try restoring this container into a different device, or you can access the files inside this container image by opening in Files app.
+  </message>
 
   <!-- Time limit notification -->
   <message name="IDS_SCREEN_TIME_NOTIFICATION_TITLE" desc="The title of the notification when screen usage limit reaches before locking the device.">

chrome/app/chromeos_strings_grdp/IDS_CROSTINI_EXPORT_NOTIFICATION_MESSAGE_FAILED_IN_PROGRESS.png.sha1

+3519b1b96658bf54a93cb2df6d8b5d711522c9f7
\ No newline at end of file

chrome/app/chromeos_strings_grdp/IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED_ARCHITECTURE.png.sha1

+0f025026240011c46a51b6d6752ae86df3d38930
\ No newline at end of file

chrome/app/chromeos_strings_grdp/IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED_IN_PROGRESS.png.sha1

+3519b1b96658bf54a93cb2df6d8b5d711522c9f7
\ No newline at end of file

chrome/browser/chromeos/crostini/crostini_export_import.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/crostini/crostini_manager_factory.h"
 #include "chrome/browser/chromeos/crostini/crostini_share_path.h"
@@ -319,6 +320,10 @@ void CrostiniExportImport::OnImportComplete(
       case crostini::CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED_VM_STARTED:
         enum_hist_result = ImportContainerResult::kFailedVmStarted;
         break;
+      case crostini::CrostiniResult::
+          CONTAINER_EXPORT_IMPORT_FAILED_ARCHITECTURE:
+        enum_hist_result = ImportContainerResult::kFailedArchitecture;
+        break;
       default:
         enum_hist_result = ImportContainerResult::kFailed;
     }
@@ -338,7 +343,9 @@ void CrostiniExportImport::OnImportContainerProgress(
     const std::string& container_name,
     ImportContainerProgressStatus status,
     int progress_percent,
-    uint64_t progress_speed) {
+    uint64_t progress_speed,
+    const std::string& architecture_device,
+    const std::string& architecture_container) {
   ContainerId container_id(vm_name, container_name);
   auto it = notifications_.find(container_id);
   DCHECK(it != notifications_.end())
@@ -357,6 +364,13 @@ void CrostiniExportImport::OnImportContainerProgress(
           CrostiniExportImportNotification::Status::RUNNING,
           50 + progress_percent / 2);
       break;
+    // Failure, set error message.
+    case ImportContainerProgressStatus::FAILURE_ARCHITECTURE:
+      it->second->set_message_failed(l10n_util::GetStringFUTF16(
+          IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED_ARCHITECTURE,
+          base::ASCIIToUTF16(architecture_container),
+          base::ASCIIToUTF16(architecture_device)));
+      break;
     default:
       LOG(WARNING) << "Unknown Export progress status " << int(status);
   }

chrome/browser/chromeos/crostini/crostini_export_import.h

@@ -10,6 +10,7 @@
 #include <string>
 
 #include "base/memory/weak_ptr.h"
+#include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/crostini/crostini_export_import_notification.h"
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
@@ -43,7 +44,8 @@ enum class ImportContainerResult {
   kFailed = 1,
   kFailedVmStopped = 2,
   kFailedVmStarted = 3,
-  kMaxValue = kFailedVmStarted,
+  kFailedArchitecture = 4,
+  kMaxValue = kFailedArchitecture,
 };
 
 // CrostiniExportImport is a keyed profile service to manage exporting and
@@ -109,11 +111,14 @@ class CrostiniExportImport : public KeyedService,
                                  uint64_t progress_speed) override;
 
   // crostini::ImportContainerProgressObserver implementation.
-  void OnImportContainerProgress(const std::string& vm_name,
-                                 const std::string& container_name,
-                                 crostini::ImportContainerProgressStatus status,
-                                 int progress_percent,
-                                 uint64_t progress_speed) override;
+  void OnImportContainerProgress(
+      const std::string& vm_name,
+      const std::string& container_name,
+      crostini::ImportContainerProgressStatus status,
+      int progress_percent,
+      uint64_t progress_speed,
+      const std::string& architecture_device,
+      const std::string& architecture_container) override;
 
   void ExportAfterSharing(const ContainerId& container_id,
                           const base::FilePath& filename,

chrome/browser/chromeos/crostini/crostini_export_import_notification.cc

@@ -6,6 +6,7 @@
 
 #include "ash/public/cpp/notification_utils.h"
 #include "ash/public/cpp/vector_icons/vector_icons.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/crostini/crostini_export_import.h"
 #include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/platform_util.h"
@@ -39,18 +40,28 @@ CrostiniExportImportNotification::CrostiniExportImportNotification(
   // Messages.
   switch (type) {
     case ExportImportType::EXPORT:
-      title_running_ = IDS_CROSTINI_EXPORT_NOTIFICATION_TITLE_RUNNING;
-      title_done_ = IDS_CROSTINI_EXPORT_NOTIFICATION_TITLE_DONE;
-      message_done_ = IDS_CROSTINI_EXPORT_NOTIFICATION_MESSAGE_DONE;
-      title_failed_ = IDS_CROSTINI_EXPORT_NOTIFICATION_TITLE_FAILED;
-      message_failed_ = IDS_CROSTINI_EXPORT_NOTIFICATION_MESSAGE_FAILED;
+      title_running_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_EXPORT_NOTIFICATION_TITLE_RUNNING);
+      title_done_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_EXPORT_NOTIFICATION_TITLE_DONE);
+      message_done_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_EXPORT_NOTIFICATION_MESSAGE_DONE);
+      title_failed_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_EXPORT_NOTIFICATION_TITLE_FAILED);
+      message_failed_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_EXPORT_NOTIFICATION_MESSAGE_FAILED);
       break;
     case ExportImportType::IMPORT:
-      title_running_ = IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_RUNNING;
-      title_done_ = IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_DONE;
-      message_done_ = IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_DONE;
-      title_failed_ = IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_FAILED;
-      message_failed_ = IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED;
+      title_running_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_RUNNING);
+      title_done_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_DONE);
+      message_done_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_DONE);
+      title_failed_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_FAILED);
+      message_failed_ = l10n_util::GetStringUTF16(
+          IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED);
       break;
     default:
       NOTREACHED();
@@ -88,7 +99,7 @@ void CrostiniExportImportNotification::UpdateStatus(Status status,
       }
       notification_->set_type(message_center::NOTIFICATION_TYPE_PROGRESS);
       notification_->set_progress(progress_percent);
-      notification_->set_title(l10n_util::GetStringUTF16(title_running_));
+      notification_->set_title(title_running_);
       notification_->set_message(
           GetTimeRemainingMessage(started_, progress_percent));
       notification_->set_never_timeout(true);
@@ -99,16 +110,16 @@ void CrostiniExportImportNotification::UpdateStatus(Status status,
         notification_->set_buttons({message_center::ButtonInfo(
             l10n_util::GetStringUTF16(IDS_DOWNLOAD_LINK_SHOW))});
       }
-      notification_->set_title(l10n_util::GetStringUTF16(title_done_));
-      notification_->set_message(l10n_util::GetStringUTF16(message_done_));
+      notification_->set_title(title_done_);
+      notification_->set_message(message_done_);
       notification_->set_never_timeout(false);
       break;
     case Status::FAILED:
       notification_->set_type(message_center::NOTIFICATION_TYPE_SIMPLE);
       notification_->set_accent_color(
           ash::kSystemNotificationColorCriticalWarning);
-      notification_->set_title(l10n_util::GetStringUTF16(title_failed_));
-      notification_->set_message(l10n_util::GetStringUTF16(message_failed_));
+      notification_->set_title(title_failed_);
+      notification_->set_message(message_failed_);
       notification_->set_never_timeout(false);
       break;
     default:

chrome/browser/chromeos/crostini/crostini_export_import_notification.h

@@ -39,6 +39,9 @@ class CrostiniExportImportNotification
   virtual ~CrostiniExportImportNotification();
 
   void UpdateStatus(Status status, int progress_percent);
+  void set_message_failed(const base::string16& message) {
+    message_failed_ = message;
+  }
 
   // Getters for testing.
   Status get_status() { return status_; }
@@ -60,11 +63,11 @@ class CrostiniExportImportNotification
   Status status_ = Status::RUNNING;
   // Time when the operation started.  Used for estimating time remaining.
   base::Time started_ = base::Time::Now();
-  int title_running_;
-  int title_done_;
-  int message_done_;
-  int title_failed_;
-  int message_failed_;
+  base::string16 title_running_;
+  base::string16 title_done_;
+  base::string16 message_done_;
+  base::string16 title_failed_;
+  base::string16 message_failed_;
   std::unique_ptr<message_center::Notification> notification_;
   bool closed_ = false;
   base::WeakPtrFactory<CrostiniExportImportNotification> weak_ptr_factory_;

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -2587,47 +2587,59 @@ void CrostiniManager::OnImportLxdContainerProgress(
   if (signal.owner_id() != owner_id_)
     return;
 
-  bool importing = false;
+  bool call_observers = false;
+  bool call_original_callback = false;
   ImportContainerProgressStatus status;
   CrostiniResult result;
   switch (signal.status()) {
     case vm_tools::cicerone::ImportLxdContainerProgressSignal::IMPORTING_UPLOAD:
-      importing = true;
+      call_observers = true;
       status = ImportContainerProgressStatus::UPLOAD;
       break;
     case vm_tools::cicerone::ImportLxdContainerProgressSignal::IMPORTING_UNPACK:
-      importing = true;
+      call_observers = true;
       status = ImportContainerProgressStatus::UNPACK;
       break;
     case vm_tools::cicerone::ImportLxdContainerProgressSignal::DONE:
+      call_original_callback = true;
       result = CrostiniResult::SUCCESS;
       break;
+    case vm_tools::cicerone::ImportLxdContainerProgressSignal::
+        FAILED_ARCHITECTURE:
+      call_observers = true;
+      status = ImportContainerProgressStatus::FAILURE_ARCHITECTURE;
+      call_original_callback = true;
+      result = CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED_ARCHITECTURE;
+      break;
     default:
+      call_original_callback = true;
       result = CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED;
       LOG(ERROR) << "Failed during import container: " << signal.status()
                  << ", " << signal.failure_reason();
   }
 
-  // If we are still importing, call progress observers.
-  if (importing) {
+  // Call progress observers.
+  if (call_observers) {
     for (auto& observer : import_container_progress_observers_) {
       observer.OnImportContainerProgress(
           signal.vm_name(), signal.container_name(), status,
-          signal.progress_percent(), signal.progress_speed());
+          signal.progress_percent(), signal.progress_speed(),
+          signal.architecture_device(), signal.architecture_container());
     }
-    return;
   }
 
   // Invoke original callback with either success or failure.
-  auto key = std::make_pair(signal.vm_name(), signal.container_name());
-  auto it = import_lxd_container_callbacks_.find(key);
-  if (it == import_lxd_container_callbacks_.end()) {
-    LOG(ERROR) << "No import callback for " << signal.vm_name() << ", "
-               << signal.container_name();
-    return;
+  if (call_original_callback) {
+    auto key = std::make_pair(signal.vm_name(), signal.container_name());
+    auto it = import_lxd_container_callbacks_.find(key);
+    if (it == import_lxd_container_callbacks_.end()) {
+      LOG(ERROR) << "No import callback for " << signal.vm_name() << ", "
+                 << signal.container_name();
+      return;
+    }
+    std::move(it->second).Run(result);
+    import_lxd_container_callbacks_.erase(it);
   }
-  std::move(it->second).Run(result);
-  import_lxd_container_callbacks_.erase(it);
 }
 
 }  // namespace crostini

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -70,6 +70,7 @@ enum class CrostiniResult {
   CONTAINER_EXPORT_IMPORT_FAILED,
   CONTAINER_EXPORT_IMPORT_FAILED_VM_STOPPED,
   CONTAINER_EXPORT_IMPORT_FAILED_VM_STARTED,
+  CONTAINER_EXPORT_IMPORT_FAILED_ARCHITECTURE,
   NOT_ALLOWED,
 };
 
@@ -100,6 +101,7 @@ enum class ExportContainerProgressStatus {
 enum class ImportContainerProgressStatus {
   UPLOAD,
   UNPACK,
+  FAILURE_ARCHITECTURE,
 };
 
 struct VmInfo {
@@ -181,12 +183,15 @@ class ImportContainerProgressObserver {
  public:
   // A successfully started container import will continually fire progress
   // events until the original callback from ImportLxdContainer is invoked with
-  // a status of SUCCESS or CONTAINER_IMPORT_FAILED.
-  virtual void OnImportContainerProgress(const std::string& vm_name,
-                                         const std::string& container_name,
-                                         ImportContainerProgressStatus status,
-                                         int progress_percent,
-                                         uint64_t progress_speed) = 0;
+  // a status of SUCCESS or CONTAINER_IMPORT_FAILED[_*].
+  virtual void OnImportContainerProgress(
+      const std::string& vm_name,
+      const std::string& container_name,
+      ImportContainerProgressStatus status,
+      int progress_percent,
+      uint64_t progress_speed,
+      const std::string& architecture_device,
+      const std::string& architecture_container) = 0;
 };
 
 class InstallerViewStatusObserver : public base::CheckedObserver {

chrome/browser/chromeos/crostini/crostini_manager_unittest.cc

@@ -1227,6 +1227,29 @@ TEST_F(CrostiniManagerTest, ImportContainerFailInProgress) {
   run_loop()->Run();
 }
 
+TEST_F(CrostiniManagerTest, ImportContainerFailArchitecture) {
+  crostini_manager()->ImportLxdContainer(
+      kVmName, kContainerName, base::FilePath("import_path"),
+      base::BindOnce(
+          &CrostiniManagerTest::CrostiniResultCallback, base::Unretained(this),
+          run_loop()->QuitClosure(),
+          CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED_ARCHITECTURE));
+
+  // Send signal with FAILED_ARCHITECTURE.
+  vm_tools::cicerone::ImportLxdContainerProgressSignal signal;
+  signal.set_owner_id(CryptohomeIdForProfile(profile()));
+  signal.set_vm_name(kVmName);
+  signal.set_container_name(kContainerName);
+  signal.set_status(
+      vm_tools::cicerone::
+          ImportLxdContainerProgressSignal_Status_FAILED_ARCHITECTURE);
+  signal.set_architecture_device("archdev");
+  signal.set_architecture_container("archcont");
+  fake_cicerone_client_->NotifyImportLxdContainerProgress(signal);
+
+  run_loop()->Run();
+}
+
 TEST_F(CrostiniManagerTest, ImportContainerFailFromSignal) {
   crostini_manager()->ImportLxdContainer(
       kVmName, kContainerName, base::FilePath("import_path"),

tools/metrics/histograms/enums.xml

@@ -10445,6 +10445,7 @@ Called by update_net_error_codes.py.-->
   <int value="1" label="Failed"/>
   <int value="2" label="Failed VM Stopped"/>
   <int value="3" label="Failed VM Started"/>
+  <int value="4" label="Failed Architecture mismatch"/>
 </enum>
 
 <enum name="CrostiniSetupResult">