crostini: Fix a disk resizing bug
Now that we round sizes to human-friendly numbers (e.g. nearest 0.1GiB),
and we round down the top of the range of allowed sizes, it's possible
to end up with a disk that's allocated a legal amount, but which is
beyond the slider range. Now we ensure that we always include whatever
size disk the user currently has in the range.
Bug: chromium:1126705
Test: Run new unit test that hits this case.
(cherry picked from commit bff26532a551629f1213d065c41919244bba21a1)
Change-Id: I7f8fb7bfe652b596da0763693699d74c6c61d39d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2459133
Commit-Queue: David Munro <[email protected]>
Auto-Submit: David Munro <[email protected]>
Reviewed-by: Fergus Dall <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#815466}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2469338
Reviewed-by: David Munro <[email protected]>
Commit-Queue: Fergus Dall <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#363}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/chrome/browser/chromeos/crostini/crostini_disk.cc b/chrome/browser/chromeos/crostini/crostini_disk.cc
index 8ea4a8c6..d792406 100644
--- a/chrome/browser/chromeos/crostini/crostini_disk.cc
+++ b/chrome/browser/chromeos/crostini/crostini_disk.cc
@@ -151,6 +151,8 @@
VLOG(1) << "image_type: " << image->image_type();
VLOG(1) << "size: " << image->size();
VLOG(1) << "user_chosen_size: " << image->user_chosen_size();
+ VLOG(1) << "free_space: " << free_space;
+ VLOG(1) << "min_size: " << image->min_size();
if (image->image_type() !=
vm_tools::concierge::DiskImageType::DISK_IMAGE_RAW) {
@@ -207,17 +209,15 @@
return {};
}
std::vector<int64_t> values = GetTicksForDiskSize(min, max);
+ DCHECK(!values.empty());
// If the current size isn't on one of the ticks insert an extra tick for it.
+ // It's possible for the current size to be greater than the maximum tick,
+ // in which case we go up to whatever that size is.
auto it = std::lower_bound(begin(values), end(values), current);
- if (it != end(values)) {
- if (*it != current) {
- values.insert(it, current);
- }
- *out_default_index = std::distance(begin(values), it);
- } else {
- DCHECK(values.empty());
- return {};
+ *out_default_index = std::distance(begin(values), it);
+ if (it == end(values) || *it != current) {
+ values.insert(it, current);
}
std::vector<crostini::mojom::DiskSliderTickPtr> ticks;
diff --git a/chrome/browser/chromeos/crostini/crostini_disk_unittest.cc b/chrome/browser/chromeos/crostini/crostini_disk_unittest.cc
index f2dd455..7df05ae 100644
--- a/chrome/browser/chromeos/crostini/crostini_disk_unittest.cc
+++ b/chrome/browser/chromeos/crostini/crostini_disk_unittest.cc
@@ -190,6 +190,24 @@
EXPECT_GT(disk_info->ticks.at(disk_info->default_index + 1)->value, 3 * kGiB);
}
+// Numbers taken from crbug/1126705.
+TEST_F(CrostiniDiskTest, AllocatedAboveMax) {
+ vm_tools::concierge::ListVmDisksResponse response;
+ auto* image = response.add_images();
+ response.set_success(true);
+ image->set_name("vm_name");
+ image->set_image_type(vm_tools::concierge::DiskImageType::DISK_IMAGE_RAW);
+ image->set_min_size(3260022784);
+ image->set_size(459561652224);
+ auto disk_info = OnListVmDisksWithResult("vm_name", 1120739328, response);
+ ASSERT_TRUE(disk_info);
+
+ ASSERT_TRUE(disk_info->ticks.size() > 3);
+ EXPECT_EQ(disk_info->default_index, disk_info->ticks.size() - 1);
+ EXPECT_EQ(disk_info->ticks.at(disk_info->default_index)->value,
+ image->size());
+}
+
TEST_F(CrostiniDiskTest, AmountOfFreeDiskSpaceFailureIsHandled) {
std::unique_ptr<CrostiniDiskInfo> disk_info;
auto store_info =
@@ -346,6 +364,7 @@
EXPECT_FLOAT_EQ(ticks[n], expected[n]);
}
}
+
TEST_F(CrostiniDiskTest, GetTicksForDiskSizeLargeRange) {
// 5 ticks for 7 GiB, largest interval is 1GiB so we should end up with 8
// 0.1 GiB ticks. For bonus coverage, start at non-zero non-round.
