Make a fixed ordering of launcher container views

There are three container views within the launcher search box: app
tiles, results list, and answer card. Currently, the order of these
containers in the UI is dynamic: it changes depending on the display
scores of the results within each container.

In practice though, we always want one ordering: app tiles, answer card,
then results list. But the dependence on result scores is a source of
bugs, because ML ranking can significantly change the UI.

This CL fixes the container scores to prevent these kinds of bugs in
future.

Bug: 1028447
Change-Id: I2217fc9c1f8715b7073f1afd976e1a1b87343e04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2345924Reviewed-by: Jenny Zhang <jennyz@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Tony Yeoman <tby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797569}

ash/app_list/views/search_box_view_unittest.cc

@@ -825,7 +825,7 @@ INSTANTIATE_TEST_SUITE_P(All,
 // titles.
 TEST_F(SearchBoxViewAutocompleteTest,
        SearchBoxAutocompletesTopListResultTitle) {
-  // Add two SearchResults, one with higher ranking. Initialize their title
+  // Add two SearchResults, one tile and one list result. Initialize their title
   // field to a non-empty string.
   CreateSearchResult(ash::SearchResultDisplayType::kList, 1.0,
                      base::ASCIIToUTF16("hello list"), base::string16());
@@ -838,16 +838,16 @@ TEST_F(SearchBoxViewAutocompleteTest,
   KeyPress(ui::VKEY_E);
   view()->ProcessAutocomplete();
 
-  EXPECT_EQ(view()->search_box()->GetText(), base::ASCIIToUTF16("hello list"));
+  EXPECT_EQ(view()->search_box()->GetText(), base::ASCIIToUTF16("hello tile"));
   EXPECT_EQ(view()->search_box()->GetSelectedText(),
-            base::ASCIIToUTF16("llo list"));
+            base::ASCIIToUTF16("llo tile"));
 }
 
 // Tests that autocomplete suggestions are consistent with top SearchResult tile
 // titles.
 TEST_F(SearchBoxViewAutocompleteTest,
        SearchBoxAutocompletesTopTileResultTitle) {
-  // Add two SearchResults, one with higher ranking. Initialize their title
+  // Add two SearchResults, one tile and one list result. Initialize their title
   // field to a non-empty string.
   CreateSearchResult(ash::SearchResultDisplayType::kTile, 1.0,
                      base::ASCIIToUTF16("hello tile"), base::string16());
@@ -868,8 +868,9 @@ TEST_F(SearchBoxViewAutocompleteTest,
 // details.
 TEST_F(SearchBoxViewAutocompleteTest,
        SearchBoxAutocompletesTopListResultDetails) {
-  // Add two SearchResults, one with higher ranking. Initialize their details
-  // field to a non-empty string.
+  // Add two SearchResults, one tile and one list result. The tile should
+  // display first, despite having a lower score. Initialize their details field
+  // to a non-empty string.
   CreateSearchResult(ash::SearchResultDisplayType::kList, 1.0, base::string16(),
                      base::ASCIIToUTF16("hello list"));
   CreateSearchResult(ash::SearchResultDisplayType::kTile, 0.5, base::string16(),
@@ -880,17 +881,17 @@ TEST_F(SearchBoxViewAutocompleteTest,
   KeyPress(ui::VKEY_H);
   KeyPress(ui::VKEY_E);
   view()->ProcessAutocomplete();
-  EXPECT_EQ(view()->search_box()->GetText(), base::ASCIIToUTF16("hello list"));
+  EXPECT_EQ(view()->search_box()->GetText(), base::ASCIIToUTF16("hello tile"));
   EXPECT_EQ(view()->search_box()->GetSelectedText(),
-            base::ASCIIToUTF16("llo list"));
+            base::ASCIIToUTF16("llo tile"));
 }
 
 // Tests that autocomplete suggestions are consistent with top SearchResult tile
 // details.
 TEST_F(SearchBoxViewAutocompleteTest,
        SearchBoxAutocompletesTopTileResultDetails) {
-  // Add two SearchResults, one with higher ranking. Initialize their details
-  // field to a non-empty string.
+  // Add two SearchResults, one tile and one list result. Initialize their
+  // details field to a non-empty string.
   CreateSearchResult(ash::SearchResultDisplayType::kTile, 1.0, base::string16(),
                      base::ASCIIToUTF16("hello tile"));
   CreateSearchResult(ash::SearchResultDisplayType::kList, 0.5, base::string16(),

ash/app_list/views/search_result_answer_card_view.cc

@@ -5,6 +5,7 @@
 #include "ash/app_list/views/search_result_answer_card_view.h"
 
 #include <memory>
+#include <string>
 #include <utility>
 #include <vector>
 
@@ -12,6 +13,7 @@
 #include "ash/app_list/app_list_view_delegate.h"
 #include "ash/app_list/views/app_list_view.h"
 #include "ash/app_list/views/search_result_base_view.h"
+#include "ash/public/cpp/app_list/app_list_config.h"
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
@@ -351,7 +353,9 @@ int SearchResultAnswerCardView::DoUpdate() {
   parent()->SetVisible(has_valid_answer_card);
 
   set_container_score(
-      has_valid_answer_card && top_result ? top_result->display_score() : -1);
+      has_valid_answer_card && top_result
+          ? AppListConfig::instance().answer_card_container_score()
+          : -1.0);
   if (top_result)
     top_result->set_is_visible(has_valid_answer_card);
 

ash/app_list/views/search_result_answer_card_view_unittest.cc

@@ -22,7 +22,7 @@ namespace test {
 
 namespace {
 constexpr char kResultTitle[] = "The weather is fine";
-constexpr double kDisplayScore = 13.0;
+constexpr double kDisplayScore = 2.0;
 }  // namespace
 
 class SearchResultAnswerCardViewTest : public views::ViewsTestBase {

ash/app_list/views/search_result_list_view.cc

@@ -284,7 +284,9 @@ int SearchResultListView::DoUpdate() {
   previous_found_drive_quick_access_ = found_drive_quick_access;
 
   set_container_score(
-      display_results.empty() ? -1 : display_results.front()->display_score());
+      display_results.empty()
+          ? -1.0
+          : AppListConfig::instance().results_list_container_score());
 
   return display_results.size();
 }

ash/app_list/views/search_result_page_view_unittest.cc

@@ -153,17 +153,15 @@ TEST_P(SearchResultPageViewTest, ResultsSorted) {
   EXPECT_EQ(tile_list_view(), view()->result_container_views()[0]);
   EXPECT_EQ(list_view(), view()->result_container_views()[1]);
 
-  // Change the relevance of the tile result and expect the list results to be
-  // displayed first.
-  // TODO(warx): fullscreen launcher should always have tile list view to be
-  // displayed first over list view.
+  // Change the relevance of the tile result to be lower than list results. The
+  // tile container should still be displayed first.
   tile_result->set_display_score(0.4);
 
   results->NotifyItemsChanged(0, 1);
   RunPendingMessages();
 
-  EXPECT_EQ(list_view(), view()->result_container_views()[0]);
-  EXPECT_EQ(tile_list_view(), view()->result_container_views()[1]);
+  EXPECT_EQ(tile_list_view(), view()->result_container_views()[0]);
+  EXPECT_EQ(list_view(), view()->result_container_views()[1]);
 }
 
 TEST_P(SearchResultPageViewTest, TileResultsSortedBeforeEmptyListResults) {

ash/app_list/views/search_result_tile_item_list_view.cc

@@ -253,7 +253,9 @@ int SearchResultTileItemListView::DoUpdate() {
   }
 
   set_container_score(
-      display_results.empty() ? -1 : display_results.front()->display_score());
+      display_results.empty()
+          ? -1.0
+          : AppListConfig::instance().app_tiles_container_score());
 
   return display_results.size();
 }

ash/public/cpp/app_list/app_list_config.h

@@ -176,6 +176,16 @@ class ASH_PUBLIC_EXPORT AppListConfig {
     return max_search_result_list_items_;
   }
 
+  double app_tiles_container_score() const {
+    return app_tiles_container_score_;
+  }
+  double results_list_container_score() const {
+    return results_list_container_score_;
+  }
+  double answer_card_container_score() const {
+    return answer_card_container_score_;
+  }
+
   gfx::Size grid_icon_size() const {
     return gfx::Size(grid_icon_dimension_, grid_icon_dimension_);
   }
@@ -462,6 +472,12 @@ class ASH_PUBLIC_EXPORT AppListConfig {
   // Max number of search result list items in the launcher suggestion window.
   const size_t max_search_result_list_items_ = 5;
 
+  // Scores for the three containers within the search box view. These are
+  // displayed in high-to-low order.
+  const double app_tiles_container_score_ = 3.0;
+  const double answer_card_container_score_ = 2.0;
+  const double results_list_container_score_ = 1.0;
+
   // Cardified app list background properties
   const SkColor cardified_background_color_;
   const SkColor cardified_background_color_active_;