Skip to content

GitLab

T
tangled
Commit b2ab3b4f authored by tfarina's avatar tfarina Committed by Commit bot
Browse files
Options

bookmarks: BookmarkNodeData's size() cleanups. 

* Fix the implementation of has_single_url() method.
It is incorrect to check is_valid() because the elements
vector can contain more than 1 item. And thus the
is_valid() check does not make sense, since it just check
if the vector is not empty, not that it just contains one element.

* Use is_valid() method when possible.
* Use has_single_url() method when it makes sense.
* Use data.size() to avoid a temp variable.

BUG=None
TEST=chrome still links and works as before, existing unit tests in components_unittests.
R=sky@chromium.org

Review URL: https://codereview.chromium.org/870293002

Cr-Commit-Position: refs/heads/master@{#313702}
parent 3380c8d1
Show whitespace changes
Inline Side-by-side
Showing with 22 additions and 23 deletions
chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc
View file @ b2ab3b4f
......@@ -163,9 +163,8 @@ CreateApiBookmarkNodeData(Profile* profile, const BookmarkNodeData& data) {
}
} else {
// We do not have a node IDs when the data comes from a different profile.
std::vector<BookmarkNodeData::Element> elements = data.elements;
for (size_t i = 0; i < elements.size(); ++i)
node_data->elements.push_back(CreateApiNodeDataElement(elements[i]));
for (size_t i = 0; i < data.size(); ++i)
node_data->elements.push_back(CreateApiNodeDataElement(data.elements[i]));
}
return node_data.Pass();
}
......@@ -310,7 +309,7 @@ void BookmarkManagerPrivateDragEventRouter::DispatchEvent(
void BookmarkManagerPrivateDragEventRouter::OnDragEnter(
const BookmarkNodeData& data) {
if (data.size() == 0)
if (!data.is_valid())
return;
DispatchEvent(bookmark_manager_private::OnDragEnter::kEventName,
bookmark_manager_private::OnDragEnter::Create(
......@@ -325,7 +324,7 @@ void BookmarkManagerPrivateDragEventRouter::OnDragOver(
void BookmarkManagerPrivateDragEventRouter::OnDragLeave(
const BookmarkNodeData& data) {
if (data.size() == 0)
if (!data.is_valid())
return;
DispatchEvent(bookmark_manager_private::OnDragLeave::kEventName,
bookmark_manager_private::OnDragLeave::Create(
......@@ -334,7 +333,7 @@ void BookmarkManagerPrivateDragEventRouter::OnDragLeave(
void BookmarkManagerPrivateDragEventRouter::OnDrop(
const BookmarkNodeData& data) {
if (data.size() == 0)
if (!data.is_valid())
return;
DispatchEvent(bookmark_manager_private::OnDrop::kEventName,
bookmark_manager_private::OnDrop::Create(
......
chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc
View file @ b2ab3b4f
......@@ -182,7 +182,7 @@ bool BookmarkMenuDelegate::CanDrop(MenuItemView* menu,
// Only accept drops of 1 node, which is the case for all data dragged from
// bookmark bar and menus.
if (!drop_data_.Read(data) || drop_data_.elements.size() != 1 ||
if (!drop_data_.Read(data) || drop_data_.size() != 1 ||
!profile_->GetPrefs()->GetBoolean(
bookmarks::prefs::kEditBookmarksEnabled))
return false;
......
components/bookmarks/browser/bookmark_node_data.cc
View file @ b2ab3b4f
......@@ -160,7 +160,7 @@ void BookmarkNodeData::WriteToClipboard(ui::ClipboardType clipboard_type) {
// If there is only one element and it is a URL, write the URL to the
// clipboard.
if (elements.size() == 1 && elements[0].is_url) {
if (has_single_url()) {
const base::string16& title = elements[0].title;
const std::string url = elements[0].url.spec();
......@@ -180,7 +180,7 @@ void BookmarkNodeData::WriteToClipboard(ui::ClipboardType clipboard_type) {
// We have either more than one URL, a folder, or a combination of URLs
// and folders.
base::string16 text;
for (size_t i = 0; i < elements.size(); i++) {
for (size_t i = 0; i < size(); i++) {
text += i == 0 ? base::ASCIIToUTF16("") : base::ASCIIToUTF16("\n");
if (!elements[i].is_url) {
// Then it's a folder. Only copy the name of the folder.
......@@ -234,9 +234,9 @@ bool BookmarkNodeData::ReadFromClipboard(ui::ClipboardType type) {
void BookmarkNodeData::WriteToPickle(const base::FilePath& profile_path,
Pickle* pickle) const {
profile_path.WriteToPickle(pickle);
pickle->WriteSizeT(elements.size());
pickle->WriteSizeT(size());
for (size_t i = 0; i < elements.size(); ++i)
for (size_t i = 0; i < size(); ++i)
elements[i].WriteToPickle(pickle);
}
......@@ -266,7 +266,7 @@ std::vector<const BookmarkNode*> BookmarkNodeData::GetNodes(
if (!IsFromProfilePath(profile_path))
return nodes;
for (size_t i = 0; i < elements.size(); ++i) {
for (size_t i = 0; i < size(); ++i) {
const BookmarkNode* node = GetBookmarkNodeByID(model, elements[i].id_);
if (!node) {
nodes.clear();
......
components/bookmarks/browser/bookmark_node_data.h
View file @ b2ab3b4f
......@@ -150,7 +150,7 @@ struct BookmarkNodeData {
bool is_valid() const { return !elements.empty(); }
// Returns true if there is a single url.
bool has_single_url() const { return is_valid() && elements[0].is_url; }
bool has_single_url() const { return size() == 1 && elements[0].is_url; }
// Number of elements.
size_t size() const { return elements.size(); }
......
components/bookmarks/browser/bookmark_node_data_unittest.cc
View file @ b2ab3b4f
......@@ -93,7 +93,7 @@ TEST_F(BookmarkNodeDataTest, JustURL) {
BookmarkNodeData drag_data;
EXPECT_TRUE(drag_data.Read(ui::OSExchangeData(CloneProvider(data))));
EXPECT_TRUE(drag_data.is_valid());
ASSERT_EQ(1u, drag_data.elements.size());
ASSERT_EQ(1u, drag_data.size());
EXPECT_TRUE(drag_data.elements[0].is_url);
EXPECT_EQ(url, drag_data.elements[0].url);
EXPECT_EQ(title, drag_data.elements[0].title);
......@@ -110,7 +110,7 @@ TEST_F(BookmarkNodeDataTest, URL) {
const BookmarkNode* node = model()->AddURL(root, 0, title, url);
BookmarkNodeData drag_data(node);
EXPECT_TRUE(drag_data.is_valid());
ASSERT_EQ(1u, drag_data.elements.size());
ASSERT_EQ(1u, drag_data.size());
EXPECT_TRUE(drag_data.elements[0].is_url);
EXPECT_EQ(url, drag_data.elements[0].url);
EXPECT_EQ(title, drag_data.elements[0].title);
......@@ -125,7 +125,7 @@ TEST_F(BookmarkNodeDataTest, URL) {
BookmarkNodeData read_data;
EXPECT_TRUE(read_data.Read(data2));
EXPECT_TRUE(read_data.is_valid());
ASSERT_EQ(1u, read_data.elements.size());
ASSERT_EQ(1u, read_data.size());
EXPECT_TRUE(read_data.elements[0].is_url);
EXPECT_EQ(url, read_data.elements[0].url);
EXPECT_EQ(title, read_data.elements[0].title);
......@@ -157,7 +157,7 @@ TEST_F(BookmarkNodeDataTest, Folder) {
BookmarkNodeData drag_data(g12);
EXPECT_TRUE(drag_data.is_valid());
ASSERT_EQ(1u, drag_data.elements.size());
ASSERT_EQ(1u, drag_data.size());
EXPECT_EQ(g12->GetTitle(), drag_data.elements[0].title);
EXPECT_FALSE(drag_data.elements[0].is_url);
EXPECT_EQ(g12->date_added(), drag_data.elements[0].date_added);
......@@ -172,7 +172,7 @@ TEST_F(BookmarkNodeDataTest, Folder) {
BookmarkNodeData read_data;
EXPECT_TRUE(read_data.Read(data2));
EXPECT_TRUE(read_data.is_valid());
ASSERT_EQ(1u, read_data.elements.size());
ASSERT_EQ(1u, read_data.size());
EXPECT_EQ(g12->GetTitle(), read_data.elements[0].title);
EXPECT_FALSE(read_data.elements[0].is_url);
EXPECT_TRUE(read_data.elements[0].date_added.is_null());
......@@ -208,7 +208,7 @@ TEST_F(BookmarkNodeDataTest, FolderWithChild) {
ui::OSExchangeData data2(CloneProvider(data));
BookmarkNodeData read_data;
EXPECT_TRUE(read_data.Read(data2));
ASSERT_EQ(1u, read_data.elements.size());
ASSERT_EQ(1u, read_data.size());
ASSERT_EQ(1u, read_data.elements[0].children.size());
const BookmarkNodeData::Element& read_child =
read_data.elements[0].children[0];
......@@ -249,7 +249,7 @@ TEST_F(BookmarkNodeDataTest, MultipleNodes) {
BookmarkNodeData read_data;
EXPECT_TRUE(read_data.Read(data2));
EXPECT_TRUE(read_data.is_valid());
ASSERT_EQ(2u, read_data.elements.size());
ASSERT_EQ(2u, read_data.size());
ASSERT_EQ(1u, read_data.elements[0].children.size());
EXPECT_TRUE(read_data.elements[0].date_added.is_null());
EXPECT_TRUE(read_data.elements[0].date_folder_modified.is_null());
......@@ -394,7 +394,7 @@ TEST_F(BookmarkNodeDataTest, MetaInfo) {
BookmarkNodeData read_data;
EXPECT_TRUE(read_data.Read(data2));
EXPECT_TRUE(read_data.is_valid());
ASSERT_EQ(1u, read_data.elements.size());
ASSERT_EQ(1u, read_data.size());
// Verify that the read data contains the same meta info.
BookmarkNode::MetaInfoMap meta_info_map = read_data.elements[0].meta_info_map;
......
components/bookmarks/browser/bookmark_node_data_views.cc
View file @ b2ab3b4f
......@@ -34,7 +34,7 @@ void BookmarkNodeData::Write(const base::FilePath& profile_path,
// If there is only one element and it is a URL, write the URL to the
// clipboard.
if (elements.size() == 1 && elements[0].is_url) {
if (has_single_url()) {
if (elements[0].url.SchemeIs(kJavaScriptScheme)) {
data->SetString(base::UTF8ToUTF16(elements[0].url.spec()));
} else {
......
components/bookmarks/browser/bookmark_utils.cc
View file @ b2ab3b4f
......@@ -258,7 +258,7 @@ void PasteFromClipboard(BookmarkModel* model,
index = parent->child_count();
ScopedGroupBookmarkActions group_paste(model);
if (bookmark_data.elements.size() == 1 &&
if (bookmark_data.size() == 1 &&
model->IsBookmarked(bookmark_data.elements[0].url)) {
MakeTitleUnique(model,
parent,
......
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