Commit d893e419 authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Notification scheduler: Remove sort for impression tracker.

This CL removes the sorting for impressions. Also move the logic that
prunes expired impressions to the point when the database is just
initialized.

This can save APK size by 4000 bytes according to diagnose_bloat.py.

Resource Sizes Diff:
Specifics:
        -4,096 bytes main lib size
        -4,002 bytes normalized apk size
InstallSize:
        -4,098 bytes APK size
        -4,098 bytes Estimated installed size (Android Go)
        -4,098 bytes Estimated installed size
InstallBreakdown (-4,098 bytes):
        -4,096 bytes Native code size
            -2 bytes Package metadata size

Bug: 972191
Change-Id: Ic6f2e971fb12de5c3b5affabd11c26b4512ea170
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761331Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688336}
parent 1242f4e1
...@@ -15,11 +15,6 @@ ...@@ -15,11 +15,6 @@
namespace notifications { namespace notifications {
namespace { namespace {
// Comparator used to sort notification entries based on creation time.
bool CreateTimeCompare(const Impression& lhs, const Impression& rhs) {
return lhs.create_time < rhs.create_time;
}
std::string ToDatabaseKey(SchedulerClientType type) { std::string ToDatabaseKey(SchedulerClientType type) {
switch (type) { switch (type) {
case SchedulerClientType::kTest1: case SchedulerClientType::kTest1:
...@@ -151,16 +146,25 @@ void ImpressionHistoryTrackerImpl::OnStoreInitialized( ...@@ -151,16 +146,25 @@ void ImpressionHistoryTrackerImpl::OnStoreInitialized(
initialized_ = true; initialized_ = true;
// Load the data to memory, and sort the impression list. // Load the data to memory, and prune expired impressions.
auto now = clock_->Now();
for (auto it = entries.begin(); it != entries.end(); ++it) { for (auto it = entries.begin(); it != entries.end(); ++it) {
auto& entry = (*it); auto& entry = (*it);
auto type = entry->type; auto type = entry->type;
std::sort(entry->impressions.begin(), entry->impressions.end(), ClientState::Impressions impressions;
&CreateTimeCompare);
for (auto& impression : entry->impressions) { for (auto& impression : entry->impressions) {
impression_map_.emplace(impression.guid, &impression); bool expired =
now - impression.create_time > config_.impression_expiration;
if (expired) {
SetNeedsUpdate(type, true);
} else {
impressions.emplace_back(impression);
impression_map_.emplace(impression.guid, &impressions.back());
}
} }
entry->impressions.swap(impressions);
client_states_.emplace(type, std::move(*it)); client_states_.emplace(type, std::move(*it));
MaybeUpdateDb(type);
} }
SyncRegisteredClients(); SyncRegisteredClients();
...@@ -200,17 +204,6 @@ void ImpressionHistoryTrackerImpl::AnalyzeImpressionHistory( ...@@ -200,17 +204,6 @@ void ImpressionHistoryTrackerImpl::AnalyzeImpressionHistory(
ClientState* client_state) { ClientState* client_state) {
DCHECK(client_state); DCHECK(client_state);
base::circular_deque<Impression*> dismisses; base::circular_deque<Impression*> dismisses;
base::Time now = clock_->Now();
// Prune out expired impression.
while (!client_state->impressions.empty() &&
now - client_state->impressions.front().create_time >
config_.impression_expiration) {
impression_map_.erase(client_state->impressions.front().guid);
client_state->impressions.pop_front();
SetNeedsUpdate(client_state->type, true);
}
for (auto it = client_state->impressions.begin(); for (auto it = client_state->impressions.begin();
it != client_state->impressions.end(); ++it) { it != client_state->impressions.end(); ++it) {
auto* impression = &*it; auto* impression = &*it;
......
...@@ -210,20 +210,13 @@ TEST_F(ImpressionHistoryTrackerTest, DeleteExpiredImpression) { ...@@ -210,20 +210,13 @@ TEST_F(ImpressionHistoryTrackerTest, DeleteExpiredImpression) {
Impression expired = CreateImpression(expired_create_time, "guid1"); Impression expired = CreateImpression(expired_create_time, "guid1");
Impression not_expired = CreateImpression(not_expired_time, "guid2"); Impression not_expired = CreateImpression(not_expired_time, "guid2");
// The impressions in the input should be sorted by creation time when gets
// loaded to memory.
test_case.input.back().impressions = {expired, not_expired, expired}; test_case.input.back().impressions = {expired, not_expired, expired};
// Expired impression created in |expired_create_time| should be deleted.
// No change expected on the next impression, which is not expired and no user
// feedback .
test_case.expected.back().impressions = {not_expired}; test_case.expected.back().impressions = {not_expired};
CreateTracker(test_case); CreateTracker(test_case);
InitTrackerWithData(test_case);
EXPECT_CALL(*store(), Update(_, _, _)); EXPECT_CALL(*store(), Update(_, _, _));
EXPECT_CALL(*delegate(), OnImpressionUpdated()); InitTrackerWithData(test_case);
tracker()->AnalyzeImpressionHistory(); EXPECT_CALL(*delegate(), OnImpressionUpdated()).Times(0);
VerifyClientStates(test_case); VerifyClientStates(test_case);
} }
......
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