Revert "Install app to first available position in app list"

This reverts commit e39c47e5.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Install app to first available position in app list
> 
> Changes:
> 1. Calculate the first available position in AppListModelUpdater for
>    newly added item.
> 2. Add test coverage.
> 
> Bug: 883939
> Test: AppListSyncableServiceTest.FirstAvailablePosition
> Change-Id: Ibdd2f27455ffd0c76f45906adbc025baab1ff319
> Reviewed-on: https://chromium-review.googlesource.com/1231193
> Commit-Queue: Weidong Guo <weidongg@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#592539}

TBR=xiyuan@chromium.org,stevenjb@chromium.org,weidongg@chromium.org

Change-Id: I03fc07c2f963eb2e569e170d8175a47c7f174fe1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 883939
Reviewed-on: https://chromium-review.googlesource.com/1237144Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592906}

ash/app_list/views/apps_grid_view.cc

@@ -1058,7 +1058,11 @@ int AppsGridView::TilesPerPage(int page) const {
   if (folder_delegate_)
     return kMaxFolderItemsPerPage;
 
-  return AppListConfig::instance().GetMaxNumOfItemsPerPage(page);
+  // In new style launcher, the first row of first page is no longger suggestion
+  // apps.
+  if (page == 0 && !is_new_style_launcher_enabled_)
+    return cols_ * (rows_per_page_ - 1);
+  return cols_ * rows_per_page_;
 }
 
 void AppsGridView::UpdatePaging() {

ash/public/cpp/app_list/app_list_config.cc

@@ -58,9 +58,8 @@ AppListConfig::AppListConfig()
       // TODO(manucornet): Share the value with ShelfConstants and use
       // 48 when the new shelf UI is turned off.
       shelf_height_(56),
-      blur_radius_(30),
-      is_new_style_launcher_enabled_(features::IsNewStyleLauncherEnabled()) {
-  if (is_new_style_launcher_enabled_) {
+      blur_radius_(30) {
+  if (features::IsNewStyleLauncherEnabled()) {
     grid_tile_width_ = 120;
     grid_tile_height_ = 112;
     grid_tile_spacing_ = 0;
@@ -123,12 +122,4 @@ int AppListConfig::GetPreferredIconDimension(
   return 0;
 }
 
-int AppListConfig::GetMaxNumOfItemsPerPage(int page) const {
-  // In new style launcher, the first row of first page is no longger suggestion
-  // apps.
-  if (page == 0 && !is_new_style_launcher_enabled_)
-    return preferred_cols_ * (preferred_rows_ - 1);
-  return preferred_cols_ * preferred_rows_;
-}
-
 }  // namespace app_list

ash/public/cpp/app_list/app_list_config.h

@@ -140,9 +140,6 @@ class ASH_PUBLIC_EXPORT AppListConfig {
   int GetPreferredIconDimension(
       ash::SearchResultDisplayType display_type) const;
 
-  // Returns the maximum number of items allowed in specified page in apps grid.
-  int GetMaxNumOfItemsPerPage(int page) const;
-
  private:
   // The tile view's width and height of the item in apps grid view.
   int grid_tile_width_;
@@ -262,9 +259,6 @@ class ASH_PUBLIC_EXPORT AppListConfig {
 
   // The blur radius used in the app list.
   int blur_radius_ = 30;
-
-  // True if new style launcher feature is enabled.
-  const bool is_new_style_launcher_enabled_;
 };
 
 }  // namespace app_list

chrome/browser/ui/BUILD.gn

@@ -3450,7 +3450,6 @@ jumbo_split_static_library("ui") {
       "app_list/app_list_controller_delegate.h",
       "app_list/app_list_model_builder.cc",
       "app_list/app_list_model_builder.h",
-      "app_list/app_list_model_updater.cc",
       "app_list/app_list_model_updater.h",
       "app_list/app_list_model_updater_delegate.h",
       "app_list/app_list_syncable_service.cc",

chrome/browser/ui/app_list/app_context_menu_unittest.cc

@@ -49,8 +49,8 @@ class FakeAppContextMenuDelegate : public app_list::AppContextMenuDelegate {
   DISALLOW_COPY_AND_ASSIGN(FakeAppContextMenuDelegate);
 };
 
-class FakeAppListControllerDelegate
-    : public test::TestAppListControllerDelegate {
+class FakeAppListControllerDelegate :
+    public test::TestAppListControllerDelegate {
  public:
   FakeAppListControllerDelegate() = default;
   ~FakeAppListControllerDelegate() override = default;
@@ -95,7 +95,7 @@ class FakeAppListControllerDelegate
 std::unique_ptr<KeyedService> MenuManagerFactory(
     content::BrowserContext* context) {
   return extensions::MenuManagerFactory::BuildServiceInstanceForTesting(
-      context);
+              context);
 }
 
 std::unique_ptr<ui::MenuModel> GetContextMenuModel(ChromeAppListItem* item) {
@@ -160,14 +160,21 @@ class AppContextMenuTest : public AppListTestBase,
  protected:
   struct MenuState {
     // Defines separator.
-    MenuState() : command_id(-1), is_enabled(true), is_checked(false) {}
+    MenuState() : command_id(-1), is_enabled(true), is_checked(false) {
+    }
 
     // Defines enabled unchecked command.
     explicit MenuState(int command_id)
-        : command_id(command_id), is_enabled(true), is_checked(false) {}
+        : command_id(command_id),
+          is_enabled(true),
+          is_checked(false) {
+    }
 
     MenuState(int command_id, bool enabled, bool checked)
-        : command_id(command_id), is_enabled(enabled), is_checked(checked) {}
+        : command_id(command_id),
+          is_enabled(enabled),
+          is_checked(checked) {
+    }
 
     int command_id;
     bool is_enabled;
@@ -179,7 +186,7 @@ class AppContextMenuTest : public AppListTestBase,
                          const MenuState& state) {
     EXPECT_EQ(state.command_id, menu_model->GetCommandIdAt(index));
     if (state.command_id == -1)
-      return;  // Don't check separator.
+      return;   // Don't check separator.
     EXPECT_EQ(state.is_enabled, menu_model->IsEnabledAt(index));
     EXPECT_EQ(state.is_checked, menu_model->IsItemCheckedAt(index));
   }
@@ -195,11 +202,17 @@ class AppContextMenuTest : public AppListTestBase,
     EXPECT_EQ(state_index, states.size());
   }
 
-  FakeAppListControllerDelegate* controller() { return controller_.get(); }
+  FakeAppListControllerDelegate* controller() {
+    return controller_.get();
+  }
 
-  FakeAppContextMenuDelegate* menu_delegate() { return menu_delegate_.get(); }
+  FakeAppContextMenuDelegate* menu_delegate() {
+    return menu_delegate_.get();
+  }
 
-  Profile* profile() { return profile_.get(); }
+  Profile* profile() {
+    return profile_.get();
+  }
 
   void AddSeparator(std::vector<MenuState>* states) {
     // TODO(newcomer): Remove this function when touchable app context menus are
@@ -233,7 +246,9 @@ class AppContextMenuTest : public AppListTestBase,
     controller_->SetAppPinnable(app_id, pinnable);
     controller_->SetCanShowAppInfo(can_show_app_info);
     controller_->SetExtensionLaunchType(profile(), app_id, launch_type);
-    app_list::ExtensionAppContextMenu menu(menu_delegate(), profile(), app_id,
+    app_list::ExtensionAppContextMenu menu(menu_delegate(),
+                                           profile(),
+                                           app_id,
                                            controller());
     menu.set_is_platform_app(platform_app);
     std::unique_ptr<ui::MenuModel> menu_model = GetMenuModel(&menu);
@@ -299,8 +314,10 @@ class AppContextMenuTest : public AppListTestBase,
   void TestChromeApp(bool can_show_app_info) {
     controller_ = std::make_unique<FakeAppListControllerDelegate>();
     controller_->SetCanShowAppInfo(can_show_app_info);
-    app_list::ExtensionAppContextMenu menu(
-        menu_delegate(), profile(), extension_misc::kChromeAppId, controller());
+    app_list::ExtensionAppContextMenu menu(menu_delegate(),
+                                           profile(),
+                                           extension_misc::kChromeAppId,
+                                           controller());
     std::unique_ptr<ui::MenuModel> menu_model = GetMenuModel(&menu);
     ASSERT_NE(nullptr, menu_model);
 
@@ -330,23 +347,29 @@ TEST_P(AppContextMenuTest, ExtensionApp) {
   app_list::ExtensionAppContextMenu::DisableInstalledExtensionCheckForTesting(
       false);
   for (extensions::LaunchType launch_type = extensions::LAUNCH_TYPE_FIRST;
-       launch_type < extensions::NUM_LAUNCH_TYPES;
-       launch_type = static_cast<extensions::LaunchType>(launch_type + 1)) {
+      launch_type < extensions::NUM_LAUNCH_TYPES;
+      launch_type = static_cast<extensions::LaunchType>(launch_type+1)) {
     AppListControllerDelegate::Pinnable pinnable;
     for (pinnable = AppListControllerDelegate::NO_PIN;
-         pinnable <= AppListControllerDelegate::PIN_FIXED;
-         pinnable =
-             static_cast<AppListControllerDelegate::Pinnable>(pinnable + 1)) {
+        pinnable <= AppListControllerDelegate::PIN_FIXED;
+        pinnable =
+            static_cast<AppListControllerDelegate::Pinnable>(pinnable+1)) {
       for (size_t combinations = 0; combinations < (1 << 2); ++combinations) {
         TestExtensionApp(AppListTestBase::kHostedAppId,
                          (combinations & (1 << 0)) != 0,
-                         (combinations & (1 << 1)) != 0, pinnable, launch_type);
+                         (combinations & (1 << 1)) != 0,
+                         pinnable,
+                         launch_type);
         TestExtensionApp(AppListTestBase::kPackagedApp1Id,
                          (combinations & (1 << 0)) != 0,
-                         (combinations & (1 << 1)) != 0, pinnable, launch_type);
+                         (combinations & (1 << 1)) != 0,
+                         pinnable,
+                         launch_type);
         TestExtensionApp(AppListTestBase::kPackagedApp2Id,
                          (combinations & (1 << 0)) != 0,
-                         (combinations & (1 << 1)) != 0, pinnable, launch_type);
+                         (combinations & (1 << 1)) != 0,
+                         pinnable,
+                         launch_type);
       }
     }
   }
@@ -362,7 +385,8 @@ TEST_P(AppContextMenuTest, ChromeApp) {
 TEST_P(AppContextMenuTest, NonExistingExtensionApp) {
   app_list::ExtensionAppContextMenu::DisableInstalledExtensionCheckForTesting(
       false);
-  app_list::ExtensionAppContextMenu menu(menu_delegate(), profile(),
+  app_list::ExtensionAppContextMenu menu(menu_delegate(),
+                                         profile(),
                                          "some_non_existing_extension_app",
                                          controller());
   std::unique_ptr<ui::MenuModel> menu_model = GetMenuModel(&menu);
@@ -690,8 +714,7 @@ TEST_P(AppContextMenuTest, InternalAppMenu) {
 
     controller()->SetAppPinnable(internal_app.app_id,
                                  AppListControllerDelegate::PIN_EDITABLE);
-    InternalAppItem item(profile(), nullptr /* model_updater */,
-                         nullptr /* sync_item */, internal_app);
+    InternalAppItem item(profile(), nullptr, internal_app);
     std::unique_ptr<ui::MenuModel> menu = GetContextMenuModel(&item);
     ASSERT_NE(nullptr, menu);
     EXPECT_EQ(1, menu->GetItemCount());

chrome/browser/ui/app_list/app_list_model_updater.cc

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-#include "chrome/browser/ui/app_list/app_list_model_updater.h"
-
-#include <algorithm>
-
-#include "ash/public/cpp/app_list/app_list_config.h"
-#include "chrome/browser/ui/app_list/chrome_app_list_item.h"
-
-// static
-syncer::StringOrdinal AppListModelUpdater::GetFirstAvailablePositionInternal(
-    const std::vector<ChromeAppListItem*>& top_level_items) {
-  // Sort the top level items by their positions.
-  std::vector<ChromeAppListItem*> sorted_items(top_level_items);
-  std::sort(sorted_items.begin(), sorted_items.end(),
-            [](ChromeAppListItem* const& item1,
-               ChromeAppListItem* const& item2) -> bool {
-              return item1->position().LessThan(item2->position());
-            });
-
-  // Find the first empty position in app list. If all pages are full, return
-  // the next position after last item.
-  int items_in_page = 0;
-  int page = 0;
-  for (size_t i = 0; i < sorted_items.size(); ++i) {
-    if (!sorted_items[i]->is_page_break()) {
-      ++items_in_page;
-      continue;
-    }
-
-    // There may be multiple "page break" items at the end of page while empty
-    // pages will not be shown in app list, so skip them.
-    const int max_items_in_page =
-        app_list::AppListConfig::instance().GetMaxNumOfItemsPerPage(page);
-    if (items_in_page > 0 && items_in_page < max_items_in_page) {
-      return sorted_items[i - 1]->position().CreateBetween(
-          sorted_items[i]->position());
-    }
-    if (items_in_page > 0)
-      ++page;
-    items_in_page = 0;
-  }
-
-  if (sorted_items.empty())
-    return syncer::StringOrdinal::CreateInitialOrdinal();
-  return sorted_items.back()->position().CreateAfter();
-}

chrome/browser/ui/app_list/app_list_model_updater.h

@@ -107,7 +107,6 @@ class AppListModelUpdater {
   virtual void ContextMenuItemSelected(const std::string& id,
                                        int command_id,
                                        int event_flags) {}
-  virtual syncer::StringOrdinal GetFirstAvailablePosition() const = 0;
 
   // Methods for AppListSyncableService:
   virtual void AddItemToOemFolder(
@@ -142,12 +141,6 @@ class AppListModelUpdater {
   virtual void OnPageBreakItemDeleted(const std::string& id) = 0;
 
   virtual void SetDelegate(AppListModelUpdaterDelegate* delegate) = 0;
-
- protected:
-  // Returns the first available position in app list. |top_level_items| are
-  // items without parents.
-  static syncer::StringOrdinal GetFirstAvailablePositionInternal(
-      const std::vector<ChromeAppListItem*>& top_level_items);
 };
 
 #endif  // CHROME_BROWSER_UI_APP_LIST_APP_LIST_MODEL_UPDATER_H_

chrome/browser/ui/app_list/app_list_syncable_service_unittest.cc

@@ -3,10 +3,8 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/app_list/app_list_syncable_service.h"
-#include "ash/public/cpp/app_list/app_list_config.h"
 #include "base/bind.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/strings/string_number_conversions.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/app_list_model_updater.h"
@@ -629,48 +627,3 @@ TEST_F(AppListSyncableServiceTest, PruneRedundantPageBreakItems) {
   ASSERT_TRUE(GetSyncItem(kItemId2));
   ASSERT_FALSE(GetSyncItem(kPageBreakItemId5));
 }
-
-TEST_F(AppListSyncableServiceTest, FirstAvailablePosition) {
-  RemoveAllExistingItems();
-
-  // Populate the first page with items and leave 1 empty slot at the end.
-  const int max_items_in_first_page =
-      app_list::AppListConfig::instance().GetMaxNumOfItemsPerPage(0);
-  syncer::StringOrdinal last_app_position =
-      syncer::StringOrdinal::CreateInitialOrdinal();
-  for (int i = 0; i < max_items_in_first_page - 1; ++i) {
-    std::unique_ptr<ChromeAppListItem> item =
-        std::make_unique<ChromeAppListItem>(
-            profile_.get(), GenerateId("item_id" + base::IntToString(i)),
-            model_updater());
-    item->SetPosition(last_app_position);
-    model_updater()->AddItem(std::move(item));
-    if (i < max_items_in_first_page - 2)
-      last_app_position = last_app_position.CreateAfter();
-  }
-  EXPECT_TRUE(last_app_position.CreateAfter().Equals(
-      model_updater()->GetFirstAvailablePosition()));
-
-  // Add a "page break" item at the end of first page.
-  std::unique_ptr<ChromeAppListItem> page_break_item =
-      std::make_unique<ChromeAppListItem>(
-          profile_.get(), GenerateId("page_break_item_id"), model_updater());
-  const syncer::StringOrdinal page_break_position =
-      last_app_position.CreateAfter();
-  page_break_item->SetPosition(page_break_position);
-  page_break_item->SetIsPageBreak(true);
-  model_updater()->AddItem((std::move(page_break_item)));
-  EXPECT_TRUE(last_app_position.CreateBetween(page_break_position)
-                  .Equals(model_updater()->GetFirstAvailablePosition()));
-
-  // Fill up the first page.
-  std::unique_ptr<ChromeAppListItem> app_item =
-      std::make_unique<ChromeAppListItem>(
-          profile_.get(),
-          GenerateId("item_id" + base::IntToString(max_items_in_first_page)),
-          model_updater());
-  app_item->SetPosition(last_app_position.CreateBetween(page_break_position));
-  model_updater()->AddItem(std::move(app_item));
-  EXPECT_TRUE(page_break_position.CreateAfter().Equals(
-      model_updater()->GetFirstAvailablePosition()));
-}

chrome/browser/ui/app_list/arc/arc_app_item.cc

@@ -26,7 +26,7 @@ ArcAppItem::ArcAppItem(
   if (sync_item && sync_item->item_ordinal.IsValid())
     UpdateFromSync(sync_item);
   else
-    SetDefaultPositionIfApplicable(model_updater);
+    SetDefaultPositionIfApplicable();
 
   // Set model updater last to avoid being called during construction.
   set_model_updater(model_updater);

chrome/browser/ui/app_list/chrome_app_list_item.cc

@@ -67,7 +67,8 @@ ChromeAppListItem::ChromeAppListItem(Profile* profile,
       profile_(profile),
       model_updater_(model_updater) {}
 
-ChromeAppListItem::~ChromeAppListItem() = default;
+ChromeAppListItem::~ChromeAppListItem() {
+}
 
 void ChromeAppListItem::SetIsInstalling(bool is_installing) {
   AppListModelUpdater* updater = model_updater();
@@ -145,29 +146,19 @@ void ChromeAppListItem::UpdateFromSync(
     SetName(sync_item->item_name);
 }
 
-void ChromeAppListItem::SetDefaultPositionIfApplicable(
-    AppListModelUpdater* model_updater) {
+void ChromeAppListItem::SetDefaultPositionIfApplicable() {
   syncer::StringOrdinal page_ordinal;
   syncer::StringOrdinal launch_ordinal;
   extensions::AppSorting* app_sorting = GetAppSorting();
-  if (app_sorting->GetDefaultOrdinals(id(), &page_ordinal, &launch_ordinal) &&
-      page_ordinal.IsValid() && launch_ordinal.IsValid()) {
-    // Set the default position if it exists.
-    SetPosition(syncer::StringOrdinal(page_ordinal.ToInternalValue() +
-                                      launch_ordinal.ToInternalValue()));
-    return;
-  }
-
-  if (model_updater) {
-    // Set the first available position in the app list.
-    SetPosition(model_updater->GetFirstAvailablePosition());
-    return;
+  if (!app_sorting->GetDefaultOrdinals(id(), &page_ordinal,
+                                       &launch_ordinal) ||
+      !page_ordinal.IsValid() || !launch_ordinal.IsValid()) {
+    app_sorting->EnsureValidOrdinals(id(), syncer::StringOrdinal());
+    page_ordinal = app_sorting->GetPageOrdinal(id());
+    launch_ordinal = app_sorting->GetAppLaunchOrdinal(id());
   }
-
-  // Set the natural position.
-  app_sorting->EnsureValidOrdinals(id(), syncer::StringOrdinal());
-  page_ordinal = app_sorting->GetPageOrdinal(id());
-  launch_ordinal = app_sorting->GetAppLaunchOrdinal(id());
+  DCHECK(page_ordinal.IsValid());
+  DCHECK(launch_ordinal.IsValid());
   SetPosition(syncer::StringOrdinal(page_ordinal.ToInternalValue() +
                                     launch_ordinal.ToInternalValue()));
 }

chrome/browser/ui/app_list/chrome_app_list_item.h

@@ -112,9 +112,8 @@ class ChromeAppListItem {
 
   std::string ToDebugString() const;
 
-  // Set the default position if it exists. Otherwise set the first available
-  // position in the app list if |model_updater| is not null.
-  void SetDefaultPositionIfApplicable(AppListModelUpdater* model_updater);
+  // Set the default position if it exists.
+  void SetDefaultPositionIfApplicable();
 
  protected:
   ChromeAppListItem(Profile* profile, const std::string& app_id);

chrome/browser/ui/app_list/chrome_app_list_model_updater.cc

@@ -7,7 +7,6 @@
 #include <unordered_map>
 #include <utility>
 
-#include "ash/public/cpp/app_list/app_list_config.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/app_list/app_list_client_impl.h"
 #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
@@ -373,16 +372,6 @@ void ChromeAppListModelUpdater::ContextMenuItemSelected(const std::string& id,
     chrome_item->ContextMenuItemSelected(command_id, event_flags);
 }
 
-syncer::StringOrdinal ChromeAppListModelUpdater::GetFirstAvailablePosition()
-    const {
-  std::vector<ChromeAppListItem*> top_level_items;
-  for (auto& item : items_) {
-    if (item.second->folder_id().empty())
-      top_level_items.emplace_back(item.second.get());
-  }
-  return GetFirstAvailablePositionInternal(top_level_items);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // Methods for AppListSyncableService
 

chrome/browser/ui/app_list/chrome_app_list_model_updater.h

@@ -88,7 +88,6 @@ class ChromeAppListModelUpdater : public AppListModelUpdater {
   void ContextMenuItemSelected(const std::string& id,
                                int command_id,
                                int event_flags) override;
-  syncer::StringOrdinal GetFirstAvailablePosition() const override;
 
   // Methods for AppListSyncableService:
   void AddItemToOemFolder(

chrome/browser/ui/app_list/crostini/crostini_app_item.cc

@@ -34,7 +34,7 @@ CrostiniAppItem::CrostiniAppItem(
   if (sync_item && sync_item->item_ordinal.IsValid()) {
     UpdateFromSync(sync_item);
   } else {
-    SetDefaultPositionIfApplicable(model_updater);
+    SetDefaultPositionIfApplicable();
 
     // Crostini app is created from scratch. Move it to default folder.
     DCHECK(folder_id().empty());

chrome/browser/ui/app_list/crostini/crostini_app_model_builder.cc

@@ -135,6 +135,6 @@ void CrostiniAppModelBuilder::MaybeCreateRootFolder() {
   crositini_folder->SetChromeIsFolder(true);
   crositini_folder->SetName(
       l10n_util::GetStringUTF8(IDS_APP_LIST_CROSTINI_DEFAULT_FOLDER_NAME));
-  crositini_folder->SetDefaultPositionIfApplicable(model_updater());
+  crositini_folder->SetDefaultPositionIfApplicable();
   InsertApp(std::move(crositini_folder));
 }

chrome/browser/ui/app_list/extension_app_item.cc

@@ -57,13 +57,14 @@ ExtensionAppItem::ExtensionAppItem(
   if (sync_item && sync_item->item_ordinal.IsValid())
     UpdateFromSync(sync_item);
   else
-    SetDefaultPositionIfApplicable(model_updater);
+    SetDefaultPositionIfApplicable();
 
   // Set model updater last to avoid being called during construction.
   set_model_updater(model_updater);
 }
 
-ExtensionAppItem::~ExtensionAppItem() = default;
+ExtensionAppItem::~ExtensionAppItem() {
+}
 
 void ExtensionAppItem::Reload() {
   const Extension* extension = GetExtension();
@@ -92,7 +93,8 @@ void ExtensionAppItem::OnIconUpdated(extensions::ChromeAppIcon* icon) {
 const Extension* ExtensionAppItem::GetExtension() const {
   const extensions::ExtensionRegistry* registry =
       extensions::ExtensionRegistry::Get(profile());
-  const Extension* extension = registry->GetInstalledExtension(extension_id());
+  const Extension* extension = registry->GetInstalledExtension(
+      extension_id());
   return extension;
 }
 
@@ -102,8 +104,8 @@ bool ExtensionAppItem::RunExtensionEnableFlow() {
     return false;
 
   if (!extension_enable_flow_) {
-    extension_enable_flow_ =
-        std::make_unique<ExtensionEnableFlow>(profile(), extension_id(), this);
+    extension_enable_flow_ = std::make_unique<ExtensionEnableFlow>(
+        profile(), extension_id(), this);
     extension_enable_flow_->StartForNativeWindow(nullptr);
   }
   return true;
@@ -151,7 +153,8 @@ void ExtensionAppItem::Activate(int event_flags) {
     return;
 
   extensions::RecordAppListMainLaunch(extension);
-  GetController()->ActivateApp(profile(), extension,
+  GetController()->ActivateApp(profile(),
+                               extension,
                                AppListControllerDelegate::LAUNCH_FROM_APP_LIST,
                                event_flags);
 }

chrome/browser/ui/app_list/internal_app/internal_app_item.cc

@@ -7,14 +7,14 @@
 #include "ash/public/cpp/app_list/app_list_config.h"
 #include "base/metrics/histogram_macros.h"
 #include "chrome/browser/ui/app_list/app_context_menu.h"
-#include "chrome/browser/ui/app_list/app_list_model_updater.h"
 #include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace {
 
 void RecordActiveHistogram(app_list::InternalAppName name) {
-  UMA_HISTOGRAM_ENUMERATION("Apps.AppListInternalApp.Activate", name);
+  UMA_HISTOGRAM_ENUMERATION(
+      "Apps.AppListInternalApp.Activate", name);
 }
 
 }  // namespace
@@ -24,7 +24,6 @@ const char InternalAppItem::kItemType[] = "InternalAppItem";
 
 InternalAppItem::InternalAppItem(
     Profile* profile,
-    AppListModelUpdater* model_updater,
     const app_list::AppListSyncableService::SyncItem* sync_item,
     const app_list::InternalApp& internal_app)
     : ChromeAppListItem(profile, internal_app.app_id) {
@@ -35,7 +34,7 @@ InternalAppItem::InternalAppItem(
   if (sync_item && sync_item->item_ordinal.IsValid())
     UpdateFromSync(sync_item);
   else
-    SetDefaultPositionIfApplicable(model_updater);
+    SetDefaultPositionIfApplicable();
 }
 
 InternalAppItem::~InternalAppItem() = default;

chrome/browser/ui/app_list/internal_app/internal_app_item.h

@@ -11,7 +11,7 @@
 namespace app_list {
 class AppContextMenu;
 struct InternalApp;
-}  // namespace app_list
+}
 
 // A class that represents an internal app in launcher.
 class InternalAppItem : public ChromeAppListItem {
@@ -19,7 +19,6 @@ class InternalAppItem : public ChromeAppListItem {
   static const char kItemType[];
 
   InternalAppItem(Profile* profile,
-                  AppListModelUpdater* model_updater,
                   const app_list::AppListSyncableService::SyncItem* sync_item,
                   const app_list::InternalApp& internal_app);
   ~InternalAppItem() override;

chrome/browser/ui/app_list/internal_app/internal_app_model_builder.cc

@@ -19,7 +19,6 @@ void InternalAppModelBuilder::BuildModel() {
       continue;
 
     InsertApp(std::make_unique<InternalAppItem>(
-        profile(), model_updater(), GetSyncItem(internal_app.app_id),
-        internal_app));
+        profile(), GetSyncItem(internal_app.app_id), internal_app));
   }
 }

chrome/browser/ui/app_list/test/fake_app_list_model_updater.cc

@@ -107,16 +107,6 @@ void FakeAppListModelUpdater::GetIdToAppListIndexMap(
   std::move(callback).Run(id_to_app_list_index);
 }
 
-syncer::StringOrdinal FakeAppListModelUpdater::GetFirstAvailablePosition()
-    const {
-  std::vector<ChromeAppListItem*> top_level_items;
-  for (auto& item : items_) {
-    if (item->folder_id().empty())
-      top_level_items.emplace_back(item.get());
-  }
-  return GetFirstAvailablePositionInternal(top_level_items);
-}
-
 void FakeAppListModelUpdater::GetContextMenuModel(
     const std::string& id,
     GetMenuModelCallback callback) {

chrome/browser/ui/app_list/test/fake_app_list_model_updater.h

@@ -51,7 +51,6 @@ class FakeAppListModelUpdater : public AppListModelUpdater {
   ChromeAppListItem* FindFolderItem(const std::string& folder_id) override;
   bool FindItemIndexForTest(const std::string& id, size_t* index) override;
   void GetIdToAppListIndexMap(GetIdToAppListIndexMapCallback callback) override;
-  syncer::StringOrdinal GetFirstAvailablePosition() const override;
   void GetContextMenuModel(const std::string& id,
                            GetMenuModelCallback callback) override;
   size_t BadgedItemCount() override;