Add ability to join two ScopedReservation's.
Bug: b:233089187
Change-Id: I7989de7982d31b45be1806bf14c22737a5ef4c82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3703493
Reviewed-by: Hong Xu <[email protected]>
Commit-Queue: Leonid Baraz <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1014255}
diff --git a/components/reporting/resources/resource_interface.cc b/components/reporting/resources/resource_interface.cc
index 727eb00..b2cd998 100644
--- a/components/reporting/resources/resource_interface.cc
+++ b/components/reporting/resources/resource_interface.cc
@@ -8,6 +8,7 @@
#include <cstdint>
+#include "base/logging.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_refptr.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
@@ -16,7 +17,7 @@
ScopedReservation::ScopedReservation(
uint64_t size,
- scoped_refptr<ResourceInterface> resource_interface)
+ scoped_refptr<ResourceInterface> resource_interface) noexcept
: resource_interface_(resource_interface) {
if (!resource_interface->Reserve(size)) {
return;
@@ -24,7 +25,7 @@
size_ = size;
}
-ScopedReservation::ScopedReservation(ScopedReservation&& other)
+ScopedReservation::ScopedReservation(ScopedReservation&& other) noexcept
: resource_interface_(other.resource_interface_),
size_(std::exchange(other.size_, absl::nullopt)) {}
@@ -36,14 +37,28 @@
if (!reserved()) {
return false;
}
- if (new_size <= 0 || size_.value() < new_size) {
+ if (new_size < 0 || size_.value() < new_size) {
return false;
}
resource_interface_->Discard(size_.value() - new_size);
- size_ = new_size;
+ if (new_size > 0) {
+ size_ = new_size;
+ } else {
+ size_ = absl::nullopt;
+ }
return true;
}
+void ScopedReservation::HandOver(ScopedReservation& other) {
+ DCHECK_EQ(resource_interface_.get(), other.resource_interface_.get())
+ << "Reservations are not related";
+ if (!other.reserved()) {
+ return; // Nothing changes.
+ }
+ const uint64_t old_size = (reserved() ? size_.value() : 0uL);
+ size_ = old_size + std::exchange(other.size_, absl::nullopt).value();
+}
+
ScopedReservation::~ScopedReservation() {
if (reserved()) {
resource_interface_->Discard(size_.value());
diff --git a/components/reporting/resources/resource_interface.h b/components/reporting/resources/resource_interface.h
index 235f42f..56fd3bce 100644
--- a/components/reporting/resources/resource_interface.h
+++ b/components/reporting/resources/resource_interface.h
@@ -60,16 +60,26 @@
// Can be handed over to another owner.
class ScopedReservation {
public:
- ScopedReservation(uint64_t size,
- scoped_refptr<ResourceInterface> resource_interface);
- ScopedReservation(ScopedReservation&& other);
+ ScopedReservation(
+ uint64_t size,
+ scoped_refptr<ResourceInterface> resource_interface) noexcept;
+ ScopedReservation(ScopedReservation&& other) noexcept;
ScopedReservation(const ScopedReservation& other) = delete;
+ ScopedReservation& operator=(ScopedReservation&& other) = delete;
ScopedReservation& operator=(const ScopedReservation& other) = delete;
~ScopedReservation();
bool reserved() const;
+
+ // Reduces reservation to |new_size|.
bool Reduce(uint64_t new_size);
+ // Adds |other| to |this| without assigning or releasing any reservation.
+ // Used for seamless transition from one reservation to another (more generic
+ // than std::move). Resets |other| to non-reserved state upon return from this
+ // method.
+ void HandOver(ScopedReservation& other);
+
private:
const scoped_refptr<ResourceInterface> resource_interface_;
absl::optional<uint64_t> size_;
diff --git a/components/reporting/resources/resource_interface_unittest.cc b/components/reporting/resources/resource_interface_unittest.cc
index 63bcbae..304800af 100644
--- a/components/reporting/resources/resource_interface_unittest.cc
+++ b/components/reporting/resources/resource_interface_unittest.cc
@@ -25,6 +25,11 @@
class ResourceInterfaceTest
: public ::testing::TestWithParam<scoped_refptr<ResourceInterface>> {
protected:
+ void SetUp() override {
+ // Make sure parameters define reasonably large total resource size.
+ ASSERT_GE(resource_interface()->GetTotal(), 1u * 1024LLu * 1024LLu);
+ }
+
scoped_refptr<ResourceInterface> resource_interface() const {
return GetParam();
}
@@ -146,7 +151,65 @@
for (; size >= 2; size /= 2) {
EXPECT_TRUE(scoped_reservation.Reduce(size / 2));
}
- EXPECT_FALSE(scoped_reservation.Reduce(size / 2));
+ EXPECT_TRUE(scoped_reservation.Reduce(size / 2));
+ EXPECT_FALSE(scoped_reservation.reserved());
+}
+
+TEST_P(ResourceInterfaceTest, ScopedReservationBasicHandOver) {
+ uint64_t size = resource_interface()->GetTotal() / 2;
+ ScopedReservation scoped_reservation(size, resource_interface());
+ ASSERT_TRUE(scoped_reservation.reserved());
+ {
+ ScopedReservation another_reservation(size - 1, resource_interface());
+ ASSERT_TRUE(another_reservation.reserved());
+ EXPECT_THAT(resource_interface()->GetUsed(),
+ Eq(resource_interface()->GetTotal() - 1));
+ EXPECT_TRUE(scoped_reservation.reserved());
+ EXPECT_TRUE(another_reservation.reserved());
+ scoped_reservation.HandOver(another_reservation);
+ EXPECT_THAT(resource_interface()->GetUsed(),
+ Eq(resource_interface()->GetTotal() - 1));
+ }
+ // Destruction of |anoter_reservation| does not change the amount used.
+ EXPECT_THAT(resource_interface()->GetUsed(),
+ Eq(resource_interface()->GetTotal() - 1));
+}
+
+TEST_P(ResourceInterfaceTest, ScopedReservationRepeatingHandOvers) {
+ uint64_t size = resource_interface()->GetTotal() / 2;
+ ScopedReservation scoped_reservation(size, resource_interface());
+ EXPECT_TRUE(scoped_reservation.reserved());
+
+ for (; size >= 2; size /= 2) {
+ ScopedReservation another_reservation(size / 2, resource_interface());
+ scoped_reservation.HandOver(another_reservation);
+ }
+ EXPECT_THAT(resource_interface()->GetUsed(),
+ Eq(resource_interface()->GetTotal() - 1));
+}
+
+TEST_P(ResourceInterfaceTest, ScopedReservationEmptyHandOver) {
+ uint64_t size = resource_interface()->GetTotal() / 2;
+ ScopedReservation scoped_reservation(size, resource_interface());
+
+ ASSERT_TRUE(scoped_reservation.reserved());
+ {
+ ScopedReservation another_reservation(size - 1, resource_interface());
+ ASSERT_TRUE(another_reservation.reserved());
+
+ EXPECT_THAT(resource_interface()->GetUsed(),
+ Eq(resource_interface()->GetTotal() - 1));
+ EXPECT_TRUE(scoped_reservation.reserved());
+ EXPECT_TRUE(another_reservation.reserved());
+
+ another_reservation.Reduce(0);
+ ASSERT_FALSE(another_reservation.reserved());
+
+ scoped_reservation.HandOver(another_reservation);
+ EXPECT_THAT(resource_interface()->GetUsed(), Eq(size));
+ }
+ // Destruction of |anoter_reservation| does not change the amount used.
+ EXPECT_THAT(resource_interface()->GetUsed(), Eq(size));
}
TEST_P(ResourceInterfaceTest, ReservationOverMaxTest) {
