Commit 6f0540bc authored by Kei Nakashima's avatar Kei Nakashima Committed by Commit Bot

Fixed bugs in VectorBackedLinkedList::MoveTo

|MoveTo| didn't work correctly when moving the target node before itself/its next node.
Also added DCHECKs, which check if the target node is not anchor and if two given node is both in-use list.

Change-Id: I60442f1d706a072eaeca9bc83eaaee9d60b0c853
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2096316
Commit-Queue: Kei Nakashima <keinakashima@google.com>
Reviewed-by: default avatarYuki Shiino <yukishiino@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748599}
parent 8e716b5d
...@@ -432,14 +432,23 @@ typename VectorBackedLinkedList<T>::iterator VectorBackedLinkedList<T>::insert( ...@@ -432,14 +432,23 @@ typename VectorBackedLinkedList<T>::iterator VectorBackedLinkedList<T>::insert(
template <typename T> template <typename T>
void VectorBackedLinkedList<T>::MoveTo(const_iterator target, void VectorBackedLinkedList<T>::MoveTo(const_iterator target,
const_iterator new_position) { const_iterator new_position) {
DCHECK(target != end());
if (target == new_position)
return;
wtf_size_t target_index = target.GetIndex(); wtf_size_t target_index = target.GetIndex();
Node& target_node = nodes_[target_index]; Node& target_node = nodes_[target_index];
wtf_size_t new_position_index = new_position.GetIndex();
Node& new_position_node = nodes_[new_position_index];
wtf_size_t prev_index = new_position_node.prev_index_;
if (prev_index == target_index)
return;
Unlink(target_node); Unlink(target_node);
wtf_size_t new_position_index = new_position.GetIndex();
wtf_size_t prev_index = nodes_[new_position_index].prev_index_;
nodes_[prev_index].next_index_ = target_index; nodes_[prev_index].next_index_ = target_index;
nodes_[new_position_index].prev_index_ = target_index; new_position_node.prev_index_ = target_index;
target_node.prev_index_ = prev_index; target_node.prev_index_ = prev_index;
target_node.next_index_ = new_position_index; target_node.next_index_ = new_position_index;
} }
...@@ -456,6 +465,7 @@ typename VectorBackedLinkedList<T>::iterator VectorBackedLinkedList<T>::erase( ...@@ -456,6 +465,7 @@ typename VectorBackedLinkedList<T>::iterator VectorBackedLinkedList<T>::erase(
node.value_ = HashTraits<T>::EmptyValue(); node.value_ = HashTraits<T>::EmptyValue();
node.next_index_ = free_head_index_; node.next_index_ = free_head_index_;
node.prev_index_ = kNotFound;
free_head_index_ = position_index; free_head_index_ = position_index;
size_--; size_--;
......
...@@ -77,13 +77,16 @@ TEST(VectorBackedLinkedList, MoveTo) { ...@@ -77,13 +77,16 @@ TEST(VectorBackedLinkedList, MoveTo) {
List list; List list;
list.push_back(1); list.push_back(1);
list.MoveTo(list.begin(), list.end());
List::iterator it = list.begin();
EXPECT_EQ(*it, 1);
list.push_back(2); list.push_back(2);
list.push_back(3); list.push_back(3);
List::iterator target = list.begin(); List::iterator target = list.begin();
list.MoveTo(target, list.end()); // {2, 3, 1} list.MoveTo(target, list.end()); // {2, 3, 1}
List::iterator it = list.begin(); it = list.begin();
EXPECT_EQ(*it, 2); EXPECT_EQ(*it, 2);
++it; ++it;
EXPECT_EQ(*it, 3); EXPECT_EQ(*it, 3);
...@@ -108,6 +111,24 @@ TEST(VectorBackedLinkedList, MoveTo) { ...@@ -108,6 +111,24 @@ TEST(VectorBackedLinkedList, MoveTo) {
EXPECT_EQ(*it, 1); EXPECT_EQ(*it, 1);
++it; ++it;
EXPECT_EQ(*it, 2); EXPECT_EQ(*it, 2);
list.MoveTo(list.begin(), list.begin());
it = list.begin();
EXPECT_EQ(*it, 3);
++it;
EXPECT_EQ(*it, 1);
++it;
EXPECT_EQ(*it, 2);
target = list.begin();
List::iterator position = ++list.begin();
list.MoveTo(target, position);
it = list.begin();
EXPECT_EQ(*it, 3);
++it;
EXPECT_EQ(*it, 1);
++it;
EXPECT_EQ(*it, 2);
} }
TEST(VectorBackedLinkedList, Erase) { TEST(VectorBackedLinkedList, Erase) {
......
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