Commit a7ea8d75 authored by Jenny Zhang's avatar Jenny Zhang Committed by Commit Bot

Fix app list sync paging issue caused by incompatible app overflow.

When there are incompatible apps on different type of devices under the
same user account, it is possible that moving or adding an app on an
empty spot on a page of a different type of device (e.g. Device 1) may
cause app overflow on another device (e.g. Device 2) since it may have
more apps on the same page. See details in
http://crbug.com/1098174#c11.

When the change is synced to the Device 2, paged view structure may load
meta data and detect a full page of apps without a page break item at
the end of the overflowed page, which will mess the calculation in
PagedViewStructure::GetTargetItemIndexForMove later if user moves app
around. Therefore, after the sync service has finished processing sync
change, SaveToMetaData should be called to insert a page break item at
the end of each full page if the page break is found missing.

Bug: 1098174
Change-Id: Iccbfde81810e337662fea2bec71e25d53d8b08db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2277201
Commit-Queue: Jenny Zhang <jennyz@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784546}
parent 0abf0ab6
......@@ -14,6 +14,7 @@
#include "ash/app_list/views/app_list_main_view.h"
#include "ash/app_list/views/app_list_view.h"
#include "ash/app_list/views/apps_container_view.h"
#include "ash/app_list/views/apps_grid_view.h"
#include "ash/app_list/views/contents_view.h"
#include "ash/app_list/views/search_box_view.h"
#include "ash/assistant/assistant_controller_impl.h"
......@@ -440,6 +441,27 @@ void AppListControllerImpl::ResolveOemFolderPosition(
std::move(callback).Run(std::move(metadata));
}
void AppListControllerImpl::NotifyProcessSyncChangesFinished() {
// When there are incompatible apps on different devices under the same
// user account, it is possible that moving or adding an app on an empty
// spot on a page of a different type of device (e.g. Device 1) may cause app
// overflow on another device (e.g. Device 2) since it may have more apps on
// the same page. See details in http://crbug.com/1098174.
// When the change is synced to the Device 2, paged view structure may load
// meta data and detect a full page of apps without a page break item
// at the end of the overflowed page. Therefore, after the sync service has
// finished processing sync change, SaveToMetaData should be called to insert
// page break items if there are any missing at the end of full pages.
AppListView* const app_list_view = presenter_.GetView();
if (app_list_view) {
app_list_view->app_list_main_view()
->contents_view()
->apps_container_view()
->apps_grid_view()
->UpdatePagedViewStructure();
}
}
void AppListControllerImpl::DismissAppList() {
if (tracked_app_window_) {
tracked_app_window_->RemoveObserver(this);
......
......@@ -113,6 +113,7 @@ class ASH_EXPORT AppListControllerImpl : public AppListController,
void ResolveOemFolderPosition(
const syncer::StringOrdinal& preferred_oem_position,
ResolveOemFolderPositionCallback callback) override;
void NotifyProcessSyncChangesFinished() override;
void DismissAppList() override;
void GetAppInfoDialogBounds(GetAppInfoDialogBoundsCallback callback) override;
void ShowAppList() override;
......
......@@ -2259,6 +2259,11 @@ AppListItemView* AppsGridView::GetCurrentPageLastItemViewInFolder() {
return view_model_.view_at(last_index);
}
void AppsGridView::UpdatePagedViewStructure() {
if (!folder_delegate_)
view_structure_.SaveToMetadata();
}
bool AppsGridView::IsTabletMode() const {
return contents_view_->app_list_view()->is_tablet_mode();
}
......
......@@ -279,6 +279,9 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
// Returns the last app list item view in the selected page in the folder.
AppListItemView* GetCurrentPageLastItemViewInFolder();
// Updates paged view structure and save it to meta data.
void UpdatePagedViewStructure();
// Returns true if tablet mode is active.
bool IsTabletMode() const;
......
......@@ -118,6 +118,9 @@ class ASH_PUBLIC_EXPORT AppListController {
const syncer::StringOrdinal& preferred_oem_position,
ResolveOemFolderPositionCallback callback) = 0;
// Notifies sync service has finished processing sync changes.
virtual void NotifyProcessSyncChangesFinished() = 0;
// Dismisses the app list.
virtual void DismissAppList() = 0;
......
......@@ -115,6 +115,7 @@ class AppListModelUpdater {
app_list::AppListSyncableService::SyncItem* sync_item,
bool update_name,
bool update_folder) {}
virtual void NotifyProcessSyncChangesFinished() {}
using GetMenuModelCallback =
base::OnceCallback<void(std::unique_ptr<ui::SimpleMenuModel>)>;
......
......@@ -1071,6 +1071,8 @@ base::Optional<syncer::ModelError> AppListSyncableService::ProcessSyncChanges(
HandleUpdateFinished(false /* clean_up_after_init_sync */);
GetModelUpdater()->NotifyProcessSyncChangesFinished();
return base::nullopt;
}
......
......@@ -450,6 +450,11 @@ void ChromeAppListModelUpdater::UpdateAppItemFromSyncItem(
}
}
void ChromeAppListModelUpdater::NotifyProcessSyncChangesFinished() {
if (app_list_controller_)
app_list_controller_->NotifyProcessSyncChangesFinished();
}
void ChromeAppListModelUpdater::AddObserver(
AppListModelUpdaterObserver* observer) {
observers_.AddObserver(observer);
......
......@@ -93,6 +93,7 @@ class ChromeAppListModelUpdater : public AppListModelUpdater {
app_list::AppListSyncableService::SyncItem* sync_item,
bool update_name,
bool update_folder) override;
void NotifyProcessSyncChangesFinished() override;
// Methods to handle model update from ash:
void OnFolderCreated(std::unique_ptr<ash::AppListItemMetadata> item) override;
......
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