Commit 21d285cc authored by Nick Carter's avatar Nick Carter Committed by Commit Bot

Introduce TableModelObserver::OnItemsMoved() and enhance ListSelectionModel::Move()

In TableModelObserver, support a OnItemsMoved() operation which indicates
that a number of rows have been reordered in the model. In a subsequent CL,
the TaskManagerTableModel will emit these, with the goal being to
keep a tab properly selected during cross-process navigation.

In TableView and its unittest, handle the OnItemsMoved() operation, by
updating the ListSelectionModel as appropriate.

In ListSelectionModel, Add a |length| parameter to Move, and provide
an efficient implementation of this. Implement IncrementFrom() and
DecrementFrom() in terms of the Move() operation. Add unittests that
exercise Move().

Bug: 527455
Change-Id: I448bc18b2a3efbe6b2acb78d73ee350212bbf485
Reviewed-on: https://chromium-review.googlesource.com/661757
Commit-Queue: Nick Carter <nick@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502933}
parent d97b9461
......@@ -1350,7 +1350,7 @@ void TabStripModel::MoveWebContentsAtImpl(int index,
contents_data_.insert(contents_data_.begin() + to_position,
std::move(moved_data));
selection_model_.Move(index, to_position);
selection_model_.Move(index, to_position, 1);
if (!selection_model_.IsSelected(to_position) && select_after_move) {
// TODO(sky): why doesn't this code notify observers?
selection_model_.SetSelectedIndex(to_position);
......
......@@ -15,12 +15,15 @@ namespace ui {
// static
const int ListSelectionModel::kUnselectedIndex = -1;
static void IncrementFromImpl(int index, int* value) {
namespace {
void IncrementFromImpl(int index, int* value) {
if (*value >= index)
(*value)++;
}
static bool DecrementFromImpl(int index, int* value) {
// Returns true if |value| should be erased from its container.
bool DecrementFromImpl(int index, int* value) {
if (*value == index) {
*value = ListSelectionModel::kUnselectedIndex;
return true;
......@@ -30,6 +33,30 @@ static bool DecrementFromImpl(int index, int* value) {
return false;
}
void MoveToLowerIndexImpl(int old_start,
int new_start,
int length,
int* value) {
DCHECK_LE(new_start, old_start);
DCHECK_GE(length, 0);
// When a range of items moves to a lower index, the only affected indices
// are those in the interval [new_start, old_start + length).
if (new_start <= *value && *value < old_start + length) {
if (*value < old_start) {
// The items originally in the interval [new_start, old_start) see
// |length| many items inserted before them, so their indices increase.
*value += length;
} else {
// The items originally in the interval [old_start, old_start + length)
// are shifted downward by (old_start - new_start) many spots, so
// their indices decrease.
*value -= (old_start - new_start);
}
}
}
} // namespace
ListSelectionModel::ListSelectionModel()
: active_(kUnselectedIndex),
anchor_(kUnselectedIndex) {
......@@ -39,7 +66,7 @@ ListSelectionModel::~ListSelectionModel() {
}
void ListSelectionModel::IncrementFrom(int index) {
// Shift the selection to account for the newly inserted tab.
// Shift the selection to account for a newly inserted item at |index|.
for (SelectedIndices::iterator i = selected_indices_.begin();
i != selected_indices_.end(); ++i) {
IncrementFromImpl(index, &(*i));
......@@ -112,24 +139,57 @@ void ListSelectionModel::AddSelectionFromAnchorTo(int index) {
}
}
void ListSelectionModel::Move(int from, int to) {
DCHECK_NE(to, from);
bool was_anchor = from == anchor_;
bool was_active = from == active_;
bool was_selected = IsSelected(from);
if (to < from) {
IncrementFrom(to);
DecrementFrom(from + 1);
} else {
DecrementFrom(from);
IncrementFrom(to);
void ListSelectionModel::Move(int old_index, int new_index, int length) {
// |length| many items are moving from index |old_index| to index |new_index|.
DCHECK_NE(old_index, new_index);
DCHECK_GT(length, 0);
// Remap move-to-higher-index operations to the equivalent move-to-lower-index
// operation. As an example, the permutation "ABCDEFG" -> "CDEFABG" can be
// thought of either as shifting 'AB' higher by 4, or by shifting 'CDEF' lower
// by 2.
if (new_index > old_index) {
Move(old_index + length, old_index, new_index - old_index);
return;
}
// We know that |old_index| > |new_index|, so this is a move to a lower index.
// Start by transforming |anchor_| and |active_|.
MoveToLowerIndexImpl(old_index, new_index, length, &anchor_);
MoveToLowerIndexImpl(old_index, new_index, length, &active_);
// When a range of items moves to a lower index, the affected items are those
// in the interval [new_index, old_index + length). Search within
// |selected_indices_| for indices that fall into that range.
auto low = std::lower_bound(selected_indices_.begin(),
selected_indices_.end(), new_index);
auto high =
std::lower_bound(low, selected_indices_.end(), old_index + length);
// The items originally in the interval [new_index, old_index) will see
// |length| many items inserted before them, so their indices increase.
auto middle = std::lower_bound(low, high, old_index);
int pivot_value = new_index + length;
for (auto it = low; it != middle; ++it) {
(*it) += length;
DCHECK(pivot_value <= (*it) && (*it) < (old_index + length));
}
if (was_active)
active_ = to;
if (was_anchor)
anchor_ = to;
if (was_selected)
AddIndexToSelection(to);
// The items originally in the interval [old_index, old_index + length) are
// shifted downward by (old_index - new_index) many spots, so their indices
// decrease.
for (auto it = middle; it != high; ++it) {
(*it) -= (old_index - new_index);
DCHECK(new_index <= (*it) && (*it) < pivot_value);
}
// Reorder the ranges [low, middle), and [middle, high) so that the elements
// in [middle, high) appear first, followed by [low, middle). This suffices to
// restore the sort condition on |selected_indices_|, because each range is
// still sorted piecewise, and |pivot_value| is a lower bound for elements in
// [low, middle), and an upper bound for [middle, high).
std::rotate(low, middle, high);
DCHECK(std::is_sorted(selected_indices_.begin(), selected_indices_.end()));
}
void ListSelectionModel::Clear() {
......
......@@ -81,13 +81,15 @@ class UI_BASE_EXPORT ListSelectionModel {
// adds to the selection.
void AddSelectionFromAnchorTo(int index);
// Invoked when an item moves. |from| is the original index, and |to| the
// target index.
// Invoked when an item moves. |old_index| is the original index, |new_index|
// is the target index, and |length| is the number of items that are moving.
//
// NOTE: this matches the TabStripModel API. If moving to a greater index,
// |to| should be the index *after* removing |from|. For example, consider
// three tabs 'A B C', to move A to the end of the list, this should be
// invoked with '0, 2'.
void Move(int from, int to);
// |new_index| should be the index *after* removing the elements at the index
// range [old_index, old_index + length). For example, consider three tabs 'A
// B C', to move A to the end of the list, this should be invoked with '0, 2,
// 1'.
void Move(int old_index, int new_index, int length);
// Sets the anchor and active to kUnselectedIndex, and removes all the
// selected indices.
......
......@@ -142,8 +142,33 @@ TEST_F(ListSelectionModelTest, MoveToLeft) {
model.AddIndexToSelection(10);
model.set_anchor(4);
model.set_active(4);
model.Move(4, 0);
EXPECT_EQ("active=4 anchor=4 selection=0 4 10", StateAsString(model));
model.Move(4, 0, 1);
EXPECT_EQ("active=0 anchor=0 selection=0 1 10", StateAsString(model));
model.Move(25, 1, 5);
EXPECT_EQ("active=0 anchor=0 selection=0 6 15", StateAsString(model));
model.Move(5, 1, 2);
EXPECT_EQ("active=0 anchor=0 selection=0 2 15", StateAsString(model));
model.Move(2, 0, 4);
EXPECT_EQ("active=4 anchor=4 selection=0 4 15", StateAsString(model));
model.Move(1, 2, 1);
EXPECT_EQ("active=4 anchor=4 selection=0 4 15", StateAsString(model));
model.Move(100, 5, 100000);
EXPECT_EQ("active=4 anchor=4 selection=0 4 100015", StateAsString(model));
model.Move(4, 0, 200000);
EXPECT_EQ("active=0 anchor=0 selection=0 100011 200000",
StateAsString(model));
model.Move(100011, 1, 1);
EXPECT_EQ("active=0 anchor=0 selection=0 1 200000", StateAsString(model));
model.Move(200000, 1, 1);
EXPECT_EQ("active=0 anchor=0 selection=0 1 2", StateAsString(model));
model.AddIndexToSelection(4);
model.AddIndexToSelection(3);
EXPECT_EQ("active=0 anchor=0 selection=0 1 2 3 4", StateAsString(model));
model.Move(3, 0, 3);
EXPECT_EQ("active=3 anchor=3 selection=0 1 3 4 5", StateAsString(model));
model.Move(3, 1, 10);
EXPECT_EQ("active=1 anchor=1 selection=0 1 2 3 11", StateAsString(model));
}
TEST_F(ListSelectionModelTest, MoveToRight) {
......@@ -153,8 +178,37 @@ TEST_F(ListSelectionModelTest, MoveToRight) {
model.AddIndexToSelection(10);
model.set_anchor(0);
model.set_active(0);
model.Move(0, 3);
EXPECT_EQ("active=0 anchor=0 selection=0 4 10", StateAsString(model));
model.Move(0, 3, 1);
EXPECT_EQ("active=3 anchor=3 selection=3 4 10", StateAsString(model));
model.Move(2, 4, 4);
EXPECT_EQ("active=5 anchor=5 selection=5 6 10", StateAsString(model));
model.Move(5, 6, 1);
EXPECT_EQ("active=6 anchor=6 selection=5 6 10", StateAsString(model));
model.Move(5, 6, 2);
EXPECT_EQ("active=7 anchor=7 selection=6 7 10", StateAsString(model));
model.Move(1, 2, 3);
EXPECT_EQ("active=7 anchor=7 selection=6 7 10", StateAsString(model));
model.Move(1, 6, 4);
EXPECT_EQ("active=3 anchor=3 selection=2 3 10", StateAsString(model));
model.Move(0, 7000000, 3);
EXPECT_EQ("active=0 anchor=0 selection=0 7 7000002", StateAsString(model));
model.Move(10, 30, 7000000);
EXPECT_EQ("active=0 anchor=0 selection=0 7 7000022", StateAsString(model));
model.AddIndexToSelection(10);
model.AddIndexToSelection(20);
model.AddIndexToSelection(21);
EXPECT_EQ("active=0 anchor=0 selection=0 7 10 20 21 7000022",
StateAsString(model));
model.Move(22, 9000000, 7000000);
EXPECT_EQ("active=0 anchor=0 selection=0 7 10 20 21 22",
StateAsString(model));
model.Move(0, 10, 10);
EXPECT_EQ("active=10 anchor=10 selection=0 10 17 20 21 22",
StateAsString(model));
model.Move(1, 10, 10);
EXPECT_EQ("active=19 anchor=19 selection=0 7 19 20 21 22",
StateAsString(model));
}
TEST_F(ListSelectionModelTest, Copy) {
......
......@@ -19,12 +19,15 @@ class UI_BASE_EXPORT TableModelObserver {
// Invoked when a range of items has changed.
virtual void OnItemsChanged(int start, int length) = 0;
// Invoked when new items are added.
// Invoked when new items have been added.
virtual void OnItemsAdded(int start, int length) = 0;
// Invoked when a range of items has been removed.
virtual void OnItemsRemoved(int start, int length) = 0;
// Invoked when a range of items has been moved to a different position.
virtual void OnItemsMoved(int old_start, int length, int new_start) {}
protected:
virtual ~TableModelObserver() {}
};
......
......@@ -490,6 +490,11 @@ void TableView::OnItemsAdded(int start, int length) {
NumRowsChanged();
}
void TableView::OnItemsMoved(int old_start, int length, int new_start) {
selection_model_.Move(old_start, new_start, length);
SortItemsAndUpdateMapping();
}
void TableView::OnItemsRemoved(int start, int length) {
// Determine the currently selected index in terms of the view. We inline the
// implementation here since ViewToModel() has DCHECKs that fail since the
......
......@@ -185,6 +185,7 @@ class VIEWS_EXPORT TableView
void OnItemsChanged(int start, int length) override;
void OnItemsAdded(int start, int length) override;
void OnItemsRemoved(int start, int length) override;
void OnItemsMoved(int old_start, int length, int new_start) override;
protected:
// View overrides:
......
......@@ -84,6 +84,9 @@ class TestTableModel2 : public ui::TableModel {
// Changes the values of the row at |row|.
void ChangeRow(int row, int c1_value, int c2_value);
// Reorders rows in the model.
void MoveRows(int row_from, int length, int row_to);
// ui::TableModel:
int RowCount() override;
base::string16 GetText(int row, int column_id) override;
......@@ -130,6 +133,21 @@ void TestTableModel2::ChangeRow(int row, int c1_value, int c2_value) {
observer_->OnItemsChanged(row, 1);
}
void TestTableModel2::MoveRows(int row_from, int length, int row_to) {
DCHECK_GT(length, 0);
DCHECK_GE(row_from, 0);
DCHECK_LE(row_from + length, static_cast<int>(rows_.size()));
DCHECK_GE(row_to, 0);
DCHECK_LE(row_to + length, static_cast<int>(rows_.size()));
auto old_start = rows_.begin() + row_from;
std::vector<std::vector<int>> temp(old_start, old_start + length);
rows_.erase(old_start, old_start + length);
rows_.insert(rows_.begin() + row_to, temp.begin(), temp.end());
if (observer_)
observer_->OnItemsMoved(row_from, length, row_to);
}
int TestTableModel2::RowCount() {
return static_cast<int>(rows_.size());
}
......@@ -684,6 +702,58 @@ TEST_F(TableViewTest, Selection) {
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=3 selection=3", SelectionStateAsString());
// Swap the first two rows. This shouldn't affect selection.
model_->MoveRows(0, 1, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=3 selection=3", SelectionStateAsString());
// Move the first row to after the selection. This will change the selection
// state.
model_->MoveRows(0, 1, 3);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=2 anchor=2 selection=2", SelectionStateAsString());
// Move the first two rows to be after the selection. This will change the
// selection state.
model_->MoveRows(0, 2, 2);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=0 anchor=0 selection=0", SelectionStateAsString());
// Move some rows after the selection.
model_->MoveRows(2, 2, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=0 anchor=0 selection=0", SelectionStateAsString());
// Move the selection itself.
model_->MoveRows(0, 1, 3);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=3 selection=3", SelectionStateAsString());
// Move-left a range that ends at the selection
model_->MoveRows(2, 2, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=2 anchor=2 selection=2", SelectionStateAsString());
// Move-right a range that ends at the selection
model_->MoveRows(1, 2, 2);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=3 selection=3", SelectionStateAsString());
// Add a row at the end.
model_->AddRow(4, 7, 9);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=3 selection=3", SelectionStateAsString());
// Move-left a range that includes the selection.
model_->MoveRows(2, 3, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=2 anchor=2 selection=2", SelectionStateAsString());
// Move-right a range that includes the selection.
model_->MoveRows(0, 4, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=3 selection=3", SelectionStateAsString());
table_->set_observer(NULL);
}
......@@ -1025,6 +1095,119 @@ TEST_F(TableViewTest, MultiselectionWithSort) {
ClickOnRow(0, ui::EF_SHIFT_DOWN);
EXPECT_EQ(1, observer.GetChangedCountAndClear());
EXPECT_EQ("active=2 anchor=4 selection=2 3 4", SelectionStateAsString());
}
TEST_F(TableViewTest, MoveRowsWithMultipleSelection) {
model_->AddRow(3, 77, 0);
// Hide column 1.
table_->SetColumnVisibility(1, false);
TableViewObserverImpl observer;
table_->set_observer(&observer);
// Select three rows.
ClickOnRow(2, 0);
ClickOnRow(4, ui::EF_SHIFT_DOWN);
EXPECT_EQ(2, observer.GetChangedCountAndClear());
EXPECT_EQ("active=4 anchor=2 selection=2 3 4", SelectionStateAsString());
EXPECT_EQ("[0], [1], [2], [77], [3]", GetRowsInViewOrderAsString(table_));
// Move the unselected rows to the middle of the current selection. None of
// the move operations should affect the view order.
model_->MoveRows(0, 2, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=4 anchor=0 selection=0 3 4", SelectionStateAsString());
EXPECT_EQ("[2], [0], [1], [77], [3]", GetRowsInViewOrderAsString(table_));
// Move the unselected rows to the end of the current selection.
model_->MoveRows(1, 2, 3);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=2 anchor=0 selection=0 1 2", SelectionStateAsString());
EXPECT_EQ("[2], [77], [3], [0], [1]", GetRowsInViewOrderAsString(table_));
// Move the unselected rows back to the middle of the selection.
model_->MoveRows(3, 2, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=4 anchor=0 selection=0 3 4", SelectionStateAsString());
EXPECT_EQ("[2], [0], [1], [77], [3]", GetRowsInViewOrderAsString(table_));
// Swap the unselected rows.
model_->MoveRows(1, 1, 2);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=4 anchor=0 selection=0 3 4", SelectionStateAsString());
EXPECT_EQ("[2], [1], [0], [77], [3]", GetRowsInViewOrderAsString(table_));
// Move the second unselected row to be between two selected rows.
model_->MoveRows(2, 1, 3);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=4 anchor=0 selection=0 2 4", SelectionStateAsString());
EXPECT_EQ("[2], [1], [77], [0], [3]", GetRowsInViewOrderAsString(table_));
// Move the three middle rows to the beginning, including one selected row.
model_->MoveRows(1, 3, 0);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=4 anchor=3 selection=1 3 4", SelectionStateAsString());
EXPECT_EQ("[1], [77], [0], [2], [3]", GetRowsInViewOrderAsString(table_));
table_->set_observer(NULL);
}
TEST_F(TableViewTest, MoveRowsWithMultipleSelectionAndSort) {
model_->AddRow(3, 77, 0);
// Sort ascending by column 0, and hide column 1. The view order should not
// change during this test.
table_->ToggleSortOrder(0);
table_->SetColumnVisibility(1, false);
const char* kViewOrder = "[0], [1], [2], [3], [77]";
EXPECT_EQ(kViewOrder, GetRowsInViewOrderAsString(table_));
TableViewObserverImpl observer;
table_->set_observer(&observer);
// Select three rows.
ClickOnRow(2, 0);
ClickOnRow(4, ui::EF_SHIFT_DOWN);
EXPECT_EQ(2, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=2 selection=2 3 4", SelectionStateAsString());
// Move the unselected rows to the middle of the current selection. None of
// the move operations should affect the view order.
model_->MoveRows(0, 2, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=0 selection=0 3 4", SelectionStateAsString());
EXPECT_EQ(kViewOrder, GetRowsInViewOrderAsString(table_));
// Move the unselected rows to the end of the current selection.
model_->MoveRows(1, 2, 3);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=1 anchor=0 selection=0 1 2", SelectionStateAsString());
EXPECT_EQ(kViewOrder, GetRowsInViewOrderAsString(table_));
// Move the unselected rows back to the middle of the selection.
model_->MoveRows(3, 2, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=0 selection=0 3 4", SelectionStateAsString());
EXPECT_EQ(kViewOrder, GetRowsInViewOrderAsString(table_));
// Swap the unselected rows.
model_->MoveRows(1, 1, 2);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=0 selection=0 3 4", SelectionStateAsString());
EXPECT_EQ(kViewOrder, GetRowsInViewOrderAsString(table_));
// Swap the unselected rows again.
model_->MoveRows(2, 1, 1);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=0 selection=0 3 4", SelectionStateAsString());
EXPECT_EQ(kViewOrder, GetRowsInViewOrderAsString(table_));
// Move the unselected rows back to the beginning.
model_->MoveRows(1, 2, 0);
EXPECT_EQ(0, observer.GetChangedCountAndClear());
EXPECT_EQ("active=3 anchor=2 selection=2 3 4", SelectionStateAsString());
EXPECT_EQ(kViewOrder, GetRowsInViewOrderAsString(table_));
table_->set_observer(NULL);
}
......
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