Implement UI-side syncing of apps grid gap

Changes:
1. Load item list with "page break" items into paged view structure
   which will be displayed to the user.
2. Add "page break" item based on the changed paged view structure after
   user operations (Add, Remove, Move an item).
3. Since model index (item view's index in view model) is no longer the
   same as item index (item's index in item list), separate them.

BUG=848917

Change-Id: I5d4c750ab87045a0e4930e549a066f24a1c1cf66
Reviewed-on: https://chromium-review.googlesource.com/1083492Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564973}

ui/app_list/paged_view_structure.cc

@@ -4,6 +4,7 @@
 
 #include "ui/app_list/paged_view_structure.h"
 
+#include "ash/app_list/model/app_list_item.h"
 #include "ui/app_list/views/app_list_item_view.h"
 #include "ui/app_list/views/apps_grid_view.h"
 #include "ui/views/view_model.h"
@@ -19,21 +20,63 @@ PagedViewStructure::PagedViewStructure(const PagedViewStructure& other) =
 PagedViewStructure::~PagedViewStructure() = default;
 
 void PagedViewStructure::LoadFromMetadata() {
-  // TODO(weidongg): Change the fake loading here and implement loading view
-  // structure from position and page position in metadata.
   auto* view_model = apps_grid_view_->view_model();
+  const auto* item_list = apps_grid_view_->item_list_;
+  int model_index = 0;
+
   pages_.clear();
   pages_.emplace_back();
-  auto& page = pages_[0];
-  for (int i = 0; i < view_model->view_size(); ++i) {
-    page.emplace_back(view_model->view_at(i));
+  for (size_t i = 0; i < item_list->item_count(); ++i) {
+    const auto* item = item_list->item_at(i);
+    auto* current_page = &pages_.back();
+    if (item->is_page_break()) {
+      // Create a new page if a "page break" item is detected and current page
+      // is not empty. Otherwise, ignore the "page break" item.
+      if (!current_page->empty())
+        pages_.emplace_back();
+      continue;
+    }
+
+    // Create a new page if the current page is full.
+    const size_t current_page_max_items =
+        apps_grid_view_->TilesPerPage(pages_.size() - 1);
+    if (current_page->size() == current_page_max_items) {
+      pages_.emplace_back();
+      current_page = &pages_.back();
+    }
+
+    current_page->emplace_back(view_model->view_at(model_index++));
   }
-  Sanitize();
+
+  // Remove trailing empty page if exist.
+  if (pages_.back().empty())
+    pages_.erase(pages_.end() - 1);
 }
 
 void PagedViewStructure::SaveToMetadata() {
-  // TODO(weidongg): Implement saving page position change of each item into
-  // metadata.
+  auto* item_list = apps_grid_view_->item_list_;
+  size_t item_index = 0;
+
+  for (const auto& page : pages_) {
+    // Skip all "page break" items before current page and after previous page.
+    while (item_index < item_list->item_count() &&
+           item_list->item_at(item_index)->is_page_break()) {
+      ++item_index;
+    }
+    item_index += page.size();
+    if (item_index < item_list->item_count() &&
+        !item_list->item_at(item_index)->is_page_break()) {
+      // There's no "page break" item at the end of current page, so add one to
+      // push overflowing items to next page.
+      apps_grid_view_->model_->AddPageBreakItemAfter(
+          item_list->item_at(item_index - 1));
+    }
+  }
+
+  // Note that we do not remove redundant "page break" items here because the
+  // item list we can access here may not be complete (e.g. Devices that do not
+  // support ARC++ or Crostini apps filter out those items.). We leave this
+  // operation to AppListSyncableService which has complete item list.
 }
 
 bool PagedViewStructure::Sanitize() {
@@ -209,6 +252,51 @@ int PagedViewStructure::GetTargetModelIndexForMove(
   return target_model_index;
 }
 
+int PagedViewStructure::GetTargetItemIndexForMove(
+    AppListItemView* moved_view,
+    const GridIndex& index) const {
+  GridIndex current_index(0, 0);
+  size_t current_item_index = 0;
+  size_t offset = 0;
+  const auto* item_list = apps_grid_view_->item_list_;
+
+  // Skip the leading "page break" items.
+  while (current_item_index < item_list->item_count() &&
+         item_list->item_at(current_item_index)->is_page_break()) {
+    ++current_item_index;
+  }
+
+  while (current_item_index < item_list->item_count()) {
+    while (current_item_index < item_list->item_count() &&
+           !item_list->item_at(current_item_index)->is_page_break() &&
+           current_index != index) {
+      if (moved_view->item() == item_list->item_at(current_item_index) &&
+          current_index.page < index.page) {
+        // If the item view is moved to a following page, we need to skip the
+        // item view. If the view is moved to the same page, do not skip the
+        // item view because the following item views will fill the gap left
+        // after dragging complete.
+        offset = 1;
+      }
+      ++current_index.slot;
+      ++current_item_index;
+    }
+
+    if (current_index == index)
+      return current_item_index - offset;
+
+    // Skip the "page break" items at the end of the page.
+    while (current_item_index < item_list->item_count() &&
+           item_list->item_at(current_item_index)->is_page_break()) {
+      ++current_item_index;
+    }
+    ++current_index.page;
+    current_index.slot = 0;
+  }
+  DCHECK(current_index == index);
+  return current_item_index - offset;
+}
+
 bool PagedViewStructure::IsValidReorderTargetIndex(
     const GridIndex& index) const {
   if (apps_grid_view_->IsValidIndex(index))

ui/app_list/paged_view_structure.h

@@ -65,6 +65,11 @@ class APP_LIST_EXPORT PagedViewStructure {
   int GetTargetModelIndexForMove(AppListItemView* moved_view,
                                  const GridIndex& index) const;
 
+  // Returns the target item index if moving the item view to specified target
+  // visual index.
+  int GetTargetItemIndexForMove(AppListItemView* moved_view,
+                                const GridIndex& index) const;
+
   // Returns true if the visual index is valid position to which an item view
   // can be moved.
   bool IsValidReorderTargetIndex(const GridIndex& index) const;

ui/app_list/test/app_list_test_model.cc

@@ -33,16 +33,13 @@ const char AppListTestModel::kItemType[] = "TestItem";
 
 // AppListTestModel::AppListTestItem
 
-AppListTestModel::AppListTestItem::AppListTestItem(
-    const std::string& id,
-    AppListTestModel* model)
-    : AppListItem(id),
-      model_(model) {
+AppListTestModel::AppListTestItem::AppListTestItem(const std::string& id,
+                                                   AppListTestModel* model)
+    : AppListItem(id), model_(model) {
   SetIcon(CreateImageSkia(kGridIconDimension, kGridIconDimension));
 }
 
-AppListTestModel::AppListTestItem::~AppListTestItem() {
-}
+AppListTestModel::AppListTestItem::~AppListTestItem() = default;
 
 void AppListTestModel::AppListTestItem::Activate(int event_flags) {
   model_->ItemActivated(this);
@@ -71,9 +68,7 @@ void AppListTestModel::AppListTestItem::SetPosition(
 // AppListTestModel
 
 AppListTestModel::AppListTestModel()
-    : activate_count_(0),
-      last_activated_(NULL) {
-}
+    : activate_count_(0), last_activated_(nullptr) {}
 
 AppListItem* AppListTestModel::AddItem(AppListItem* item) {
   return AppListModel::AddItem(base::WrapUnique(item));
@@ -89,7 +84,6 @@ void AppListTestModel::MoveItemToFolder(AppListItem* item,
   AppListModel::MoveItemToFolder(item, folder_id);
 }
 
-
 std::string AppListTestModel::GetItemName(int id) {
   return base::StringPrintf("Item %d", id);
 }
@@ -138,7 +132,8 @@ std::string AppListTestModel::GetModelContent() {
   for (size_t i = 0; i < top_level_item_list()->item_count(); ++i) {
     if (i > 0)
       content += ',';
-    content += top_level_item_list()->item_at(i)->id();
+    AppListItem* item = top_level_item_list()->item_at(i);
+    content += item->is_page_break() ? "PageBreakItem" : item->id();
   }
   return content;
 }

ui/app_list/views/apps_grid_view.h

@@ -556,6 +556,11 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
   int GetTargetModelIndexForMove(AppListItemView* moved_view,
                                  const GridIndex& index) const;
 
+  // Returns the target item index if moving the item view to specified target
+  // visual index.
+  size_t GetTargetItemIndexForMove(AppListItemView* moved_view,
+                                   const GridIndex& index) const;
+
   // Returns true if an item view exists in the visual index.
   bool IsValidIndex(const GridIndex& index) const;
 
@@ -569,6 +574,14 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
   // Calculates the item views' bounds when apps grid gap is enabled.
   void CalculateIdealBoundsWithGridGap();
 
+  // Returns model index of the item view of the specified item.
+  int GetModelIndexOfItem(const AppListItem* item);
+
+  // Returns the target model index based on item index. (Item index is the
+  // index of an item in item list.) This should be used when the item is
+  // updated in item list but its item view has not been updated in view model.
+  int GetTargetModelIndexFromItemIndex(size_t item_index);
+
   AppListModel* model_ = nullptr;         // Owned by AppListView.
   AppListItemList* item_list_ = nullptr;  // Not owned.
 

ui/app_list/views/apps_grid_view_unittest.cc

@@ -298,7 +298,7 @@ class AppsGridViewTest : public views::ViewsTestBase,
       nullptr;                                    // Owned by |apps_grid_view_|.
   ExpandArrowView* expand_arrow_view_ = nullptr;  // Owned by |apps_grid_view_|.
   std::unique_ptr<AppListTestViewDelegate> delegate_;
-  AppListTestModel* model_ = nullptr;  // Owned by |delegate_|.
+  AppListTestModel* model_ = nullptr;    // Owned by |delegate_|.
   SearchModel* search_model_ = nullptr;  // Owned by |delegate_|.
   std::unique_ptr<AppsGridViewTestApi> test_api_;
   bool is_rtl_ = false;
@@ -1191,6 +1191,7 @@ TEST_P(AppsGridGapTest, MoveAnItemToNewEmptyPage) {
   EXPECT_EQ(view_model->view_at(1),
             test_api_->GetViewAtVisualIndex(0 /* page */, 1 /* slot */));
   EXPECT_EQ("Item 1", view_model->view_at(1)->item()->id());
+  EXPECT_EQ(std::string("Item 0,Item 1"), model_->GetModelContent());
 
   // Drag the first item to the page bottom.
   gfx::Point from = GetItemRectOnCurrentPageAt(0, 0).CenterPoint();
@@ -1210,6 +1211,8 @@ TEST_P(AppsGridGapTest, MoveAnItemToNewEmptyPage) {
   EXPECT_EQ(view_model->view_at(1),
             test_api_->GetViewAtVisualIndex(1 /* page */, 0 /* slot */));
   EXPECT_EQ("Item 0", view_model->view_at(1)->item()->id());
+  EXPECT_EQ(std::string("Item 1,PageBreakItem,Item 0"),
+            model_->GetModelContent());
 }
 
 TEST_P(AppsGridGapTest, MoveLastItemToCreateFolderInNextPage) {
@@ -1229,6 +1232,7 @@ TEST_P(AppsGridGapTest, MoveLastItemToCreateFolderInNextPage) {
   EXPECT_EQ(view_model->view_at(1),
             test_api_->GetViewAtVisualIndex(0 /* page */, 1 /* slot */));
   EXPECT_EQ("Item 1", view_model->view_at(1)->item()->id());
+  EXPECT_EQ(std::string("Item 0,Item 1"), model_->GetModelContent());
 
   // Drag the first item to next page and drag the second item to overlap with
   // the first item.
@@ -1247,7 +1251,13 @@ TEST_P(AppsGridGapTest, MoveLastItemToCreateFolderInNextPage) {
   EXPECT_EQ(1, view_model->view_size());
   EXPECT_EQ(view_model->view_at(0),
             test_api_->GetViewAtVisualIndex(0 /* page */, 0 /* slot */));
-  EXPECT_TRUE(view_model->view_at(0)->item()->is_folder());
+  const AppListItem* folder_item = view_model->view_at(0)->item();
+  EXPECT_TRUE(folder_item->is_folder());
+
+  // The "page break" item remains, but it will be removed later in
+  // AppListSyncableService.
+  EXPECT_EQ(std::string("PageBreakItem," + folder_item->id()),
+            model_->GetModelContent());
 }
 
 TEST_P(AppsGridGapTest, MoveLastItemForReorderInNextPage) {
@@ -1267,6 +1277,7 @@ TEST_P(AppsGridGapTest, MoveLastItemForReorderInNextPage) {
   EXPECT_EQ(view_model->view_at(1),
             test_api_->GetViewAtVisualIndex(0 /* page */, 1 /* slot */));
   EXPECT_EQ("Item 1", view_model->view_at(1)->item()->id());
+  EXPECT_EQ(std::string("Item 0,Item 1"), model_->GetModelContent());
 
   // Drag the first item to next page and drag the second item to the left of
   // the first item.
@@ -1290,6 +1301,11 @@ TEST_P(AppsGridGapTest, MoveLastItemForReorderInNextPage) {
   EXPECT_EQ(view_model->view_at(1),
             test_api_->GetViewAtVisualIndex(0 /* page */, 1 /* slot */));
   EXPECT_EQ("Item 0", view_model->view_at(1)->item()->id());
+
+  // The "page break" item remains, but it will be removed later in
+  // AppListSyncableService.
+  EXPECT_EQ(std::string("PageBreakItem,Item 1,Item 0"),
+            model_->GetModelContent());
 }
 
 TEST_P(AppsGridGapTest, MoveLastItemToNewEmptyPage) {
@@ -1306,6 +1322,7 @@ TEST_P(AppsGridGapTest, MoveLastItemToNewEmptyPage) {
   EXPECT_EQ(view_model->view_at(0),
             test_api_->GetViewAtVisualIndex(0 /* page */, 0 /* slot */));
   EXPECT_EQ("Item 0", view_model->view_at(0)->item()->id());
+  EXPECT_EQ(std::string("Item 0"), model_->GetModelContent());
 
   // Drag the item to next page.
   gfx::Point from = GetItemRectOnCurrentPageAt(0, 0).CenterPoint();
@@ -1324,6 +1341,7 @@ TEST_P(AppsGridGapTest, MoveLastItemToNewEmptyPage) {
   EXPECT_EQ(view_model->view_at(0),
             test_api_->GetViewAtVisualIndex(0 /* page */, 0 /* slot */));
   EXPECT_EQ("Item 0", view_model->view_at(0)->item()->id());
+  EXPECT_EQ(std::string("Item 0"), model_->GetModelContent());
 }
 
 TEST_P(AppsGridGapTest, MoveItemToPreviousFullPage) {
@@ -1348,6 +1366,13 @@ TEST_P(AppsGridGapTest, MoveItemToPreviousFullPage) {
   EXPECT_EQ("Item " + std::to_string(kApps - 1),
             view_model->view_at(kApps - 1)->item()->id());
 
+  // There's no "page break" item between Item 19 and 20, although there are two
+  // pages. It will only be added after user operations.
+  EXPECT_EQ(std::string("Item 0,Item 1,Item 2,Item 3,Item 4,Item 5,Item 6,Item "
+                        "7,Item 8,Item 9,Item 10,Item 11,Item 12,Item 13,Item "
+                        "14,Item 15,Item 16,Item 17,Item 18,Item 19,Item 20"),
+            model_->GetModelContent());
+
   // Drag the last item to the first item's left position in previous page.
   gfx::Point from = test_api_->GetItemTileRectAtVisualIndex(1, 0).CenterPoint();
   gfx::Rect tile_rect = test_api_->GetItemTileRectAtVisualIndex(0, 0);
@@ -1376,6 +1401,13 @@ TEST_P(AppsGridGapTest, MoveItemToPreviousFullPage) {
             test_api_->GetViewAtVisualIndex(1 /* page */, 0 /* slot */));
   EXPECT_EQ("Item " + std::to_string(kApps - 2),
             view_model->view_at(kApps - 1)->item()->id());
+
+  // A "page break" item is added to split the pages.
+  EXPECT_EQ(
+      std::string("Item 20,Item 0,Item 1,Item 2,Item 3,Item 4,Item 5,Item "
+                  "6,Item 7,Item 8,Item 9,Item 10,Item 11,Item 12,Item 13,Item "
+                  "14,Item 15,Item 16,Item 17,Item 18,PageBreakItem,Item 19"),
+      model_->GetModelContent());
 }
 
 }  // namespace test

ui/views/view_model.cc

@@ -67,8 +67,7 @@ int ViewModelBase::GetIndexOfView(const View* view) const {
   return -1;
 }
 
-ViewModelBase::ViewModelBase() {
-}
+ViewModelBase::ViewModelBase() = default;
 
 void ViewModelBase::AddUnsafe(View* view, int index) {
   DCHECK_LE(index, static_cast<int>(entries_.size()));