Add X button in search bar with active query

Changes:
1. Add X button in search box view.
2. Replace speech button with X button when the search box is active.
3. When X button is clicked, clear query in search box.

Screenshot:https://screenshot.googleplex.com/aNdmi8pgqFD
Specs: https://screenshot.googleplex.com/TX3rT2XXW3f

BUG=735499
TEST=app_list_unittests --gtest_filter='SearchBoxViewTest.CloseButtonTest'

Change-Id: I60490d2b9bfbabf573f66d372cd98c70a95ac787
Reviewed-on: https://chromium-review.googlesource.com/568225Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486577}

ui/app_list/vector_icons/BUILD.gn

@@ -16,6 +16,8 @@ aggregate_vector_icons("app_list_vector_icons") {
     "ic_badge_play.icon",
     "ic_badge_rating.1x.icon",
     "ic_badge_rating.icon",
+    "ic_close.1x.icon",
+    "ic_close.icon",
     "ic_google_black.1x.icon",
     "ic_google_black.icon",
     "ic_mic_black.1x.icon",

ui/app_list/vector_icons/ic_close.1x.icon

+// Copyright 2017 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.
+
+CANVAS_DIMENSIONS, 24,
+MOVE_TO, 19, 6.41f,
+LINE_TO, 17.59f, 5,
+LINE_TO, 12, 10.59f,
+LINE_TO, 6.41f, 5,
+LINE_TO, 5, 6.41f,
+LINE_TO, 10.59f, 12,
+LINE_TO, 5, 17.59f,
+LINE_TO, 6.41f, 19,
+LINE_TO, 12, 13.41f,
+LINE_TO, 17.59f, 19,
+LINE_TO, 19, 17.59f,
+LINE_TO, 13.41f, 12,
+CLOSE,
+END

ui/app_list/vector_icons/ic_close.icon

+// Copyright 2017 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.
+
+CANVAS_DIMENSIONS, 48,
+MOVE_TO, 38, 12.82f,
+LINE_TO, 35.18f, 10,
+LINE_TO, 24, 21.18f,
+LINE_TO, 12.82f, 10,
+LINE_TO, 10, 12.82f,
+LINE_TO, 21.18f, 24,
+LINE_TO, 10, 35.18f,
+LINE_TO, 12.82f, 38,
+LINE_TO, 24, 26.82f,
+LINE_TO, 35.18f, 38,
+LINE_TO, 38, 35.18f,
+LINE_TO, 26.82f, 24,
+CLOSE,
+END

ui/app_list/views/search_box_view.cc

@@ -62,6 +62,7 @@ constexpr int kBackgroundBorderCornerRadiusFullscreen = 24;
 constexpr int kBackgroundBorderCornerRadiusSearchResult = 4;
 constexpr int kGoogleIconSize = 24;
 constexpr int kMicIconSize = 24;
+constexpr int kCloseIconSize = 24;
 
 // Default color used when wallpaper customized color is not available for
 // searchbox, #000 at 87% opacity.
@@ -171,13 +172,8 @@ SearchBoxView::SearchBoxView(SearchBoxViewDelegate* delegate,
                              AppListView* app_list_view)
     : delegate_(delegate),
       view_delegate_(view_delegate),
-      model_(nullptr),
       content_container_(new views::View),
-      google_icon_(nullptr),
-      back_button_(nullptr),
-      speech_button_(nullptr),
       search_box_(new views::Textfield),
-      contents_view_(nullptr),
       app_list_view_(app_list_view),
       focused_view_(FOCUS_SEARCH_BOX),
       is_fullscreen_app_list_enabled_(features::IsFullscreenAppListEnabled()) {
@@ -233,6 +229,15 @@ SearchBoxView::SearchBoxView(SearchBoxViewDelegate* delegate,
   content_container_->AddChildView(search_box_);
   layout->SetFlexForView(search_box_, 1);
 
+  if (is_fullscreen_app_list_enabled_) {
+    close_button_ = new SearchBoxImageButton(this);
+    close_button_->SetImage(views::ImageButton::STATE_NORMAL,
+                            gfx::CreateVectorIcon(kIcCloseIcon, kCloseIconSize,
+                                                  kDefaultSearchboxColor));
+    close_button_->SetVisible(false);
+    content_container_->AddChildView(close_button_);
+  }
+
   view_delegate_->GetSpeechUI()->AddObserver(this);
   ModelChanged();
 }
@@ -288,6 +293,10 @@ views::ImageButton* SearchBoxView::back_button() {
   return static_cast<views::ImageButton*>(back_button_);
 }
 
+bool SearchBoxView::IsCloseButtonVisible() const {
+  return close_button_ && close_button_->visible();
+}
+
 // Returns true if set internally, i.e. if focused_view_ != CONTENTS_VIEW.
 // Note: because we always want to be able to type in the edit box, this is only
 // a faux-focus so that buttons can respond to the ENTER key.
@@ -296,6 +305,8 @@ bool SearchBoxView::MoveTabFocus(bool move_backwards) {
     back_button_->SetSelected(false);
   if (speech_button_)
     speech_button_->SetSelected(false);
+  if (close_button_)
+    close_button_->SetSelected(false);
 
   switch (focused_view_) {
     case FOCUS_BACK_BUTTON:
@@ -309,18 +320,23 @@ bool SearchBoxView::MoveTabFocus(bool move_backwards) {
       } else {
         focused_view_ = speech_button_ && speech_button_->visible()
                             ? FOCUS_MIC_BUTTON
-                            : FOCUS_CONTENTS_VIEW;
+                            : (close_button_ && close_button_->visible()
+                                   ? FOCUS_CLOSE_BUTTON
+                                   : FOCUS_CONTENTS_VIEW);
       }
       break;
     case FOCUS_MIC_BUTTON:
+    case FOCUS_CLOSE_BUTTON:
       focused_view_ = move_backwards ? FOCUS_SEARCH_BOX : FOCUS_CONTENTS_VIEW;
       break;
     case FOCUS_CONTENTS_VIEW:
-      focused_view_ =
-          move_backwards
-              ? (speech_button_ && speech_button_->visible() ? FOCUS_MIC_BUTTON
-                                                             : FOCUS_SEARCH_BOX)
-              : FOCUS_CONTENTS_VIEW;
+      if (move_backwards) {
+        focused_view_ = speech_button_ && speech_button_->visible()
+                            ? FOCUS_MIC_BUTTON
+                            : (close_button_ && close_button_->visible()
+                                   ? FOCUS_CLOSE_BUTTON
+                                   : FOCUS_SEARCH_BOX);
+      }
       break;
     default:
       DCHECK(false);
@@ -344,6 +360,10 @@ bool SearchBoxView::MoveTabFocus(bool move_backwards) {
       if (speech_button_)
         speech_button_->SetSelected(true);
       break;
+    case FOCUS_CLOSE_BUTTON:
+      if (close_button_)
+        close_button_->SetSelected(true);
+      break;
     default:
       break;
   }
@@ -359,6 +379,8 @@ void SearchBoxView::ResetTabFocus(bool on_contents) {
     back_button_->SetSelected(false);
   if (speech_button_)
     speech_button_->SetSelected(false);
+  if (close_button_)
+    close_button_->SetSelected(false);
   focused_view_ = on_contents ? FOCUS_CONTENTS_VIEW : FOCUS_SEARCH_BOX;
 }
 
@@ -396,6 +418,11 @@ void SearchBoxView::SetSearchBoxActive(bool active) {
                                                  : kDefaultSearchboxColor);
   search_box_->SetCursorEnabled(active);
   search_box_->SchedulePaint();
+
+  if (speech_button_)
+    speech_button_->SetVisible(!active);
+  close_button_->SetVisible(active);
+  content_container_->Layout();
 }
 
 void SearchBoxView::HandleSearchBoxEvent(ui::LocatedEvent* located_event) {
@@ -441,6 +468,8 @@ void SearchBoxView::OnEnabledChanged() {
   search_box_->SetEnabled(enabled());
   if (speech_button_)
     speech_button_->SetEnabled(enabled());
+  if (close_button_)
+    close_button_->SetEnabled(enabled());
 }
 
 const char* SearchBoxView::GetClassName() const {
@@ -511,6 +540,10 @@ bool SearchBoxView::HandleKeyEvent(views::Textfield* sender,
         speech_button_->OnKeyPressed(key_event))
       return true;
 
+    if (focused_view_ == FOCUS_CLOSE_BUTTON && close_button_ &&
+        close_button_->OnKeyPressed(key_event))
+      return true;
+
     const bool handled = contents_view_ && contents_view_->visible() &&
                          contents_view_->OnKeyPressed(key_event);
 
@@ -539,6 +572,10 @@ bool SearchBoxView::HandleKeyEvent(views::Textfield* sender,
         speech_button_->OnKeyReleased(key_event))
       return true;
 
+    if (focused_view_ == FOCUS_CLOSE_BUTTON && close_button_ &&
+        close_button_->OnKeyReleased(key_event))
+      return true;
+
     return contents_view_ && contents_view_->visible() &&
            contents_view_->OnKeyReleased(key_event);
   }
@@ -562,6 +599,8 @@ void SearchBoxView::ButtonPressed(views::Button* sender,
     delegate_->BackButtonPressed();
   else if (speech_button_ && sender == speech_button_)
     view_delegate_->StartSpeechRecognition();
+  else if (close_button_ && sender == close_button_)
+    ClearSearch();
   else
     NOTREACHED();
 }
@@ -602,7 +641,7 @@ void SearchBoxView::SpeechRecognitionButtonPropChanged() {
     if (speech_button_) {
       // Deleting a view will detach it from its parent.
       delete speech_button_;
-      speech_button_ = NULL;
+      speech_button_ = nullptr;
     }
   }
   Layout();
@@ -645,6 +684,11 @@ void SearchBoxView::WallpaperProminentColorsChanged() {
       gfx::CreateVectorIcon(
           kIcMicBlackIcon, kMicIconSize,
           dark_muted_available ? dark_muted : kDefaultSearchboxColor));
+  close_button_->SetImage(
+      views::Button::STATE_NORMAL,
+      gfx::CreateVectorIcon(
+          kIcCloseIcon, kCloseIconSize,
+          dark_muted_available ? dark_muted : kDefaultSearchboxColor));
   search_box_->set_placeholder_text_color(
       dark_muted_available ? dark_muted : kDefaultSearchboxColor);
 

ui/app_list/views/search_box_view.h

@@ -28,6 +28,7 @@ enum SearchBoxFocus {
   FOCUS_BACK_BUTTON,    // Back button, only responds to ENTER
   FOCUS_SEARCH_BOX,     // Nothing else has partial focus
   FOCUS_MIC_BUTTON,     // Mic button, only responds to ENTER
+  FOCUS_CLOSE_BUTTON,   // Close button, only responds to ENTER
   FOCUS_CONTENTS_VIEW,  // Something outside the SearchBox is selected
 };
 
@@ -67,6 +68,7 @@ class APP_LIST_EXPORT SearchBoxView : public views::View,
 
   views::ImageButton* back_button();
   views::Textfield* search_box() { return search_box_; }
+  bool IsCloseButtonVisible() const;
 
   void set_contents_view(views::View* contents_view) {
     contents_view_ = contents_view;
@@ -143,15 +145,17 @@ class APP_LIST_EXPORT SearchBoxView : public views::View,
 
   SearchBoxViewDelegate* delegate_;     // Not owned.
   AppListViewDelegate* view_delegate_;  // Not owned.
-  AppListModel* model_;                 // Owned by the profile-keyed service.
-
-  views::View* content_container_;        // Owned by views hierarchy.
-  views::ImageView* google_icon_;         // Owned by views hierarchy.
-  SearchBoxImageButton* back_button_;     // Owned by views hierarchy.
-  SearchBoxImageButton* speech_button_;   // Owned by views hierarchy.
-  views::Textfield* search_box_;          // Owned by views hierarchy.
-  views::View* contents_view_;            // Owned by views hierarchy.
-  app_list::AppListView* app_list_view_;  // Owned by views hierarchy.
+  AppListModel* model_ = nullptr;       // Owned by the profile-keyed service.
+
+  // Owned by views hierarchy.
+  views::View* content_container_;
+  views::ImageView* google_icon_ = nullptr;
+  SearchBoxImageButton* back_button_ = nullptr;
+  SearchBoxImageButton* speech_button_ = nullptr;
+  SearchBoxImageButton* close_button_ = nullptr;
+  views::Textfield* search_box_;
+  views::View* contents_view_ = nullptr;
+  app_list::AppListView* app_list_view_;
 
   SearchBoxFocus focused_view_;  // Which element has TAB'd focus.
 

ui/app_list/views/search_box_view_unittest.cc

@@ -9,7 +9,10 @@
 
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
+#include "ui/app_list/app_list_features.h"
 #include "ui/app_list/test/app_list_test_view_delegate.h"
+#include "ui/app_list/views/app_list_view.h"
 #include "ui/app_list/views/search_box_view_delegate.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/test/widget_test.h"
@@ -43,16 +46,31 @@ class KeyPressCounterView : public views::View {
 };
 
 class SearchBoxViewTest : public views::test::WidgetTest,
-                          public SearchBoxViewDelegate {
+                          public SearchBoxViewDelegate,
+                          public testing::WithParamInterface<bool> {
  public:
-  SearchBoxViewTest() : query_changed_count_(0) {}
-  ~SearchBoxViewTest() override {}
+  SearchBoxViewTest() = default;
+  ~SearchBoxViewTest() override = default;
 
   // Overridden from testing::Test:
   void SetUp() override {
     views::test::WidgetTest::SetUp();
+
+    if (testing::UnitTest::GetInstance()->current_test_info()->value_param()) {
+      // Current test is parameterized.
+      test_with_fullscreen_ = GetParam();
+      if (test_with_fullscreen_) {
+        scoped_feature_list_.InitAndEnableFeature(
+            features::kEnableFullscreenAppList);
+      }
+    }
+
+    gfx::NativeView parent = GetContext();
+    app_list_view_ = new AppListView(&view_delegate_);
+    app_list_view_->Initialize(parent, 0, false, false);
+
     widget_ = CreateTopLevelPlatformWidget();
-    view_ = new SearchBoxView(this, &view_delegate_);
+    view_ = new SearchBoxView(this, &view_delegate_, app_list_view_);
     counter_view_ = new KeyPressCounterView();
     widget_->GetContentsView()->AddChildView(view_);
     widget_->GetContentsView()->AddChildView(counter_view_);
@@ -60,6 +78,7 @@ class SearchBoxViewTest : public views::test::WidgetTest,
   }
 
   void TearDown() override {
+    app_list_view_->GetWidget()->Close();
     widget_->CloseNow();
     views::test::WidgetTest::TearDown();
   }
@@ -67,6 +86,8 @@ class SearchBoxViewTest : public views::test::WidgetTest,
  protected:
   SearchBoxView* view() { return view_; }
 
+  bool test_with_fullscreen() { return test_with_fullscreen_; }
+
   void SetLongAutoLaunchTimeout() {
     // Sets a long timeout that lasts longer than the test run.
     view_delegate_.set_auto_launch_timeout(base::TimeDelta::FromDays(1));
@@ -120,14 +141,21 @@ class SearchBoxViewTest : public views::test::WidgetTest,
   AppListTestViewDelegate view_delegate_;
   views::Widget* widget_;
   SearchBoxView* view_;
+  AppListView* app_list_view_ = nullptr;
   KeyPressCounterView* counter_view_;
   base::string16 last_query_;
-  int query_changed_count_;
+  int query_changed_count_ = 0;
+  bool test_with_fullscreen_ = false;
+  base::test::ScopedFeatureList scoped_feature_list_;
 
   DISALLOW_COPY_AND_ASSIGN(SearchBoxViewTest);
 };
 
-TEST_F(SearchBoxViewTest, Basic) {
+// Instantiate the Boolean which is used to toggle the Fullscreen app list in
+// the parameterized tests.
+INSTANTIATE_TEST_CASE_P(, SearchBoxViewTest, testing::Bool());
+
+TEST_P(SearchBoxViewTest, Basic) {
   KeyPress(ui::VKEY_A);
   EXPECT_EQ("a", GetLastQueryAndReset());
   EXPECT_EQ(1, GetQueryChangedCountAndReset());
@@ -142,7 +170,7 @@ TEST_F(SearchBoxViewTest, Basic) {
   EXPECT_TRUE(GetLastQueryAndReset().empty());
 }
 
-TEST_F(SearchBoxViewTest, CancelAutoLaunch) {
+TEST_P(SearchBoxViewTest, CancelAutoLaunch) {
   SetLongAutoLaunchTimeout();
   ASSERT_NE(base::TimeDelta(), GetAutoLaunchTimeout());
 
@@ -164,5 +192,16 @@ TEST_F(SearchBoxViewTest, CancelAutoLaunch) {
   EXPECT_EQ(base::TimeDelta(), GetAutoLaunchTimeout());
 }
 
+TEST_P(SearchBoxViewTest, CloseButtonTest) {
+  if (!test_with_fullscreen())
+    return;
+
+  KeyPress(ui::VKEY_A);
+  EXPECT_TRUE(view()->IsCloseButtonVisible());
+
+  view()->ClearSearch();
+  EXPECT_FALSE(view()->IsCloseButtonVisible());
+}
+
 }  // namespace test
 }  // namespace app_list