Commit 2809a910 authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

Create the model once in ManagedBookmarkServiceTest.EmptyManagedNode

When migrating bookmarks sync to USS architecture, it assumes that
the bookmark model is creating only once.
ManagedBookmarkServiceTest.EmptyManagedNode is creating the model
twice which doesn't match the expecations of the new architecture.

This CL refactor the test to make sure the bookmark model is created
only once. In fact, the new scenario being tested resembles
reality a lot closer, since keyed services are never destroyed during
the lifetime of a profile.

Bug: 516866
Change-Id: I4a966270cc6bfcbdb7e1c87a096fea6a81e4b1b4
Reviewed-on: https://chromium-review.googlesource.com/1243075Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594496}
parent 717ca900
...@@ -35,6 +35,28 @@ using bookmarks::ManagedBookmarkService; ...@@ -35,6 +35,28 @@ using bookmarks::ManagedBookmarkService;
using testing::Mock; using testing::Mock;
using testing::_; using testing::_;
TEST(ManagedBookmarkServiceNoPolicyTest, EmptyManagedNode) {
// Verifies that the managed node is empty and invisible when the policy is
// not set.
content::TestBrowserThreadBundle thread_bundle;
TestingProfile profile;
// Make sure the policy isn't set.
ASSERT_EQ(nullptr, profile.GetTestingPrefService()->GetManagedPref(
bookmarks::prefs::kManagedBookmarks));
profile.CreateBookmarkModel(false);
BookmarkModel* model = BookmarkModelFactory::GetForBrowserContext(&profile);
bookmarks::test::WaitForBookmarkModelToLoad(model);
ManagedBookmarkService* managed =
ManagedBookmarkServiceFactory::GetForProfile(&profile);
DCHECK(managed);
ASSERT_TRUE(managed->managed_node());
EXPECT_TRUE(managed->managed_node()->empty());
EXPECT_FALSE(managed->managed_node()->IsVisible());
}
class ManagedBookmarkServiceTest : public testing::Test { class ManagedBookmarkServiceTest : public testing::Test {
public: public:
ManagedBookmarkServiceTest() : managed_(NULL), model_(NULL) {} ManagedBookmarkServiceTest() : managed_(NULL), model_(NULL) {}
...@@ -47,25 +69,23 @@ class ManagedBookmarkServiceTest : public testing::Test { ...@@ -47,25 +69,23 @@ class ManagedBookmarkServiceTest : public testing::Test {
// TODO(crbug.com/697817): Convert SetManagedPrefs to take a unique_ptr. // TODO(crbug.com/697817): Convert SetManagedPrefs to take a unique_ptr.
prefs_->SetManagedPref(bookmarks::prefs::kManagedBookmarks, prefs_->SetManagedPref(bookmarks::prefs::kManagedBookmarks,
CreateTestTree()); CreateTestTree());
ResetModel();
// The managed node always exists.
ASSERT_TRUE(managed_->managed_node());
ASSERT_TRUE(managed_->managed_node()->parent() == model_->root_node());
EXPECT_NE(-1, model_->root_node()->GetIndexOf(managed_->managed_node()));
}
void TearDown() override { model_->RemoveObserver(&observer_); } // Create and load the bookmark model.
void ResetModel() {
profile_.CreateBookmarkModel(false); profile_.CreateBookmarkModel(false);
model_ = BookmarkModelFactory::GetForBrowserContext(&profile_); model_ = BookmarkModelFactory::GetForBrowserContext(&profile_);
bookmarks::test::WaitForBookmarkModelToLoad(model_); bookmarks::test::WaitForBookmarkModelToLoad(model_);
model_->AddObserver(&observer_); model_->AddObserver(&observer_);
managed_ = ManagedBookmarkServiceFactory::GetForProfile(&profile_); managed_ = ManagedBookmarkServiceFactory::GetForProfile(&profile_);
DCHECK(managed_); DCHECK(managed_);
// The managed node always exists.
ASSERT_TRUE(managed_->managed_node());
ASSERT_TRUE(managed_->managed_node()->parent() == model_->root_node());
EXPECT_NE(-1, model_->root_node()->GetIndexOf(managed_->managed_node()));
} }
void TearDown() override { model_->RemoveObserver(&observer_); }
static std::unique_ptr<base::DictionaryValue> CreateBookmark( static std::unique_ptr<base::DictionaryValue> CreateBookmark(
const std::string& title, const std::string& title,
const std::string& url) { const std::string& url) {
...@@ -145,18 +165,6 @@ class ManagedBookmarkServiceTest : public testing::Test { ...@@ -145,18 +165,6 @@ class ManagedBookmarkServiceTest : public testing::Test {
DISALLOW_COPY_AND_ASSIGN(ManagedBookmarkServiceTest); DISALLOW_COPY_AND_ASSIGN(ManagedBookmarkServiceTest);
}; };
TEST_F(ManagedBookmarkServiceTest, EmptyManagedNode) {
// Verifies that the managed node is empty and invisible when the policy is
// not set.
model_->RemoveObserver(&observer_);
prefs_->RemoveManagedPref(bookmarks::prefs::kManagedBookmarks);
ResetModel();
ASSERT_TRUE(managed_->managed_node());
EXPECT_TRUE(managed_->managed_node()->empty());
EXPECT_FALSE(managed_->managed_node()->IsVisible());
}
TEST_F(ManagedBookmarkServiceTest, LoadInitial) { TEST_F(ManagedBookmarkServiceTest, LoadInitial) {
// Verifies that the initial load picks up the initial policy too. // Verifies that the initial load picks up the initial policy too.
EXPECT_TRUE(model_->bookmark_bar_node()->empty()); EXPECT_TRUE(model_->bookmark_bar_node()->empty());
......
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