Commit bff26532 authored by David Munro's avatar David Munro Committed by Commit Bot

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.

Change-Id: I7f8fb7bfe652b596da0763693699d74c6c61d39d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2459133
Commit-Queue: David Munro <davidmunro@google.com>
Auto-Submit: David Munro <davidmunro@google.com>
Reviewed-by: default avatarFergus Dall <sidereal@google.com>
Cr-Commit-Position: refs/heads/master@{#815466}
parent 77b634cd
...@@ -151,6 +151,8 @@ void OnListVmDisks( ...@@ -151,6 +151,8 @@ void OnListVmDisks(
VLOG(1) << "image_type: " << image->image_type(); VLOG(1) << "image_type: " << image->image_type();
VLOG(1) << "size: " << image->size(); VLOG(1) << "size: " << image->size();
VLOG(1) << "user_chosen_size: " << image->user_chosen_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() != if (image->image_type() !=
vm_tools::concierge::DiskImageType::DISK_IMAGE_RAW) { vm_tools::concierge::DiskImageType::DISK_IMAGE_RAW) {
...@@ -207,17 +209,15 @@ std::vector<crostini::mojom::DiskSliderTickPtr> GetTicks( ...@@ -207,17 +209,15 @@ std::vector<crostini::mojom::DiskSliderTickPtr> GetTicks(
return {}; return {};
} }
std::vector<int64_t> values = GetTicksForDiskSize(min, max); 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. // 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); auto it = std::lower_bound(begin(values), end(values), current);
if (it != end(values)) { *out_default_index = std::distance(begin(values), it);
if (*it != current) { if (it == end(values) || *it != current) {
values.insert(it, current); values.insert(it, current);
}
*out_default_index = std::distance(begin(values), it);
} else {
DCHECK(values.empty());
return {};
} }
std::vector<crostini::mojom::DiskSliderTickPtr> ticks; std::vector<crostini::mojom::DiskSliderTickPtr> ticks;
......
...@@ -190,6 +190,24 @@ TEST_F(CrostiniDiskTest, DefaultIsCurrentValue) { ...@@ -190,6 +190,24 @@ TEST_F(CrostiniDiskTest, DefaultIsCurrentValue) {
EXPECT_GT(disk_info->ticks.at(disk_info->default_index + 1)->value, 3 * kGiB); 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) { TEST_F(CrostiniDiskTest, AmountOfFreeDiskSpaceFailureIsHandled) {
std::unique_ptr<CrostiniDiskInfo> disk_info; std::unique_ptr<CrostiniDiskInfo> disk_info;
auto store_info = auto store_info =
...@@ -346,6 +364,7 @@ TEST_F(CrostiniDiskTest, GetTicksForDiskSizeSmallRangeNonZeroStart) { ...@@ -346,6 +364,7 @@ TEST_F(CrostiniDiskTest, GetTicksForDiskSizeSmallRangeNonZeroStart) {
EXPECT_FLOAT_EQ(ticks[n], expected[n]); EXPECT_FLOAT_EQ(ticks[n], expected[n]);
} }
} }
TEST_F(CrostiniDiskTest, GetTicksForDiskSizeLargeRange) { TEST_F(CrostiniDiskTest, GetTicksForDiskSizeLargeRange) {
// 5 ticks for 7 GiB, largest interval is 1GiB so we should end up with 8 // 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. // 0.1 GiB ticks. For bonus coverage, start at non-zero non-round.
......
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