Commit 4e8f4a61 authored by Julian Watson's avatar Julian Watson Committed by Commit Bot

crostini: always check export/import iterator validity

Bug: 995615, 991461
Change-Id: I7dd728be3e8889103df660b92d16893367150f46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1775883
Commit-Queue: Julian Watson <juwa@google.com>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Auto-Submit: Julian Watson <juwa@google.com>
Reviewed-by: default avatarJoel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691866}
parent 72e287ce
...@@ -221,9 +221,12 @@ void CrostiniExportImport::ExportAfterSharing( ...@@ -221,9 +221,12 @@ void CrostiniExportImport::ExportAfterSharing(
LOG(ERROR) << "Error sharing for export " << container_path.value() << ": " LOG(ERROR) << "Error sharing for export " << container_path.value() << ": "
<< failure_reason; << failure_reason;
auto it = notifications_.find(container_id); auto it = notifications_.find(container_id);
DCHECK(it != notifications_.end()) << ContainerIdToString(container_id) if (it != notifications_.end()) {
<< " has no notification to update"; RemoveNotification(it).SetStatusFailed();
RemoveNotification(it).SetStatusFailed(); } else {
NOTREACHED() << ContainerIdToString(container_id)
<< " has no notification to update";
}
return; return;
} }
CrostiniManager::GetForProfile(profile_)->ExportLxdContainer( CrostiniManager::GetForProfile(profile_)->ExportLxdContainer(
...@@ -241,8 +244,11 @@ void CrostiniExportImport::OnExportComplete( ...@@ -241,8 +244,11 @@ void CrostiniExportImport::OnExportComplete(
uint64_t container_size, uint64_t container_size,
uint64_t compressed_size) { uint64_t compressed_size) {
auto it = notifications_.find(container_id); auto it = notifications_.find(container_id);
DCHECK(it != notifications_.end()) if (it == notifications_.end()) {
<< ContainerIdToString(container_id) << " has no notification to update"; NOTREACHED() << ContainerIdToString(container_id)
<< " has no notification to update";
return;
}
ExportContainerResult enum_hist_result = ExportContainerResult::kSuccess; ExportContainerResult enum_hist_result = ExportContainerResult::kSuccess;
if (result == CrostiniResult::SUCCESS) { if (result == CrostiniResult::SUCCESS) {
...@@ -334,8 +340,11 @@ void CrostiniExportImport::OnExportContainerProgress( ...@@ -334,8 +340,11 @@ void CrostiniExportImport::OnExportContainerProgress(
uint64_t progress_speed) { uint64_t progress_speed) {
ContainerId container_id(vm_name, container_name); ContainerId container_id(vm_name, container_name);
auto it = notifications_.find(container_id); auto it = notifications_.find(container_id);
DCHECK(it != notifications_.end()) if (it == notifications_.end()) {
<< ContainerIdToString(container_id) << " has no notification to update"; NOTREACHED() << ContainerIdToString(container_id)
<< " has no notification to update";
return;
}
switch (status) { switch (status) {
// Rescale PACK:1-100 => 0-50. // Rescale PACK:1-100 => 0-50.
...@@ -357,8 +366,11 @@ void CrostiniExportImport::OnExportContainerProgress( ...@@ -357,8 +366,11 @@ void CrostiniExportImport::OnExportContainerProgress(
const StreamingExportStatus& status) { const StreamingExportStatus& status) {
ContainerId container_id(vm_name, container_name); ContainerId container_id(vm_name, container_name);
auto it = notifications_.find(container_id); auto it = notifications_.find(container_id);
DCHECK(it != notifications_.end()) if (it == notifications_.end()) {
<< ContainerIdToString(container_id) << " has no notification to update"; NOTREACHED() << ContainerIdToString(container_id)
<< " has no notification to update";
return;
}
const auto files_percent = 100.0 * status.exported_files / status.total_files; const auto files_percent = 100.0 * status.exported_files / status.total_files;
const auto bytes_percent = 100.0 * status.exported_bytes / status.total_bytes; const auto bytes_percent = 100.0 * status.exported_bytes / status.total_bytes;
...@@ -380,9 +392,12 @@ void CrostiniExportImport::ImportAfterSharing( ...@@ -380,9 +392,12 @@ void CrostiniExportImport::ImportAfterSharing(
LOG(ERROR) << "Error sharing for import " << container_path.value() << ": " LOG(ERROR) << "Error sharing for import " << container_path.value() << ": "
<< failure_reason; << failure_reason;
auto it = notifications_.find(container_id); auto it = notifications_.find(container_id);
DCHECK(it != notifications_.end()) << ContainerIdToString(container_id) if (it != notifications_.end()) {
<< " has no notification to update"; RemoveNotification(it).SetStatusFailed();
RemoveNotification(it).SetStatusFailed(); } else {
NOTREACHED() << ContainerIdToString(container_id)
<< " has no notification to update";
}
return; return;
} }
CrostiniManager::GetForProfile(profile_)->ImportLxdContainer( CrostiniManager::GetForProfile(profile_)->ImportLxdContainer(
...@@ -403,31 +418,38 @@ void CrostiniExportImport::OnImportComplete( ...@@ -403,31 +418,38 @@ void CrostiniExportImport::OnImportComplete(
if (result == CrostiniResult::SUCCESS) { if (result == CrostiniResult::SUCCESS) {
UMA_HISTOGRAM_LONG_TIMES("Crostini.RestoreTimeSuccess", UMA_HISTOGRAM_LONG_TIMES("Crostini.RestoreTimeSuccess",
base::Time::Now() - start); base::Time::Now() - start);
DCHECK(it != notifications_.end()) << ContainerIdToString(container_id) if (it != notifications_.end()) {
<< " has no notification to update"; switch (it->second->status()) {
switch (it->second->status()) { case CrostiniExportImportNotification::Status::RUNNING:
case CrostiniExportImportNotification::Status::RUNNING: // If a user requests to cancel, but the import completes before the
// If a user requests to cancel, but the import completes before the // cancel can happen, then the container will have been imported over
// cancel can happen, then the container will have been imported over // and the cancel will have failed. However the period of time in
// and the cancel will have failed. However the period of time in which // which this can happen is very small (<5s), so it feels quite
// this can happen is very small (<5s), so it feels quite natural to // natural to pretend the cancel did not happen, and instead display
// pretend the cancel did not happen, and instead display success. // success.
case CrostiniExportImportNotification::Status::CANCELLING: case CrostiniExportImportNotification::Status::CANCELLING:
RemoveNotification(it).SetStatusDone(); RemoveNotification(it).SetStatusDone();
break; break;
default: default:
NOTREACHED(); NOTREACHED();
}
} else {
NOTREACHED() << ContainerIdToString(container_id)
<< " has no notification to update";
} }
} else if (result == } else if (result ==
crostini::CrostiniResult::CONTAINER_EXPORT_IMPORT_CANCELLED) { crostini::CrostiniResult::CONTAINER_EXPORT_IMPORT_CANCELLED) {
DCHECK(it != notifications_.end()) << ContainerIdToString(container_id) if (it != notifications_.end()) {
<< " has no notification to update"; switch (it->second->status()) {
switch (it->second->status()) { case CrostiniExportImportNotification::Status::CANCELLING:
case CrostiniExportImportNotification::Status::CANCELLING: RemoveNotification(it).SetStatusCancelled();
RemoveNotification(it).SetStatusCancelled(); break;
break; default:
default: NOTREACHED();
NOTREACHED(); }
} else {
NOTREACHED() << ContainerIdToString(container_id)
<< " has no notification to update";
} }
} else { } else {
LOG(ERROR) << "Error importing " << int(result); LOG(ERROR) << "Error importing " << int(result);
...@@ -454,11 +476,14 @@ void CrostiniExportImport::OnImportComplete( ...@@ -454,11 +476,14 @@ void CrostiniExportImport::OnImportComplete(
if (result == CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED || if (result == CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED ||
result == CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED_VM_STOPPED || result == CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED_VM_STOPPED ||
result == CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED_VM_STARTED) { result == CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED_VM_STARTED) {
DCHECK(it != notifications_.end()) << ContainerIdToString(container_id) if (it != notifications_.end()) {
<< " has no notification to update"; DCHECK(it->second->status() ==
DCHECK(it->second->status() == CrostiniExportImportNotification::Status::RUNNING);
CrostiniExportImportNotification::Status::RUNNING); RemoveNotification(it).SetStatusFailed();
RemoveNotification(it).SetStatusFailed(); } else {
NOTREACHED() << ContainerIdToString(container_id)
<< " has no notification to update";
}
} else { } else {
DCHECK(it == notifications_.end()) << ContainerIdToString(container_id) DCHECK(it == notifications_.end()) << ContainerIdToString(container_id)
<< " has unexpected notification"; << " has unexpected notification";
...@@ -485,8 +510,11 @@ void CrostiniExportImport::OnImportContainerProgress( ...@@ -485,8 +510,11 @@ void CrostiniExportImport::OnImportContainerProgress(
uint64_t minimum_required_space) { uint64_t minimum_required_space) {
ContainerId container_id(vm_name, container_name); ContainerId container_id(vm_name, container_name);
auto it = notifications_.find(container_id); auto it = notifications_.find(container_id);
DCHECK(it != notifications_.end()) if (it == notifications_.end()) {
<< ContainerIdToString(container_id) << " has no notification to update"; NOTREACHED() << ContainerIdToString(container_id)
<< " has no notification to update";
return;
}
switch (status) { switch (status) {
// Rescale UPLOAD:1-100 => 0-50. // Rescale UPLOAD:1-100 => 0-50.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment