Skip to content

Commit

Permalink
[power bookmarks] Add basic local storage implementation
Browse files Browse the repository at this point in the history 
Implement basic CRUD functions and add unit tests for them.

Bug: 1376612
Change-Id: If5b1aa372a7eef5b45803e49dad8ea0d1a9bfaa5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3966724
Reviewed-by: bttk - <bttk@chromium.org>
Reviewed-by: Yuheng Huang <yuhengh@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1069847}
  • Loading branch information
Brandon Wylie authored and Chromium LUCI CQ committed Nov 10, 2022
1 parent 07ebeb6 commit 9a9461f
Show file tree
Hide file tree
Showing 21 changed files with 1,234 additions and 91 deletions.
1 change: 1 addition & 0 deletions components/BUILD.gn
View file
Expand Up @@ -189,6 +189,7 @@ test("components_unittests") {
"//components/policy/core/browser:unit_tests",
"//components/policy/core/common:unit_tests",
"//components/power_bookmarks/core:unit_tests",
"//components/power_bookmarks/storage:unit_tests",
"//components/power_scheduler:unit_tests",
"//components/prefs:unit_tests",
"//components/profile_metrics:unit_tests",
Expand Down
3 changes: 1 addition & 2 deletions components/power_bookmarks/core/BUILD.gn
View file
Expand Up @@ -52,8 +52,7 @@ proto_library("proto") {
proto_in_dir = "//"
sources = [
"proto/power_bookmark_meta.proto",
"proto/power_specifics.proto",
"proto/save_specifics.proto",
"proto/power_bookmark_specifics.proto",
"proto/shopping_specifics.proto",
]
deps = [ "//components/commerce/core:proto" ]
Expand Down
16 changes: 13 additions & 3 deletions components/power_bookmarks/core/power_bookmark_service.cc
View file
Expand Up @@ -28,7 +28,10 @@ PowerBookmarkService::PowerBookmarkService(

backend_ = base::SequenceBound<PowerBookmarkBackend>(backend_task_runner_,
database_dir);
backend_.AsyncCall(&PowerBookmarkBackend::Init);
// Features that wish to use the real database, must call
// `InitPowerBookmarkDatabase`.
backend_.AsyncCall(&PowerBookmarkBackend::Init)
.WithArgs(/*use_database=*/false);
}

PowerBookmarkService::~PowerBookmarkService() {
Expand All @@ -39,10 +42,16 @@ PowerBookmarkService::~PowerBookmarkService() {
backend_task_runner_ = nullptr;
}

void PowerBookmarkService::InitPowerBookmarkDatabase() {
backend_.AsyncCall(&PowerBookmarkBackend::Init)
.WithArgs(/*use_database=*/true);
}

void PowerBookmarkService::GetPowersForURL(const GURL& url,
const PowerType& power_type,
PowersCallback callback) {
backend_.AsyncCall(&PowerBookmarkBackend::GetPowersForURL)
.WithArgs(url)
.WithArgs(url, power_type)
.Then(std::move(callback));
}

Expand Down Expand Up @@ -76,9 +85,10 @@ void PowerBookmarkService::DeletePower(const base::GUID& guid,
}

void PowerBookmarkService::DeletePowersForURL(const GURL& url,
const PowerType& power_type,
SuccessCallback callback) {
backend_.AsyncCall(&PowerBookmarkBackend::DeletePowersForURL)
.WithArgs(url)
.WithArgs(url, power_type)
.Then(std::move(callback));
}

Expand Down
21 changes: 16 additions & 5 deletions components/power_bookmarks/core/power_bookmark_service.h
View file
Expand Up @@ -13,7 +13,7 @@
#include "base/threading/sequence_bound.h"
#include "components/bookmarks/browser/base_bookmark_model_observer.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/power_bookmarks/core/proto/save_specifics.pb.h"
#include "components/power_bookmarks/core/proto/power_bookmark_specifics.pb.h"

namespace bookmarks {
class BookmarkModel;
Expand Down Expand Up @@ -45,9 +45,14 @@ class PowerBookmarkService : public KeyedService,

~PowerBookmarkService() override;

void InitPowerBookmarkDatabase();

// Returns a vector of Powers for the given `url` through the given
// `callback`.
void GetPowersForURL(const GURL& url, PowersCallback callback);
// `callback`. Use `power_type` to restrict which type is returned or use
// POWER_TYPE_UNSPECIFIED to return everything.
void GetPowersForURL(const GURL& url,
const PowerType& power_type,
PowersCallback callback);

// Returns a vector of PowerOverviews for the given `power_type` through the
// given `callback`.
Expand All @@ -58,17 +63,23 @@ class PowerBookmarkService : public KeyedService,
// will be updated. Success of the operation is returned through the given
// `callback`.
void CreatePower(std::unique_ptr<Power> power, SuccessCallback callback);

// Update the given `power` in the database. If it doesn't exist, then it
// will be created instead. Success of the operation is returned through the
// given `callback`.
void UpdatePower(std::unique_ptr<Power> power, SuccessCallback callback);

// Delete the given `guid` in the database, if it exists. Success of the
// operation is returned through the given `callback`.
// TODO(crbug.com/1378793): Encapsulate the storage key if possible.
void DeletePower(const base::GUID& guid, SuccessCallback callback);

// Delete all powers for the given `url`. Success of the operation is
// returned through the given `callback`.
void DeletePowersForURL(const GURL& url, SuccessCallback callback);
// returned through the given `callback`. Use `power_type` to restrict which
// type is deleted or use POWER_TYPE_UNSPECIFIED to delete everything.
void DeletePowersForURL(const GURL& url,
const PowerType& power_type,
SuccessCallback callback);

// Allow features to receive notification when a bookmark node is created to
// add extra information. The `data_provider` can be removed with the remove
Expand Down
126 changes: 114 additions & 12 deletions components/power_bookmarks/core/power_bookmark_service_unittest.cc
View file
Expand Up @@ -5,6 +5,7 @@
#include <memory>

#include "base/files/scoped_temp_dir.h"
#include "base/guid.h"
#include "base/task/sequenced_task_runner.h"
#include "base/task/thread_pool.h"
#include "base/test/mock_callback.h"
Expand All @@ -22,9 +23,14 @@
using testing::_;
using testing::IsEmpty;
using testing::IsFalse;
using testing::IsTrue;
using testing::SizeIs;

namespace power_bookmarks {

// Tests for the power bookmark service.
// In-depth tests for the actual storage can be found in
// `power_bookmark_database_unittest`.
class PowerBookmarkServiceTest : public testing::Test {
protected:
void SetUp() override {
Expand All @@ -37,6 +43,7 @@ class PowerBookmarkServiceTest : public testing::Test {
service_ = std::make_unique<PowerBookmarkService>(
model_.get(), temp_directory_.GetPath(), backend_task_runner_);
RunUntilIdle();
service_->InitPowerBookmarkDatabase();
}

void TearDown() override {
Expand Down Expand Up @@ -108,24 +115,66 @@ TEST_F(PowerBookmarkServiceTest, Shutdown) {
}

TEST_F(PowerBookmarkServiceTest, GetPowersForURL) {
base::MockCallback<PowersCallback> cb;
EXPECT_CALL(cb, Run(IsEmpty()));
base::MockCallback<SuccessCallback> success_cb;
EXPECT_CALL(success_cb, Run(IsTrue()));

service()->GetPowersForURL(GURL("https://google.com"), cb.Get());
std::unique_ptr<PowerSpecifics> power_specifics =
std::make_unique<PowerSpecifics>();
std::unique_ptr<Power> power =
std::make_unique<Power>(std::move(power_specifics));
power->set_url(GURL("https://google.com"));
power->set_power_type(PowerType::POWER_TYPE_MOCK);
service()->CreatePower(std::move(power), success_cb.Get());
RunUntilIdle();

base::MockCallback<PowersCallback> powers_cb;
EXPECT_CALL(powers_cb, Run(SizeIs(1)));

service()->GetPowersForURL(GURL("https://google.com"),
PowerType::POWER_TYPE_UNSPECIFIED,
powers_cb.Get());
RunUntilIdle();
}

TEST_F(PowerBookmarkServiceTest, GetPowerOverviewsForType) {
base::MockCallback<SuccessCallback> success_cb;
EXPECT_CALL(success_cb, Run(IsTrue()));

std::unique_ptr<PowerSpecifics> power_specifics =
std::make_unique<PowerSpecifics>();
std::unique_ptr<Power> power =
std::make_unique<Power>(std::move(power_specifics));
power->set_url(GURL("https://google.com"));
power->set_power_type(PowerType::POWER_TYPE_MOCK);
service()->CreatePower(std::move(power), success_cb.Get());
RunUntilIdle();

EXPECT_CALL(success_cb, Run(IsTrue()));
power_specifics = std::make_unique<PowerSpecifics>();
power = std::make_unique<Power>(std::move(power_specifics));
power->set_url(GURL("https://google.com"));
power->set_power_type(PowerType::POWER_TYPE_MOCK);
service()->CreatePower(std::move(power), success_cb.Get());
RunUntilIdle();

EXPECT_CALL(success_cb, Run(IsTrue()));
power_specifics = std::make_unique<PowerSpecifics>();
power = std::make_unique<Power>(std::move(power_specifics));
power->set_url(GURL("https://boogle.com"));
power->set_power_type(PowerType::POWER_TYPE_MOCK);
service()->CreatePower(std::move(power), success_cb.Get());
RunUntilIdle();

base::MockCallback<PowerOverviewsCallback> cb;
EXPECT_CALL(cb, Run(IsEmpty()));
EXPECT_CALL(cb, Run(SizeIs(2)));

service()->GetPowerOverviewsForType(PowerType::POWER_TYPE_MOCK, cb.Get());
RunUntilIdle();
}

TEST_F(PowerBookmarkServiceTest, CreatePower) {
base::MockCallback<SuccessCallback> cb;
EXPECT_CALL(cb, Run(IsFalse()));
EXPECT_CALL(cb, Run(IsTrue()));

std::unique_ptr<PowerSpecifics> power_specifics =
std::make_unique<PowerSpecifics>();
Expand All @@ -139,7 +188,7 @@ TEST_F(PowerBookmarkServiceTest, CreatePower) {

TEST_F(PowerBookmarkServiceTest, UpdatePower) {
base::MockCallback<SuccessCallback> cb;
EXPECT_CALL(cb, Run(IsFalse()));
EXPECT_CALL(cb, Run(IsTrue()));

std::unique_ptr<PowerSpecifics> power_specifics =
std::make_unique<PowerSpecifics>();
Expand All @@ -152,24 +201,77 @@ TEST_F(PowerBookmarkServiceTest, UpdatePower) {
}

TEST_F(PowerBookmarkServiceTest, DeletePower) {
base::MockCallback<SuccessCallback> cb;
EXPECT_CALL(cb, Run(IsFalse()));
base::MockCallback<SuccessCallback> success_cb;
EXPECT_CALL(success_cb, Run(IsTrue()));

base::GUID guid = base::GUID::GenerateRandomV4();
std::unique_ptr<PowerSpecifics> power_specifics =
std::make_unique<PowerSpecifics>();
std::unique_ptr<Power> power =
std::make_unique<Power>(std::move(power_specifics));
power->set_guid(guid);
power->set_url(GURL("https://google.com"));
power->set_power_type(PowerType::POWER_TYPE_MOCK);
service()->DeletePower(power->guid(), cb.Get());
service()->CreatePower(std::move(power), success_cb.Get());
RunUntilIdle();

base::MockCallback<PowersCallback> powers_cb;
EXPECT_CALL(powers_cb, Run(SizeIs(1)));
service()->GetPowersForURL(GURL("https://google.com"),
PowerType::POWER_TYPE_UNSPECIFIED,
powers_cb.Get());
RunUntilIdle();

EXPECT_CALL(success_cb, Run(IsTrue()));
service()->DeletePower(guid, success_cb.Get());
RunUntilIdle();

EXPECT_CALL(powers_cb, Run(SizeIs(0)));
service()->GetPowersForURL(GURL("https://google.com"),
PowerType::POWER_TYPE_UNSPECIFIED,
powers_cb.Get());
RunUntilIdle();
}

TEST_F(PowerBookmarkServiceTest, DeletePowersForURL) {
base::MockCallback<SuccessCallback> cb;
EXPECT_CALL(cb, Run(IsFalse()));
base::MockCallback<SuccessCallback> success_cb;
EXPECT_CALL(success_cb, Run(IsTrue()));

std::unique_ptr<PowerSpecifics> power_specifics =
std::make_unique<PowerSpecifics>();
std::unique_ptr<Power> power =
std::make_unique<Power>(std::move(power_specifics));
power->set_url(GURL("https://google.com"));
power->set_power_type(PowerType::POWER_TYPE_MOCK);
service()->CreatePower(std::move(power), success_cb.Get());
RunUntilIdle();

EXPECT_CALL(success_cb, Run(IsTrue()));
power_specifics = std::make_unique<PowerSpecifics>();
power = std::make_unique<Power>(std::move(power_specifics));
power->set_url(GURL("https://google.com"));
power->set_power_type(PowerType::POWER_TYPE_MOCK);
service()->CreatePower(std::move(power), success_cb.Get());
RunUntilIdle();

base::MockCallback<PowersCallback> powers_cb;
EXPECT_CALL(powers_cb, Run(SizeIs(2)));

service()->GetPowersForURL(GURL("https://google.com"),
PowerType::POWER_TYPE_UNSPECIFIED,
powers_cb.Get());
RunUntilIdle();

EXPECT_CALL(success_cb, Run(IsTrue()));
service()->DeletePowersForURL(GURL("https://google.com"),
PowerType::POWER_TYPE_UNSPECIFIED,
success_cb.Get());
RunUntilIdle();

service()->DeletePowersForURL(GURL("https://google.com"), cb.Get());
EXPECT_CALL(powers_cb, Run(SizeIs(0)));
service()->GetPowersForURL(GURL("https://google.com"),
PowerType::POWER_TYPE_UNSPECIFIED,
powers_cb.Get());
RunUntilIdle();
}

Expand Down
30 changes: 14 additions & 16 deletions components/power_bookmarks/core/powers/power.cc
View file
Expand Up @@ -4,46 +4,44 @@

#include "components/power_bookmarks/core/powers/power.h"

#include "components/power_bookmarks/core/proto/power_specifics.pb.h"

namespace power_bookmarks {

Power::Power(std::unique_ptr<PowerSpecifics> power_specifics) {
CHECK(power_specifics);
power_specifics_ = std::move(power_specifics);
}

Power::Power(SaveSpecifics& save_specifics) {
guid_ = base::GUID::ParseLowercase(save_specifics.guid());
url_ = GURL(save_specifics.url());
power_type_ = save_specifics.power_type();
Power::Power(const PowerBookmarkSpecifics& specifics) {
guid_ = base::GUID::ParseLowercase(specifics.guid());
url_ = GURL(specifics.url());
power_type_ = specifics.power_type();

// See:
// https://source.chromium.org/chromium/chromium/src/+/main:base/time/time.h;l=49-60;drc=e8d37dfe21715cd84536a1412b778e1a5a39fb0c
time_added_ = base::Time::FromDeltaSinceWindowsEpoch(
base::Microseconds(save_specifics.creation_time_usec()));
base::Microseconds(specifics.creation_time_usec()));
time_modified_ = base::Time::FromDeltaSinceWindowsEpoch(
base::Microseconds(save_specifics.update_time_usec()));
base::Microseconds(specifics.update_time_usec()));

power_specifics_ = std::make_unique<PowerSpecifics>();
power_specifics_->CopyFrom(save_specifics.power_specifics());
power_specifics_->CopyFrom(specifics.power_specifics());
}

Power::~Power() = default;

void Power::ToSaveSpecifics(SaveSpecifics* save_specifics) {
save_specifics->set_guid(guid_.AsLowercaseString());
save_specifics->set_url(url_.spec());
save_specifics->set_power_type(power_type_);
void Power::ToPowerBookmarkSpecifics(PowerBookmarkSpecifics* specifics) {
specifics->set_guid(guid_.AsLowercaseString());
specifics->set_url(url_.spec());
specifics->set_power_type(power_type_);

// See:
// https://source.chromium.org/chromium/chromium/src/+/main:base/time/time.h;l=49-60;drc=e8d37dfe21715cd84536a1412b778e1a5a39fb0c
save_specifics->set_creation_time_usec(
specifics->set_creation_time_usec(
time_added_.ToDeltaSinceWindowsEpoch().InMicroseconds());
save_specifics->set_update_time_usec(
specifics->set_update_time_usec(
time_modified_.ToDeltaSinceWindowsEpoch().InMicroseconds());

save_specifics->mutable_power_specifics()->CopyFrom(*power_specifics_.get());
specifics->mutable_power_specifics()->CopyFrom(*power_specifics_.get());
}

} // namespace power_bookmarks

0 comments on commit 9a9461f

Please sign in to comment.