crostini: refactor export import notification

This change refactors the export import notification, such that all
string handling is internal, and such that there is a more defined API
for setting the many status variants.

This change facilitates two incoming changes: one that adds the status
CANCELLING and CANCELLED to the notification, and another that adds another
variant on the failed status to do with concurrent operations.

BUG=chromium:978318

Change-Id: Icc101378ce2f6650f1dc646d06af868684bd56b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1702346
Commit-Queue: Julian Watson <juwa@google.com>
Auto-Submit: Julian Watson <juwa@google.com>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#679821}

chrome/browser/chromeos/crostini/crostini_export_import.h

@@ -83,7 +83,8 @@ class CrostiniExportImport : public KeyedService,
                        CrostiniManager::CrostiniResultCallback callback);
 
   // Called by the notification when it is closed so it can be destroyed.
-  void NotificationCompleted(CrostiniExportImportNotification* notification);
+  void NotificationCompleted(
+      const CrostiniExportImportNotification& notification);
 
   CrostiniExportImportNotification* GetNotificationForTesting(
       ContainerId container_id);
@@ -105,7 +106,7 @@ class CrostiniExportImport : public KeyedService,
                     void* params) override;
 
   void Start(ExportImportType type,
-             const ContainerId& container_id,
+             ContainerId container_id,
              base::FilePath path,
              CrostiniManager::CrostiniResultCallback callback);
 

chrome/browser/chromeos/crostini/crostini_export_import_notification.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/text/bytes_formatting.h"
 #include "ui/message_center/public/cpp/message_center_constants.h"
 #include "ui/message_center/public/cpp/notification.h"
 #include "ui/message_center/public/cpp/notification_delegate.h"
@@ -37,35 +38,7 @@ CrostiniExportImportNotification::CrostiniExportImportNotification(
       type_(type),
       path_(path),
       weak_ptr_factory_(this) {
-  // Messages.
-  switch (type) {
-    case ExportImportType::EXPORT:
-      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_ = 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();
-  }
+  DCHECK(type == ExportImportType::EXPORT || type == ExportImportType::IMPORT);
 
   message_center::RichNotificationData rich_notification_data;
   rich_notification_data.vector_small_image = &kNotificationLinuxIcon;
@@ -84,47 +57,99 @@ CrostiniExportImportNotification::CrostiniExportImportNotification(
       base::MakeRefCounted<message_center::ThunkNotificationDelegate>(
           weak_ptr_factory_.GetWeakPtr()));
 
-  UpdateStatus(Status::RUNNING, 0);
+  SetStatusRunning(0);
 }
 
 CrostiniExportImportNotification::~CrostiniExportImportNotification() = default;
 
-void CrostiniExportImportNotification::UpdateStatus(Status status,
-                                                    int progress_percent) {
-  status_ = status;
-  switch (status) {
-    case Status::RUNNING:
-      if (closed_) {
-        return;
-      }
-      notification_->set_type(message_center::NOTIFICATION_TYPE_PROGRESS);
-      notification_->set_progress(progress_percent);
-      notification_->set_title(title_running_);
-      notification_->set_message(
-          GetTimeRemainingMessage(started_, progress_percent));
-      notification_->set_never_timeout(true);
-      break;
-    case Status::DONE:
-      notification_->set_type(message_center::NOTIFICATION_TYPE_SIMPLE);
-      if (type_ == ExportImportType::EXPORT) {
-        notification_->set_buttons({message_center::ButtonInfo(
-            l10n_util::GetStringUTF16(IDS_DOWNLOAD_LINK_SHOW))});
-      }
-      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(title_failed_);
-      notification_->set_message(message_failed_);
-      notification_->set_never_timeout(false);
-      break;
-    default:
-      NOTREACHED();
+void CrostiniExportImportNotification::SetStatusRunning(int progress_percent) {
+  status_ = Status::RUNNING;
+
+  if (closed_) {
+    return;
   }
+
+  notification_->set_type(message_center::NOTIFICATION_TYPE_PROGRESS);
+  notification_->set_accent_color(ash::kSystemNotificationColorNormal);
+  notification_->set_title(l10n_util::GetStringUTF16(
+      type_ == ExportImportType::EXPORT
+          ? IDS_CROSTINI_EXPORT_NOTIFICATION_TITLE_RUNNING
+          : IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_RUNNING));
+  notification_->set_message(
+      GetTimeRemainingMessage(started_, progress_percent));
+  notification_->set_buttons({});
+  notification_->set_never_timeout(true);
+  notification_->set_progress(progress_percent);
+
+  NotificationDisplayService::GetForProfile(profile_)->Display(
+      NotificationHandler::Type::TRANSIENT, *notification_,
+      /*metadata=*/nullptr);
+}
+
+void CrostiniExportImportNotification::SetStatusDone() {
+  status_ = Status::DONE;
+
+  notification_->set_type(message_center::NOTIFICATION_TYPE_SIMPLE);
+  notification_->set_accent_color(ash::kSystemNotificationColorNormal);
+  notification_->set_title(l10n_util::GetStringUTF16(
+      type_ == ExportImportType::EXPORT
+          ? IDS_CROSTINI_EXPORT_NOTIFICATION_TITLE_DONE
+          : IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_DONE));
+  notification_->set_message(l10n_util::GetStringUTF16(
+      type_ == ExportImportType::EXPORT
+          ? IDS_CROSTINI_EXPORT_NOTIFICATION_MESSAGE_DONE
+          : IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_DONE));
+  notification_->set_buttons(
+      type_ == ExportImportType::EXPORT
+          ? std::vector<message_center::ButtonInfo>{message_center::ButtonInfo(
+                l10n_util::GetStringUTF16(IDS_DOWNLOAD_LINK_SHOW))}
+          : std::vector<message_center::ButtonInfo>{});
+  notification_->set_never_timeout(false);
+
+  NotificationDisplayService::GetForProfile(profile_)->Display(
+      NotificationHandler::Type::TRANSIENT, *notification_,
+      /*metadata=*/nullptr);
+}
+
+void CrostiniExportImportNotification::SetStatusFailed() {
+  SetStatusFailed(l10n_util::GetStringUTF16(
+      type_ == ExportImportType::EXPORT
+          ? IDS_CROSTINI_EXPORT_NOTIFICATION_MESSAGE_FAILED
+          : IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED));
+}
+
+void CrostiniExportImportNotification::SetStatusFailedArchitectureMismatch(
+    const std::string& architecture_container,
+    const std::string& architecture_device) {
+  DCHECK(type_ == ExportImportType::IMPORT);
+  SetStatusFailed(l10n_util::GetStringFUTF16(
+      IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED_ARCHITECTURE,
+      base::ASCIIToUTF16(architecture_container),
+      base::ASCIIToUTF16(architecture_device)));
+}
+
+void CrostiniExportImportNotification::SetStatusFailedInsufficientSpace(
+    uint64_t additional_required_space) {
+  DCHECK(type_ == ExportImportType::IMPORT);
+  SetStatusFailed(l10n_util::GetStringFUTF16(
+      IDS_CROSTINI_IMPORT_NOTIFICATION_MESSAGE_FAILED_SPACE,
+      ui::FormatBytes(additional_required_space)));
+}
+
+void CrostiniExportImportNotification::SetStatusFailed(
+    const base::string16& message) {
+  status_ = Status::FAILED;
+
+  notification_->set_type(message_center::NOTIFICATION_TYPE_SIMPLE);
+  notification_->set_accent_color(ash::kSystemNotificationColorCriticalWarning);
+  notification_->set_title(l10n_util::GetStringUTF16(
+      type_ == ExportImportType::EXPORT
+          ? IDS_CROSTINI_EXPORT_NOTIFICATION_TITLE_FAILED
+          : IDS_CROSTINI_IMPORT_NOTIFICATION_TITLE_FAILED));
+  notification_->set_message(message);
+  notification_->set_buttons({});
+  notification_->set_never_timeout(false);
+
   NotificationDisplayService::GetForProfile(profile_)->Display(
       NotificationHandler::Type::TRANSIENT, *notification_,
       /*metadata=*/nullptr);
@@ -133,7 +158,7 @@ void CrostiniExportImportNotification::UpdateStatus(Status status,
 void CrostiniExportImportNotification::Close(bool by_user) {
   closed_ = true;
   if (status_ != Status::RUNNING) {
-    service_->NotificationCompleted(this);
+    service_->NotificationCompleted(*this);
   }
 }
 

chrome/browser/chromeos/crostini/crostini_export_import_notification.h

@@ -38,13 +38,16 @@ class CrostiniExportImportNotification
                                    const base::FilePath& path);
   virtual ~CrostiniExportImportNotification();
 
-  void UpdateStatus(Status status, int progress_percent);
-  void set_message_failed(const base::string16& message) {
-    message_failed_ = message;
-  }
-
+  void SetStatusRunning(int progress_percent);
+  void SetStatusDone();
+  void SetStatusFailed();
+  void SetStatusFailedArchitectureMismatch(
+      const std::string& architecture_container,
+      const std::string& architecture_device);
+  void SetStatusFailedInsufficientSpace(uint64_t additional_required_space);
+
+  Status get_status() const { return status_; }
   // Getters for testing.
-  Status get_status() { return status_; }
   message_center::Notification* get_notification() {
     return notification_.get();
   }
@@ -55,6 +58,8 @@ class CrostiniExportImportNotification
              const base::Optional<base::string16>& reply) override;
 
  private:
+  void SetStatusFailed(const base::string16& message);
+
   Profile* profile_;
   // These notifications are owned by the export service.
   CrostiniExportImport* service_;
@@ -63,11 +68,6 @@ class CrostiniExportImportNotification
   Status status_ = Status::RUNNING;
   // Time when the operation started.  Used for estimating time remaining.
   base::TimeTicks started_ = base::TimeTicks::Now();
-  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_;