Commit 8c8047ec authored by Kristi Park's avatar Kristi Park Committed by Commit Bot

Add ability to reorder custom links

Add functionality for reordering custom links. Will initialize if
custom links has not been initialized yet.

Bug: 851335
Change-Id: I67710b4187f8f5a45d7f96814c17294f87d60b22
Reviewed-on: https://chromium-review.googlesource.com/c/1308899
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604419}
parent 41d82520
...@@ -69,6 +69,11 @@ class CustomLinksManager { ...@@ -69,6 +69,11 @@ class CustomLinksManager {
virtual bool UpdateLink(const GURL& url, virtual bool UpdateLink(const GURL& url,
const GURL& new_url, const GURL& new_url,
const base::string16& new_title) = 0; const base::string16& new_title) = 0;
// Moves the specified link from its current index and inserts it at
// |new_pos|. Returns false and does nothing if custom links is not
// initialized, |url| is invalid, |url| does not exist in the list, or
// |new_pos| is invalid/already the current index.
virtual bool ReorderLink(const GURL& url, size_t new_pos) = 0;
// Deletes the link with the specified |url|. Returns false and does nothing // Deletes the link with the specified |url|. Returns false and does nothing
// if custom links is not initialized, |url| is invalid, or |url| does not // if custom links is not initialized, |url| is invalid, or |url| does not
// exist in the list. // exist in the list.
......
...@@ -124,6 +124,35 @@ bool CustomLinksManagerImpl::UpdateLink(const GURL& url, ...@@ -124,6 +124,35 @@ bool CustomLinksManagerImpl::UpdateLink(const GURL& url,
return true; return true;
} }
bool CustomLinksManagerImpl::ReorderLink(const GURL& url, size_t new_pos) {
if (!IsInitialized() || !url.is_valid() || new_pos < 0 ||
new_pos >= current_links_.size()) {
return false;
}
auto curr_it = FindLinkWithUrl(url);
if (curr_it == current_links_.end())
return false;
auto new_it = current_links_.begin() + new_pos;
if (new_it == curr_it)
return false;
previous_links_ = current_links_;
// If the new position is to the left of the current position, left rotate the
// range [new_pos, curr_pos] until the link is first.
if (new_it < curr_it)
std::rotate(new_it, curr_it, curr_it + 1);
// If the new position is to the right, we only need to left rotate the range
// [curr_pos, new_pos] once so that the link is last.
else
std::rotate(curr_it, curr_it + 1, new_it + 1);
StoreLinks();
return true;
}
bool CustomLinksManagerImpl::DeleteLink(const GURL& url) { bool CustomLinksManagerImpl::DeleteLink(const GURL& url) {
if (!IsInitialized() || !url.is_valid()) if (!IsInitialized() || !url.is_valid())
return false; return false;
......
...@@ -49,6 +49,7 @@ class CustomLinksManagerImpl : public CustomLinksManager, ...@@ -49,6 +49,7 @@ class CustomLinksManagerImpl : public CustomLinksManager,
bool UpdateLink(const GURL& url, bool UpdateLink(const GURL& url,
const GURL& new_url, const GURL& new_url,
const base::string16& new_title) override; const base::string16& new_title) override;
bool ReorderLink(const GURL& url, size_t new_pos) override;
bool DeleteLink(const GURL& url) override; bool DeleteLink(const GURL& url) override;
bool UndoAction() override; bool UndoAction() override;
......
...@@ -34,6 +34,11 @@ const TestCaseItem kTestCase2[] = { ...@@ -34,6 +34,11 @@ const TestCaseItem kTestCase2[] = {
{"http://foo1.com/", "Foo1"}, {"http://foo1.com/", "Foo1"},
{"http://foo2.com/", "Foo2"}, {"http://foo2.com/", "Foo2"},
}; };
const TestCaseItem kTestCase3[] = {
{"http://foo1.com/", "Foo1"},
{"http://foo2.com/", "Foo2"},
{"http://foo3.com/", "Foo3"},
};
const TestCaseItem kTestCaseMax[] = { const TestCaseItem kTestCaseMax[] = {
{"http://foo1.com/", "Foo1"}, {"http://foo2.com/", "Foo2"}, {"http://foo1.com/", "Foo1"}, {"http://foo2.com/", "Foo2"},
{"http://foo3.com/", "Foo3"}, {"http://foo4.com/", "Foo4"}, {"http://foo3.com/", "Foo3"}, {"http://foo4.com/", "Foo4"},
...@@ -278,6 +283,54 @@ TEST_F(CustomLinksManagerImplTest, UpdateLinkWhenUrlAlreadyExists) { ...@@ -278,6 +283,54 @@ TEST_F(CustomLinksManagerImplTest, UpdateLinkWhenUrlAlreadyExists) {
EXPECT_EQ(initial_links, custom_links_->GetLinks()); EXPECT_EQ(initial_links, custom_links_->GetLinks());
} }
TEST_F(CustomLinksManagerImplTest, ReorderLink) {
NTPTilesVector initial_tiles = FillTestTiles(kTestCase3);
std::vector<Link> initial_links = FillTestLinks(kTestCase3);
std::vector<Link> links_after_reorder1(
{Link{GURL(kTestCase3[2].url), base::UTF8ToUTF16(kTestCase3[2].title),
true},
Link{GURL(kTestCase3[0].url), base::UTF8ToUTF16(kTestCase3[0].title),
true},
Link{GURL(kTestCase3[1].url), base::UTF8ToUTF16(kTestCase3[1].title),
true}});
std::vector<Link> links_after_reorder2(
{Link{GURL(kTestCase3[0].url), base::UTF8ToUTF16(kTestCase3[0].title),
true},
Link{GURL(kTestCase3[2].url), base::UTF8ToUTF16(kTestCase3[2].title),
true},
Link{GURL(kTestCase3[1].url), base::UTF8ToUTF16(kTestCase3[1].title),
true}});
// Initialize.
ASSERT_TRUE(custom_links_->Initialize(initial_tiles));
ASSERT_EQ(initial_links, custom_links_->GetLinks());
// Try to call reorder with the current index. This should fail and not modify
// the list.
EXPECT_FALSE(custom_links_->ReorderLink(GURL(kTestCase3[2].url), (size_t)2));
EXPECT_EQ(initial_links, custom_links_->GetLinks());
// Try to call reorder with an invalid index. This should fail and not modify
// the list.
EXPECT_FALSE(custom_links_->ReorderLink(GURL(kTestCase3[2].url), (size_t)-1));
EXPECT_EQ(initial_links, custom_links_->GetLinks());
EXPECT_FALSE(custom_links_->ReorderLink(GURL(kTestCase3[2].url),
initial_links.size()));
EXPECT_EQ(initial_links, custom_links_->GetLinks());
// Move the last link to the front.
EXPECT_TRUE(custom_links_->ReorderLink(GURL(kTestCase3[2].url), (size_t)0));
EXPECT_EQ(links_after_reorder1, custom_links_->GetLinks());
// Move the same link to the right.
EXPECT_TRUE(custom_links_->ReorderLink(GURL(kTestCase3[2].url), (size_t)1));
EXPECT_EQ(links_after_reorder2, custom_links_->GetLinks());
// Move the same link to the end.
EXPECT_TRUE(custom_links_->ReorderLink(GURL(kTestCase3[2].url), (size_t)2));
EXPECT_EQ(initial_links, custom_links_->GetLinks());
}
TEST_F(CustomLinksManagerImplTest, DeleteLink) { TEST_F(CustomLinksManagerImplTest, DeleteLink) {
NTPTilesVector initial_tiles; NTPTilesVector initial_tiles;
AddTile(&initial_tiles, kTestUrl, kTestTitle); AddTile(&initial_tiles, kTestUrl, kTestTitle);
......
...@@ -315,6 +315,22 @@ bool MostVisitedSites::UpdateCustomLink(const GURL& url, ...@@ -315,6 +315,22 @@ bool MostVisitedSites::UpdateCustomLink(const GURL& url,
return success; return success;
} }
bool MostVisitedSites::ReorderCustomLink(const GURL& url, size_t new_pos) {
if (!custom_links_ || !custom_links_enabled_)
return false;
// Initialize custom links if they have not been initialized yet.
InitializeCustomLinks();
bool success = custom_links_->ReorderLink(url, new_pos);
if (success) {
if (custom_links_action_count_ != -1)
custom_links_action_count_++;
BuildCurrentTiles();
}
return success;
}
bool MostVisitedSites::DeleteCustomLink(const GURL& url) { bool MostVisitedSites::DeleteCustomLink(const GURL& url) {
if (!custom_links_ || !custom_links_enabled_) if (!custom_links_ || !custom_links_enabled_)
return false; return false;
......
...@@ -184,6 +184,11 @@ class MostVisitedSites : public history::TopSitesObserver, ...@@ -184,6 +184,11 @@ class MostVisitedSites : public history::TopSitesObserver,
bool UpdateCustomLink(const GURL& url, bool UpdateCustomLink(const GURL& url,
const GURL& new_url, const GURL& new_url,
const base::string16& new_title); const base::string16& new_title);
// Moves the custom link specified by |url| to the index |new_pos|. If |url|
// does not exist, or |new_pos| is invalid, returns false and does nothing.
// Will initialize custom links if they have not been initialized yet. Custom
// links must be enabled.
bool ReorderCustomLink(const GURL& url, size_t new_pos);
// Deletes the custom link with the specified |url|. If |url| does not exist // Deletes the custom link with the specified |url|. If |url| does not exist
// in the custom link list, returns false and does nothing. Will initialize // in the custom link list, returns false and does nothing. Will initialize
// custom links if they have not been initialized yet. Custom links must be // custom links if they have not been initialized yet. Custom links must be
......
...@@ -276,6 +276,7 @@ class MockCustomLinksManager : public CustomLinksManager { ...@@ -276,6 +276,7 @@ class MockCustomLinksManager : public CustomLinksManager {
bool(const GURL& url, bool(const GURL& url,
const GURL& new_url, const GURL& new_url,
const base::string16& new_title)); const base::string16& new_title));
MOCK_METHOD2(ReorderLink, bool(const GURL& url, size_t new_pos));
MOCK_METHOD1(DeleteLink, bool(const GURL& url)); MOCK_METHOD1(DeleteLink, bool(const GURL& url));
MOCK_METHOD0(UndoAction, bool()); MOCK_METHOD0(UndoAction, bool());
MOCK_METHOD1(RegisterCallbackForOnChanged, MOCK_METHOD1(RegisterCallbackForOnChanged,
......
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