diff options
author | Matt Giuca <mgiuca@chromium.org> | 2015-03-04 16:16:53 +1100 |
---|---|---|
committer | Matt Giuca <mgiuca@chromium.org> | 2015-03-04 05:18:19 +0000 |
commit | 548e4b38503f6aec3f473391490a939b38e80dc2 (patch) | |
tree | 2093acf09b9406f2478c81dde8bb442fa421db77 | |
parent | c23a60d352afdaa200dc92689e6b504b184c2ab8 (diff) | |
download | chromium_src-548e4b38503f6aec3f473391490a939b38e80dc2.zip chromium_src-548e4b38503f6aec3f473391490a939b38e80dc2.tar.gz chromium_src-548e4b38503f6aec3f473391490a939b38e80dc2.tar.bz2 |
Fix sort order of unlaunched apps on app list start page.
This CL fixes an issue with the app list start page sort order for
unlaunched apps. Folder items were being weighted too highly, causing
them to appear first. The items that are not in a folder will now
appear first in the recommendations, in app list order.
BUG=455338
Review URL: https://codereview.chromium.org/954363002
Cr-Commit-Position: refs/heads/master@{#318642}
(cherry picked from commit 0f1ee572dcbbd4a283bccafe07ac071092003357)
TBR=mgiuca@chromium.org
Review URL: https://codereview.chromium.org/977823002
Cr-Commit-Position: refs/branch-heads/2311@{#123}
Cr-Branched-From: 09b7de5dd7254947cd4306de907274fa63373d48-refs/heads/master@{#317474}
-rw-r--r-- | chrome/browser/ui/app_list/search/app_search_provider.cc | 13 | ||||
-rw-r--r-- | chrome/browser/ui/app_list/search/app_search_provider_unittest.cc | 14 |
2 files changed, 24 insertions, 3 deletions
diff --git a/chrome/browser/ui/app_list/search/app_search_provider.cc b/chrome/browser/ui/app_list/search/app_search_provider.cc index b81afa6..5009b3a 100644 --- a/chrome/browser/ui/app_list/search/app_search_provider.cc +++ b/chrome/browser/ui/app_list/search/app_search_provider.cc @@ -112,9 +112,16 @@ void AppSearchProvider::UpdateResults() { // Use the app list order to tiebreak apps that have never been launched. if (app->last_launch_time().is_null()) { - result->set_relevance( - kUnlaunchedAppRelevanceStepSize * - (apps_.size() - id_to_app_list_index[app->app_id()])); + auto it = id_to_app_list_index.find(app->app_id()); + // If it's in a folder, it won't be in |id_to_app_list_index|. Rank + // those as if they are at the end of the list. + size_t app_list_index = + it == id_to_app_list_index.end() ? apps_.size() : (*it).second; + if (app_list_index > apps_.size()) + app_list_index = apps_.size(); + + result->set_relevance(kUnlaunchedAppRelevanceStepSize * + (apps_.size() - app_list_index)); } else { result->UpdateFromLastLaunched(clock_->Now(), app->last_launch_time()); } diff --git a/chrome/browser/ui/app_list/search/app_search_provider_unittest.cc b/chrome/browser/ui/app_list/search/app_search_provider_unittest.cc index 8fb6188..01c506d 100644 --- a/chrome/browser/ui/app_list/search/app_search_provider_unittest.cc +++ b/chrome/browser/ui/app_list/search/app_search_provider_unittest.cc @@ -21,6 +21,7 @@ #include "extensions/common/extension_set.h" #include "sync/api/string_ordinal.h" #include "testing/gtest/include/gtest/gtest.h" +#include "ui/app_list/app_list_folder_item.h" #include "ui/app_list/app_list_item.h" #include "ui/app_list/app_list_model.h" #include "ui/app_list/search_result.h" @@ -158,6 +159,7 @@ TEST_F(AppSearchProviderTest, FetchRecommendations) { } TEST_F(AppSearchProviderTest, FetchUnlaunchedRecommendations) { + const char kFolderId[] = "folder1"; extensions::ExtensionPrefs* prefs = extensions::ExtensionPrefs::Get(profile_.get()); @@ -174,6 +176,18 @@ TEST_F(AppSearchProviderTest, FetchUnlaunchedRecommendations) { model()->FindItem(kPackagedApp2Id), model()->FindItem(kPackagedApp1Id)->position().CreateBefore()); EXPECT_EQ("Hosted App,Packaged App 2,Packaged App 1", RunQuery("")); + + // Moving an app into a folder deprioritizes it. + model()->AddItem(scoped_ptr<AppListFolderItem>( + new AppListFolderItem(kFolderId, AppListFolderItem::FOLDER_TYPE_NORMAL))); + model()->MoveItemToFolder(model()->FindItem(kPackagedApp2Id), kFolderId); + EXPECT_EQ("Hosted App,Packaged App 1,Packaged App 2", RunQuery("")); + + // The position of the folder shouldn't matter. + model()->SetItemPosition( + model()->FindItem(kFolderId), + model()->FindItem(kPackagedApp1Id)->position().CreateBefore()); + EXPECT_EQ("Hosted App,Packaged App 1,Packaged App 2", RunQuery("")); } } // namespace test |