Skip to content

Commit

Permalink
[M110] breadcrumbs: fix previous session event overrun
Browse files Browse the repository at this point in the history
BreadcrumbManager's event log can grow unbounded if the events added by
SetPreviousSessionEvents make it become oversized. This change adds
logic to SetPreviousSessionEvents to ensure it does not make the event
log grow past its max size.

This change also adds tests to prevent similar issues from happening in
the future.

(cherry picked from commit ee93e9c)

Bug: 1405018
Change-Id: I31b53823dbae8c89eabf6fe62ce9b7432c03a982
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4158873
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Reviewed-by: David Bienvenu <davidbienvenu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1091960}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4204728
Cr-Commit-Position: refs/branch-heads/5481@{#808}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
jessemckenna authored and Chromium LUCI CQ committed Jan 30, 2023
1 parent 80fa229 commit c260741
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 1 deletion.
10 changes: 9 additions & 1 deletion components/breadcrumbs/core/breadcrumb_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,15 @@ void BreadcrumbManager::AddEvent(const std::string& event) {

void BreadcrumbManager::SetPreviousSessionEvents(
const std::vector<std::string>& events) {
breadcrumbs_.insert(breadcrumbs_.begin(), events.begin(), events.end());
// Insert `events` into the event log, skipping the oldest events (which are
// at the front) if needed to keep the event log below `kMaxBreadcrumbs`.
const size_t breadcrumbs_capacity = kMaxBreadcrumbs - breadcrumbs_.size();
const size_t breadcrumbs_to_insert =
std::min(events.size(), breadcrumbs_capacity);

breadcrumbs_.insert(breadcrumbs_.begin(),
events.end() - breadcrumbs_to_insert, events.end());

for (auto& observer : observers_) {
observer.PreviousSessionEventsAdded();
}
Expand Down
59 changes: 59 additions & 0 deletions components/breadcrumbs/core/breadcrumb_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
#include "components/breadcrumbs/core/breadcrumb_manager.h"

#include <string>
#include <vector>

#include "base/strings/string_number_conversions.h"
#include "base/test/task_environment.h"
#include "base/time/time.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand All @@ -20,6 +22,10 @@ void AddEvent(const std::string& event) {
BreadcrumbManager::GetInstance().AddEvent(event);
}

void SetPreviousSessionEvents(const std::vector<std::string>& events) {
BreadcrumbManager::GetInstance().SetPreviousSessionEvents(events);
}

} // namespace

// Test fixture for testing BreadcrumbManager class.
Expand Down Expand Up @@ -77,4 +83,57 @@ TEST_F(BreadcrumbManagerTest, EventTimestampsFormatted) {
EXPECT_EQ("101:41:40 event4", events.back());
}

// Tests that previous session events are inserted at the start of the event
// log.
TEST_F(BreadcrumbManagerTest, SetPreviousSessionEvents) {
const auto& events = BreadcrumbManager::GetInstance().GetEvents();
ASSERT_EQ(0u, events.size());

std::vector<std::string> previous_events;
previous_events.push_back("0:00:00 event 1");
previous_events.push_back("0:00:00 event 2");
SetPreviousSessionEvents(previous_events);

// The previous session events should have been added to the event log.
EXPECT_EQ(2u, events.size());
EXPECT_EQ("0:00:00 event 1", events.front());
EXPECT_EQ("0:00:00 event 2", events.back());

previous_events.clear();
previous_events.push_back("0:00:00 event 3");
SetPreviousSessionEvents(previous_events);

// The previous session events should be at the front of the event log.
EXPECT_EQ(3u, events.size());
EXPECT_EQ("0:00:00 event 3", events.front());
}

// Tests that no more than `kMaxBreadcrumbs` events are stored after previous
// session events are retrieved.
TEST_F(BreadcrumbManagerTest, SetPreviousSessionEventsMaxEvents) {
const auto& events = BreadcrumbManager::GetInstance().GetEvents();
AddEvent("current event");
ASSERT_EQ(1u, events.size());

// Set the previous session events to a large list of events, such that the
// event log will become oversized when it's inserted.
const std::string previous_event = "0:00:00 previous event ";
std::vector<std::string> oversized_events;
oversized_events.reserve(kMaxBreadcrumbs);
int previous_event_num = 1;
for (size_t i = 0u; i < kMaxBreadcrumbs; i++) {
oversized_events.push_back(previous_event +
base::NumberToString(previous_event_num));
previous_event_num++;
}
ASSERT_EQ(kMaxBreadcrumbs, oversized_events.size());
SetPreviousSessionEvents(oversized_events);

// The oldest previous event should have been removed to keep the number of
// events limited to `kMaxBreadcrumbs`.
EXPECT_EQ(kMaxBreadcrumbs, events.size());
EXPECT_EQ("0:00:00 previous event 2", events.front());
EXPECT_EQ("0:00:00 current event", events.back());
}

} // namespace breadcrumbs

0 comments on commit c260741

Please sign in to comment.