Commit e490f8a2 authored by Jit Yao Yap's avatar Jit Yao Yap Committed by Chromium LUCI CQ

Add AppListModelUpdater::GetPositionBeforeFirstItem

This CL adds GetPositionBeforeFirstItem which returns a
syncer::StringOrdinal which is before the position of the first item
in the item list.

This position is used to allow inserting of app items to the front of
the item list.

Bug: 1167998
Change-Id: I89c41383e72c471723e1eb9457bb7d4508f85218
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2640379
Commit-Queue: Jit Yao Yap <jityao@google.com>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845984}
parent 7ed5c8c1
......@@ -68,3 +68,19 @@ syncer::StringOrdinal AppListModelUpdater::GetFirstAvailablePositionInternal(
return syncer::StringOrdinal::CreateInitialOrdinal();
return sorted_items.back()->position().CreateAfter();
}
// static
syncer::StringOrdinal AppListModelUpdater::GetPositionBeforeFirstItemInternal(
const std::vector<ChromeAppListItem*>& top_level_items) {
auto iter =
std::min_element(top_level_items.begin(), top_level_items.end(),
[](ChromeAppListItem* const& item1,
ChromeAppListItem* const& item2) -> bool {
return item1->position().LessThan(item2->position());
});
if (iter == top_level_items.end())
return syncer::StringOrdinal::CreateInitialOrdinal();
return (*iter)->position().CreateBefore();
}
......@@ -100,6 +100,8 @@ class AppListModelUpdater {
virtual void GetIdToAppListIndexMap(GetIdToAppListIndexMapCallback callback) {
}
virtual syncer::StringOrdinal GetFirstAvailablePosition() const = 0;
// Returns a position which is before the first item in the item list.
virtual syncer::StringOrdinal GetPositionBeforeFirstItem() const = 0;
// Methods for AppListSyncableService:
virtual void AddItemToOemFolder(
......@@ -146,6 +148,11 @@ class AppListModelUpdater {
static syncer::StringOrdinal GetFirstAvailablePositionInternal(
const std::vector<ChromeAppListItem*>& top_level_items);
// Returns a position which is before the first item in the app list. If
// |top_level_items| is empty, creates an initial position instead.
static syncer::StringOrdinal GetPositionBeforeFirstItemInternal(
const std::vector<ChromeAppListItem*>& top_level_items);
private:
const int model_id_;
};
......
......@@ -352,17 +352,12 @@ void ChromeAppListModelUpdater::GetContextMenuModel(
syncer::StringOrdinal ChromeAppListModelUpdater::GetFirstAvailablePosition()
const {
std::vector<ChromeAppListItem*> top_level_items;
for (auto& entry : items_) {
ChromeAppListItem* item = entry.second.get();
DCHECK(item->position().IsValid())
<< "Item with invalid position: id=" << item->id()
<< ", name=" << item->name() << ", is_folder=" << item->is_folder()
<< ", is_page_break=" << item->is_page_break();
if (item->folder_id().empty() && item->position().IsValid())
top_level_items.emplace_back(item);
}
return GetFirstAvailablePositionInternal(top_level_items);
return GetFirstAvailablePositionInternal(GetTopLevelItems());
}
syncer::StringOrdinal ChromeAppListModelUpdater::GetPositionBeforeFirstItem()
const {
return GetPositionBeforeFirstItemInternal(GetTopLevelItems());
}
////////////////////////////////////////////////////////////////////////////////
......@@ -547,3 +542,18 @@ void ChromeAppListModelUpdater::OnPageBreakItemDeleted(const std::string& id) {
observer.OnAppListItemWillBeDeleted(chrome_item);
items_.erase(id);
}
std::vector<ChromeAppListItem*> ChromeAppListModelUpdater::GetTopLevelItems()
const {
std::vector<ChromeAppListItem*> top_level_items;
for (auto& entry : items_) {
ChromeAppListItem* item = entry.second.get();
DCHECK(item->position().IsValid())
<< "Item with invalid position: id=" << item->id()
<< ", name=" << item->name() << ", is_folder=" << item->is_folder()
<< ", is_page_break=" << item->is_page_break();
if (item->folder_id().empty() && item->position().IsValid())
top_level_items.emplace_back(item);
}
return top_level_items;
}
......@@ -83,6 +83,7 @@ class ChromeAppListModelUpdater : public AppListModelUpdater {
void GetContextMenuModel(const std::string& id,
GetMenuModelCallback callback) override;
syncer::StringOrdinal GetFirstAvailablePosition() const override;
syncer::StringOrdinal GetPositionBeforeFirstItem() const override;
// Methods for AppListSyncableService:
void AddItemToOemFolder(
......@@ -109,6 +110,8 @@ class ChromeAppListModelUpdater : public AppListModelUpdater {
void RemoveObserver(AppListModelUpdaterObserver* observer) override;
private:
std::vector<ChromeAppListItem*> GetTopLevelItems() const;
// A map from a ChromeAppListItem's id to its unique pointer. This item set
// matches the one in AppListModel.
std::map<std::string, std::unique_ptr<ChromeAppListItem>> items_;
......
......@@ -25,6 +25,7 @@
#include "chrome/common/chrome_switches.h"
#include "components/account_id/account_id.h"
#include "components/session_manager/core/session_manager.h"
#include "components/sync/model/string_ordinal.h"
#include "content/public/test/browser_test.h"
#include "extensions/browser/extension_system.h"
......@@ -122,6 +123,48 @@ IN_PROC_BROWSER_TEST_F(OemAppPositionTest, ValidOemAppPosition) {
EXPECT_TRUE(oem_folder->position().IsValid());
}
IN_PROC_BROWSER_TEST_F(AppPositionReorderingTest,
GetPositionBeforeFirstItemTest) {
AppListClientImpl* client = AppListClientImpl::GetInstance();
ASSERT_TRUE(client);
AppListModelUpdater* model_updater = test::GetModelUpdater(client);
// Default apps will be present but we add an app to guarantee there will be
// at least 1 app.
const std::string app1_id =
LoadExtension(test_data_dir_.AppendASCII("app1"))->id();
ASSERT_FALSE(app1_id.empty());
// Create the app list view and show the apps grid.
ash::AcceleratorController::Get()->PerformActionIfEnabled(
ash::TOGGLE_APP_LIST_FULLSCREEN, {});
std::vector<std::string> top_level_id_list =
app_list_test_api_.GetTopLevelViewIdList();
syncer::StringOrdinal position_before_first_item =
model_updater->GetPositionBeforeFirstItem();
// Check that position is before all items in the list.
for (const auto& id : top_level_id_list) {
ChromeAppListItem* item = model_updater->FindItem(id);
ASSERT_TRUE(position_before_first_item.LessThan(item->position()));
}
// Move app to the front.
app_list_test_api_.MoveItemToPosition(app1_id, 0);
std::vector<std::string> reordered_top_level_id_list =
app_list_test_api_.GetTopLevelViewIdList();
syncer::StringOrdinal new_position_before_first_item =
model_updater->GetPositionBeforeFirstItem();
// Re-check that position is before all items in the list.
for (const auto& id : reordered_top_level_id_list) {
ChromeAppListItem* item = model_updater->FindItem(id);
ASSERT_TRUE(new_position_before_first_item.LessThan(item->position()));
}
}
IN_PROC_BROWSER_TEST_F(AppPositionReorderingTest,
PRE_ReorderAppPositionInTopLevelAppList) {
const std::string app1_id =
......
......@@ -120,16 +120,12 @@ void FakeAppListModelUpdater::GetIdToAppListIndexMap(
syncer::StringOrdinal FakeAppListModelUpdater::GetFirstAvailablePosition()
const {
std::vector<ChromeAppListItem*> top_level_items;
for (auto& item : items_) {
DCHECK(item->position().IsValid())
<< "Item with invalid position: id=" << item->id()
<< ", name=" << item->name() << ", is_folder=" << item->is_folder()
<< ", is_page_break=" << item->is_page_break();
if (item->folder_id().empty() && item->position().IsValid())
top_level_items.emplace_back(item.get());
}
return GetFirstAvailablePositionInternal(top_level_items);
return GetFirstAvailablePositionInternal(GetTopLevelItems());
}
syncer::StringOrdinal FakeAppListModelUpdater::GetPositionBeforeFirstItem()
const {
return GetPositionBeforeFirstItemInternal(GetTopLevelItems());
}
void FakeAppListModelUpdater::GetContextMenuModel(
......@@ -264,3 +260,17 @@ void FakeAppListModelUpdater::WaitForIconUpdates(size_t expected_updates) {
icon_updated_callback_ = run_loop.QuitClosure();
run_loop.Run();
}
std::vector<ChromeAppListItem*> FakeAppListModelUpdater::GetTopLevelItems()
const {
std::vector<ChromeAppListItem*> top_level_items;
for (auto& item : items_) {
DCHECK(item->position().IsValid())
<< "Item with invalid position: id=" << item->id()
<< ", name=" << item->name() << ", is_folder=" << item->is_folder()
<< ", is_page_break=" << item->is_page_break();
if (item->folder_id().empty() && item->position().IsValid())
top_level_items.emplace_back(item.get());
}
return top_level_items;
}
......@@ -56,6 +56,7 @@ class FakeAppListModelUpdater : public AppListModelUpdater {
bool FindItemIndexForTest(const std::string& id, size_t* index) override;
void GetIdToAppListIndexMap(GetIdToAppListIndexMapCallback callback) override;
syncer::StringOrdinal GetFirstAvailablePosition() const override;
syncer::StringOrdinal GetPositionBeforeFirstItem() const override;
void GetContextMenuModel(const std::string& id,
GetMenuModelCallback callback) override;
size_t BadgedItemCount() override;
......@@ -78,6 +79,8 @@ class FakeAppListModelUpdater : public AppListModelUpdater {
size_t update_image_count() const { return update_image_count_; }
std::vector<ChromeAppListItem*> GetTopLevelItems() const;
private:
bool search_engine_is_google_ = false;
std::vector<std::unique_ptr<ChromeAppListItem>> items_;
......
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