Commit c883abe3 authored by Anne Lim's avatar Anne Lim Committed by Commit Bot

[Autofill] Remove expired strikes on StrikeDB init

Added GetExpiryTime() to StrikeDatabase interface and removes
expired striked on StrikeDB init.

Bug: 884817
Change-Id: Ie99173b631f668f3be848f1afa925d7feec801ef
Reviewed-on: https://chromium-review.googlesource.com/c/1351887Reviewed-by: default avatarSebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: default avatarJared Saul <jsaul@google.com>
Commit-Queue: Anne Lim <annelim@google.com>
Cr-Commit-Position: refs/heads/master@{#612754}
parent afad373b
......@@ -22,4 +22,9 @@ int CreditCardSaveStrikeDatabase::GetMaxStrikesLimit() {
return 3;
}
long long CreditCardSaveStrikeDatabase::GetExpiryTimeMicros() {
// Expiry time is 6 months.
return (long long)1000000 * 60 * 60 * 24 * 180;
}
} // namespace autofill
......@@ -20,6 +20,7 @@ class CreditCardSaveStrikeDatabase : public StrikeDatabase {
std::string GetProjectPrefix() override;
int GetMaxStrikesLimit() override;
long long GetExpiryTimeMicros() override;
};
} // namespace autofill
......
......@@ -13,7 +13,9 @@
#include "base/test/scoped_task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/autofill/core/browser/proto/strike_data.pb.h"
#include "components/autofill/core/browser/test_autofill_clock.h"
#include "components/autofill/core/browser/test_credit_card_save_strike_database.h"
#include "components/autofill/core/common/autofill_clock.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace autofill {
......@@ -44,6 +46,11 @@ TEST_F(CreditCardSaveStrikeDatabaseTest, GetKeyForCreditCardSaveTest) {
EXPECT_EQ("CreditCardSave__1234", strike_database_.GetKey(last_four));
}
TEST_F(CreditCardSaveStrikeDatabaseTest, GetIdForCreditCardSaveTest) {
const std::string key = "CreditCardSave__1234";
EXPECT_EQ("1234", strike_database_.GetIdPartFromKey(key));
}
TEST_F(CreditCardSaveStrikeDatabaseTest, MaxStrikesLimitReachedTest) {
const std::string last_four = "1234";
EXPECT_EQ(false, strike_database_.IsMaxStrikesLimitReached(last_four));
......@@ -102,4 +109,47 @@ TEST_F(CreditCardSaveStrikeDatabaseTest, ClearStrikesForZeroStrikesTest) {
EXPECT_EQ(0, strike_database_.GetStrikes(last_four));
}
TEST_F(CreditCardSaveStrikeDatabaseTest, RemoveExpiredStrikesOnLoadTest) {
autofill::TestAutofillClock test_clock;
test_clock.SetNow(AutofillClock::Now());
const std::string last_four1 = "1234";
const std::string last_four2 = "9876";
StrikeData data1;
data1.set_num_strikes(2);
data1.set_last_update_timestamp(
AutofillClock::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
// Advance clock to past |data1|'s expiry time.
test_clock.Advance(base::TimeDelta::FromMicroseconds(
strike_database_.GetExpiryTimeMicros() + 1));
StrikeData data2;
data2.set_num_strikes(2);
data2.set_last_update_timestamp(
AutofillClock::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
std::map<std::string, StrikeData> entries;
entries[strike_database_.GetKey(last_four1)] = data1;
entries[strike_database_.GetKey(last_four2)] = data2;
// |data1| should have its most recent strike expire, but |data2| should not.
strike_database_.OnDatabaseLoadKeysAndEntries(
true, std::make_unique<std::map<std::string, StrikeData>>(entries));
EXPECT_EQ(1, strike_database_.GetStrikes(last_four1));
EXPECT_EQ(2, strike_database_.GetStrikes(last_four2));
// Advance clock to past both |data1| andd |data2|'s expiry time.
test_clock.Advance(base::TimeDelta::FromMicroseconds(
strike_database_.GetExpiryTimeMicros() + 1));
// |data1| and |data2| should both have its most recent strike expire.
strike_database_.OnDatabaseLoadKeysAndEntries(
true, std::make_unique<std::map<std::string, StrikeData>>(entries));
EXPECT_EQ(0, strike_database_.GetStrikes(last_four1));
EXPECT_EQ(1, strike_database_.GetStrikes(last_four2));
}
} // namespace autofill
......@@ -14,6 +14,7 @@
#include "base/task/post_task.h"
#include "base/time/time.h"
#include "components/autofill/core/browser/proto/strike_data.pb.h"
#include "components/autofill/core/common/autofill_clock.h"
#include "components/leveldb_proto/proto_database_impl.h"
namespace autofill {
......@@ -48,18 +49,25 @@ int StrikeDatabase::AddStrike(const std::string id) {
int num_strikes = strike_map_cache_.count(key) // Cache has entry for |key|.
? strike_map_cache_[key].num_strikes() + 1
: 1;
StrikeData data;
data.set_num_strikes(num_strikes);
data.set_last_update_timestamp(
base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
UpdateCache(key, data);
SetProtoStrikeData(key, data, base::DoNothing());
SetStrikeData(key, num_strikes);
base::UmaHistogramCounts1000(
"Autofill.StrikeDatabase.NthStrikeAdded." + GetProjectPrefix(),
num_strikes);
return num_strikes;
}
int StrikeDatabase::RemoveStrike(const std::string id) {
std::string key = GetKey(id);
DCHECK(strike_map_cache_.count(key));
int num_strikes = strike_map_cache_[key].num_strikes() - 1;
if (num_strikes < 1) {
ClearStrikes(id);
return 0;
}
SetStrikeData(key, num_strikes);
return num_strikes;
}
int StrikeDatabase::GetStrikes(const std::string id) {
std::string key = GetKey(id);
return strike_map_cache_.count(key) // Cache contains entry for |key|.
......@@ -105,12 +113,34 @@ void StrikeDatabase::OnDatabaseLoadKeysAndEntries(
return;
}
strike_map_cache_.insert(entries->begin(), entries->end());
// Remove all expired strikes.
for (auto entry : *entries) {
if (AutofillClock::Now().ToDeltaSinceWindowsEpoch().InMicroseconds() -
entry.second.last_update_timestamp() >
GetExpiryTimeMicros()) {
if (GetStrikes(GetIdPartFromKey(entry.first)) > 0)
RemoveStrike(GetIdPartFromKey(entry.first));
}
}
}
std::string StrikeDatabase::GetKey(const std::string id) {
return GetProjectPrefix() + kKeyDeliminator + id;
}
std::string StrikeDatabase::GetIdPartFromKey(const std::string key) {
return key.substr((GetProjectPrefix() + kKeyDeliminator).size());
}
void StrikeDatabase::SetStrikeData(const std::string key, int num_strikes) {
StrikeData data;
data.set_num_strikes(num_strikes);
data.set_last_update_timestamp(
base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
UpdateCache(key, data);
SetProtoStrikeData(key, data, base::DoNothing());
}
void StrikeDatabase::GetProtoStrikes(const std::string key,
const StrikesCallback& outer_callback) {
if (!database_initialized_) {
......
......@@ -48,6 +48,10 @@ class StrikeDatabase : public KeyedService {
// Increments in-memory cache and updates underlying ProtoDatabase.
int AddStrike(const std::string id);
// Removes an in-memory cache strike, updates last_update_timestamp, and
// updates underlying ProtoDatabase.
int RemoveStrike(const std::string id);
// Returns strike count from in-memory cache.
int GetStrikes(const std::string id);
......@@ -80,6 +84,10 @@ class StrikeDatabase : public KeyedService {
StrikeDatabaseEmptyOnAutofillRemoveEverything);
FRIEND_TEST_ALL_PREFIXES(CreditCardSaveStrikeDatabaseTest,
GetKeyForCreditCardSaveTest);
FRIEND_TEST_ALL_PREFIXES(CreditCardSaveStrikeDatabaseTest,
GetIdForCreditCardSaveTest);
FRIEND_TEST_ALL_PREFIXES(CreditCardSaveStrikeDatabaseTest,
RemoveExpiredStrikesOnLoadTest);
friend class StrikeDatabaseTest;
friend class StrikeDatabaseTester;
......@@ -97,9 +105,19 @@ class StrikeDatabase : public KeyedService {
// opportunity stops being offered.
virtual int GetMaxStrikesLimit() = 0;
// Returns the time after which the most recent strike should expire.
virtual long long GetExpiryTimeMicros() = 0;
// Generates key based on project-specific string identifier.
std::string GetKey(const std::string id);
// Generates project-specific string identifier based on key.
std::string GetIdPartFromKey(const std::string key);
// Updates the StrikeData for |key| in the cache and ProtoDatabase to have
// |num_stikes|, and the current time as timestamp.
void SetStrikeData(const std::string key, int num_strikes);
// Passes the number of strikes for |key| to |outer_callback|. In the case
// that the database fails to retrieve the strike update or if no entry is
// found for |key|, 0 is passed.
......
......@@ -59,6 +59,13 @@ class TestStrikeDatabase : public StrikeDatabase {
NOTIMPLEMENTED();
return 0;
}
// Do not use. This virtual function needed to be implemented but
// TestStrikeDatabase is not a project class.
long long GetExpiryTimeMicros() override {
NOTIMPLEMENTED();
return 0;
}
};
} // anonymous namespace
......
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