Commit 179d953c authored by Michael Wasserman's avatar Michael Wasserman Committed by Commit Bot

Revert "app_launcher: Fix OEM folder is not created in new user flow."

This reverts commit 94fac28e.

Reason for revert: Broke compile: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/5265

Original change's description:
> app_launcher: Fix OEM folder is not created in new user flow.
> 
> This fixes the issue when OEM folder is created empty after new user
> flow and OEM items are placed to non-named folder. This also fixes
> various crashes when trying to manipulate these items.
> 
> Bug: 828209
> Test: Manually
> Change-Id: Ib07f0ab1d301d1e38a6ad91fca10396e9f58ccf7
> Reviewed-on: https://chromium-review.googlesource.com/1016024
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: Yury Khmel <khmel@google.com>
> Cr-Commit-Position: refs/heads/master@{#551716}

TBR=xiyuan@chromium.org,khmel@google.com

Change-Id: Iaa41f7f11f9992b9d5037b4ae5dd2289aa99b4cb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 828209
Reviewed-on: https://chromium-review.googlesource.com/1017261Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551725}
parent 396164ee
...@@ -589,13 +589,7 @@ syncer::StringOrdinal AppListControllerImpl::GetOemFolderPos() { ...@@ -589,13 +589,7 @@ syncer::StringOrdinal AppListControllerImpl::GetOemFolderPos() {
std::unique_ptr<app_list::AppListItem> AppListControllerImpl::CreateAppListItem( std::unique_ptr<app_list::AppListItem> AppListControllerImpl::CreateAppListItem(
AppListItemMetadataPtr metadata) { AppListItemMetadataPtr metadata) {
std::unique_ptr<app_list::AppListItem> app_list_item = std::unique_ptr<app_list::AppListItem> app_list_item =
metadata->is_folder std::make_unique<app_list::AppListItem>(metadata->id);
? std::make_unique<app_list::AppListFolderItem>(
metadata->id,
metadata->id == kOemFolderId
? app_list::AppListFolderItem::FOLDER_TYPE_OEM
: app_list::AppListFolderItem::FOLDER_TYPE_NORMAL)
: std::make_unique<app_list::AppListItem>(metadata->id);
app_list_item->SetMetadata(std::move(metadata)); app_list_item->SetMetadata(std::move(metadata));
return app_list_item; return app_list_item;
} }
......
...@@ -6,8 +6,6 @@ ...@@ -6,8 +6,6 @@
namespace ash { namespace ash {
const char kOemFolderId[] = "ddb1da55-d478-4243-8642-56d3041f0263";
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// SearchResultTag: // SearchResultTag:
......
...@@ -13,9 +13,6 @@ ...@@ -13,9 +13,6 @@
namespace ash { namespace ash {
// Id of OEM folder in app list.
extern const char kOemFolderId[];
// All possible states of the app list. // All possible states of the app list.
// Note: Do not change the order of these as they are used for metrics. // Note: Do not change the order of these as they are used for metrics.
enum class AppListState { enum class AppListState {
......
...@@ -290,6 +290,10 @@ class AppListSyncableService::ModelUpdaterDelegate ...@@ -290,6 +290,10 @@ class AppListSyncableService::ModelUpdaterDelegate
// AppListSyncableService // AppListSyncableService
// static
const char AppListSyncableService::kOemFolderId[] =
"ddb1da55-d478-4243-8642-56d3041f0263";
// static // static
void AppListSyncableService::RegisterProfilePrefs( void AppListSyncableService::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) { user_prefs::PrefRegistrySyncable* registry) {
...@@ -439,7 +443,7 @@ AppListSyncableService::GetSyncItem(const std::string& id) const { ...@@ -439,7 +443,7 @@ AppListSyncableService::GetSyncItem(const std::string& id) const {
void AppListSyncableService::SetOemFolderName(const std::string& name) { void AppListSyncableService::SetOemFolderName(const std::string& name) {
oem_folder_name_ = name; oem_folder_name_ = name;
model_updater_->SetItemName(ash::kOemFolderId, oem_folder_name_); model_updater_->SetItemName(kOemFolderId, oem_folder_name_);
} }
AppListModelUpdater* AppListSyncableService::GetModelUpdater() { AppListModelUpdater* AppListSyncableService::GetModelUpdater() {
...@@ -473,7 +477,7 @@ void AppListSyncableService::AddItem( ...@@ -473,7 +477,7 @@ void AppListSyncableService::AddItem(
if (AppIsOem(app_item->id())) { if (AppIsOem(app_item->id())) {
VLOG(2) << this << ": AddItem to OEM folder: " << sync_item->ToString(); VLOG(2) << this << ": AddItem to OEM folder: " << sync_item->ToString();
model_updater_->AddItemToOemFolder( model_updater_->AddItemToOemFolder(
std::move(app_item), FindSyncItem(ash::kOemFolderId), ash::kOemFolderId, std::move(app_item), FindSyncItem(kOemFolderId), kOemFolderId,
oem_folder_name_, GetPreferredOemFolderPos()); oem_folder_name_, GetPreferredOemFolderPos());
} else { } else {
std::string folder_id = sync_item->parent_id; std::string folder_id = sync_item->parent_id;
...@@ -554,7 +558,7 @@ void AppListSyncableService::AddOrUpdateFromSyncItem( ...@@ -554,7 +558,7 @@ void AppListSyncableService::AddOrUpdateFromSyncItem(
const ChromeAppListItem* app_item) { const ChromeAppListItem* app_item) {
// Do not create a sync item for the OEM folder here, do that in // Do not create a sync item for the OEM folder here, do that in
// ResolveFolderPositions once the position has been resolved. // ResolveFolderPositions once the position has been resolved.
if (app_item->id() == ash::kOemFolderId) if (app_item->id() == kOemFolderId)
return; return;
DCHECK(app_item->position().IsValid()); DCHECK(app_item->position().IsValid());
...@@ -563,9 +567,8 @@ void AppListSyncableService::AddOrUpdateFromSyncItem( ...@@ -563,9 +567,8 @@ void AppListSyncableService::AddOrUpdateFromSyncItem(
if (sync_item) { if (sync_item) {
model_updater_->UpdateAppItemFromSyncItem( model_updater_->UpdateAppItemFromSyncItem(
sync_item, sync_item,
sync_item->item_id != sync_item->item_id != kOemFolderId, // Don't sync oem folder's name.
ash::kOemFolderId, // Don't sync oem folder's name. false); // Don't sync its folder here.
false); // Don't sync its folder here.
if (!sync_item->item_ordinal.IsValid()) { if (!sync_item->item_ordinal.IsValid()) {
UpdateSyncItem(app_item); UpdateSyncItem(app_item);
VLOG(2) << "Flushing position to sync item " << sync_item; VLOG(2) << "Flushing position to sync item " << sync_item;
...@@ -644,10 +647,10 @@ void AppListSyncableService::RemoveUninstalledItem(const std::string& id) { ...@@ -644,10 +647,10 @@ void AppListSyncableService::RemoveUninstalledItem(const std::string& id) {
void AppListSyncableService::UpdateItem(const ChromeAppListItem* app_item) { void AppListSyncableService::UpdateItem(const ChromeAppListItem* app_item) {
// Check to see if the item needs to be moved to/from the OEM folder. // Check to see if the item needs to be moved to/from the OEM folder.
bool is_oem = AppIsOem(app_item->id()); bool is_oem = AppIsOem(app_item->id());
if (!is_oem && app_item->folder_id() == ash::kOemFolderId) if (!is_oem && app_item->folder_id() == kOemFolderId)
model_updater_->MoveItemToFolder(app_item->id(), ""); model_updater_->MoveItemToFolder(app_item->id(), "");
else if (is_oem && app_item->folder_id() != ash::kOemFolderId) else if (is_oem && app_item->folder_id() != kOemFolderId)
model_updater_->MoveItemToFolder(app_item->id(), ash::kOemFolderId); model_updater_->MoveItemToFolder(app_item->id(), kOemFolderId);
} }
void AppListSyncableService::RemoveSyncItem(const std::string& id) { void AppListSyncableService::RemoveSyncItem(const std::string& id) {
...@@ -691,15 +694,14 @@ void AppListSyncableService::ResolveFolderPositions() { ...@@ -691,15 +694,14 @@ void AppListSyncableService::ResolveFolderPositions() {
model_updater_->UpdateAppItemFromSyncItem( model_updater_->UpdateAppItemFromSyncItem(
sync_item, sync_item,
sync_item->item_id != sync_item->item_id != kOemFolderId, // Don't sync oem folder's name.
ash::kOemFolderId, // Don't sync oem folder's name. false); // Don't sync its folder here.
false); // Don't sync its folder here.
} }
// Move the OEM folder if one exists and we have not synced its position. // Move the OEM folder if one exists and we have not synced its position.
if (!FindSyncItem(ash::kOemFolderId)) { if (!FindSyncItem(kOemFolderId)) {
model_updater_->ResolveOemFolderPosition( model_updater_->ResolveOemFolderPosition(
ash::kOemFolderId, GetPreferredOemFolderPos(), kOemFolderId, GetPreferredOemFolderPos(),
base::BindOnce( base::BindOnce(
[](base::WeakPtr<AppListSyncableService> self, [](base::WeakPtr<AppListSyncableService> self,
ChromeAppListItem* oem_folder) { ChromeAppListItem* oem_folder) {
...@@ -961,9 +963,8 @@ void AppListSyncableService::ProcessNewSyncItem(SyncItem* sync_item) { ...@@ -961,9 +963,8 @@ void AppListSyncableService::ProcessNewSyncItem(SyncItem* sync_item) {
// We don't create new folders here, the model will do that. // We don't create new folders here, the model will do that.
model_updater_->UpdateAppItemFromSyncItem( model_updater_->UpdateAppItemFromSyncItem(
sync_item, sync_item,
sync_item->item_id != sync_item->item_id != kOemFolderId, // Don't sync oem folder's name.
ash::kOemFolderId, // Don't sync oem folder's name. false); // It's a folder itself.
false); // It's a folder itself.
return; return;
} }
case sync_pb::AppListSpecifics::TYPE_URL: { case sync_pb::AppListSpecifics::TYPE_URL: {
...@@ -984,7 +985,7 @@ void AppListSyncableService::ProcessExistingSyncItem(SyncItem* sync_item) { ...@@ -984,7 +985,7 @@ void AppListSyncableService::ProcessExistingSyncItem(SyncItem* sync_item) {
model_updater_->UpdateAppItemFromSyncItem( model_updater_->UpdateAppItemFromSyncItem(
sync_item, sync_item,
sync_item->item_id != ash::kOemFolderId, // Don't sync oem folder's name. sync_item->item_id != kOemFolderId, // Don't sync oem folder's name.
true); // The only place where sync can change an item's folder. true); // The only place where sync can change an item's folder.
} }
......
...@@ -100,6 +100,8 @@ class AppListSyncableService : public syncer::SyncableService, ...@@ -100,6 +100,8 @@ class AppListSyncableService : public syncer::SyncableService,
~AppListSyncableService() override; ~AppListSyncableService() override;
static const char kOemFolderId[];
// Registers prefs to support local storage. // Registers prefs to support local storage.
static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
......
...@@ -262,9 +262,11 @@ TEST_F(AppListSyncableServiceTest, OEMFolderForConflictingPos) { ...@@ -262,9 +262,11 @@ TEST_F(AppListSyncableServiceTest, OEMFolderForConflictingPos) {
// OEM item is not top level element. // OEM item is not top level element.
ChromeAppListItem* oem_app_item = model_updater()->FindItem(oem_app_id); ChromeAppListItem* oem_app_item = model_updater()->FindItem(oem_app_id);
EXPECT_NE(nullptr, oem_app_item); EXPECT_NE(nullptr, oem_app_item);
EXPECT_EQ(oem_app_item->folder_id(), ash::kOemFolderId); EXPECT_EQ(oem_app_item->folder_id(),
app_list::AppListSyncableService::kOemFolderId);
// But OEM folder is. // But OEM folder is.
ChromeAppListItem* oem_folder = model_updater()->FindItem(ash::kOemFolderId); ChromeAppListItem* oem_folder =
model_updater()->FindItem(app_list::AppListSyncableService::kOemFolderId);
EXPECT_NE(nullptr, oem_folder); EXPECT_NE(nullptr, oem_folder);
EXPECT_EQ(oem_folder->folder_id(), ""); EXPECT_EQ(oem_folder->folder_id(), "");
} }
......
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