SearchBoxView now enables/disables cursor based on user interaction.

ui/app_list/views/search_box_view.cc

Index: ui/app_list/views/search_box_view.cc
diff --git a/ui/app_list/views/search_box_view.cc b/ui/app_list/views/search_box_view.cc
index 7841388bd63fffa5cdfefc49a25236866a59fb02..60459cd87d10aad1653518ba7357a05b25bb48ae 100644
--- a/ui/app_list/views/search_box_view.cc
+++ b/ui/app_list/views/search_box_view.cc
@@ -18,9 +18,12 @@
 #include "ui/app_list/search_box_model.h"
 #include "ui/app_list/speech_ui_model.h"
 #include "ui/app_list/vector_icons.h"
+#include "ui/app_list/views/app_list_main_view.h"
 #include "ui/app_list/views/app_list_view.h"
 #include "ui/app_list/views/contents_view.h"
 #include "ui/app_list/views/search_box_view_delegate.h"
+#include "ui/app_list/views/search_result_container_view.h"
+#include "ui/app_list/views/search_result_page_view.h"
 #include "ui/base/ime/text_input_flags.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -62,6 +65,10 @@ constexpr int kMicIconSize = 24;
 constexpr SkColor kDefaultSearchboxColor =
     SkColorSetARGBMacro(0xDE, 0x00, 0x00, 0x00);
 
+// Color of placeholder text in zero query state.
+constexpr SkColor kZeroQuerySearchboxColor =
+    SkColorSetARGBMacro(0x8A, 0x00, 0x00, 0x00);
+
 // A background that paints a solid white rounded rect with a thin grey border.
 class SearchBoxBackground : public views::Background {
  public:
@@ -173,6 +180,8 @@ SearchBoxView::SearchBoxView(SearchBoxViewDelegate* delegate,
   search_box_->SetTextInputType(ui::TEXT_INPUT_TYPE_SEARCH);
   search_box_->SetTextInputFlags(ui::TEXT_INPUT_FLAG_AUTOCORRECT_OFF);
   if (is_fullscreen_app_list_enabled_) {
+    app_list_view_->PrependPreTargetHandler(this);

[oshima, 2017/06/29 16:58:50]

just curious. Why you need to use target handler? (because views will receive
these events)

[newcomer, 2017/06/30 21:47:23]

This has been removed.

There were some issues that I fixed in the leaf views that were preventing the
events from properly propogating to the AppListView. I'll annotate them in the
next patch.

+
     google_icon_ = new views::ImageView();
     google_icon_->SetImage(gfx::CreateVectorIcon(
         kIcGoogleBlackIcon, kGoogleIconSize, kDefaultSearchboxColor));
@@ -182,6 +191,7 @@ SearchBoxView::SearchBoxView(SearchBoxViewDelegate* delegate,
     search_box_->set_placeholder_text_draw_flags(
         gfx::Canvas::TEXT_ALIGN_CENTER);
     search_box_->SetFontList(search_box_->GetFontList().DeriveWithSizeDelta(2));
+    search_box_->SetCursorEnabled(is_search_box_active_);
   } else {
     back_button_ = new SearchBoxImageButton(this);
     ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
@@ -205,6 +215,8 @@ SearchBoxView::SearchBoxView(SearchBoxViewDelegate* delegate,
 SearchBoxView::~SearchBoxView() {
   view_delegate_->GetSpeechUI()->RemoveObserver(this);
   model_->search_box()->RemoveObserver(this);
+  if (is_fullscreen_app_list_enabled_)
+    app_list_view_->RemovePreTargetHandler(this);
 }
 
 void SearchBoxView::ModelChanged() {
@@ -229,6 +241,10 @@ void SearchBoxView::ClearSearch() {
   // does not generate ContentsChanged() notification.
   UpdateModel();
   NotifyQueryChanged();
+  if (is_fullscreen_app_list_enabled_) {
+    SetSearchBoxActive(false);
+    app_list_view_->SetStateFromSearchBoxView(true);
+  }
 }
 
 void SearchBoxView::SetShadow(const gfx::ShadowValue& shadow) {
@@ -332,6 +348,69 @@ void SearchBoxView::SetBackButtonLabel(bool folder) {
   back_button_->SetTooltipText(back_button_label);
 }
 
+void SearchBoxView::SetSearchBoxActive(bool active) {
+  if (active == is_search_box_active_)
+    return;
+
+  is_search_box_active_ = active;
+  search_box_->set_placeholder_text_draw_flags(
+      active ? gfx::Canvas::TEXT_ALIGN_LEFT : gfx::Canvas::TEXT_ALIGN_CENTER);
+  search_box_->set_placeholder_text_color(active ? kZeroQuerySearchboxColor
+                                                 : kDefaultSearchboxColor);
+  search_box_->SetCursorEnabled(active);
+  search_box_->SchedulePaint();
+}
+
+void SearchBoxView::HandleSearchBoxEvent(ui::LocatedEvent* located_event,
+                                         ui::EventType type) {
+  if (!is_fullscreen_app_list_enabled_)
+    return;
+  if (type != ui::ET_MOUSE_PRESSED && type != ui::ET_GESTURE_TAP)
+    return;
+
+  bool event_is_in_searchbox_bounds =
+      GetWidget()->GetWindowBoundsInScreen().Contains(
+          located_event->root_location());
+
+  bool event_is_in_search_result_bounds = false;
+  ContentsView* main_contents_view =
+      app_list_view_->app_list_main_view()->contents_view();
+
+  // If a search result is visible check if the event was within its bounds.
+  if (main_contents_view->IsShowingSearchResults()) {
+    View* search_result_contents_view =
+        main_contents_view->search_results_page_view()->contents_view();
+
+    const gfx::Rect search_result_bounds =
+        search_result_contents_view->GetBoundsInScreen();
+    event_is_in_search_result_bounds =
+        search_result_bounds.Contains(located_event->root_location());
+  }
+
+  if (is_search_box_active_ && !event_is_in_searchbox_bounds &&
+      !event_is_in_search_result_bounds) {
+    // If the event was not within searchbox bounds or search result bounds and
+    // the search box is active, close the app list.
+    located_event->SetHandled();
+    app_list_view_->SetState(AppListView::CLOSED);

[xiyuan, 2017/06/27 23:11:10]

It seems we want to dismiss app list when there is a unhandled (click/tap) event
hits the root AppListView. Can we do this in AppListView::OnEvent instead of
installing SearchBoxView as a pre target handler?

[newcomer, 2017/06/30 21:47:23]

Handled offline. We found the problems in the leaf views that were preventing
the events from propogating.

+  } else if (!is_search_box_active_ && event_is_in_searchbox_bounds &&
+             search_box_->text().empty()) {
+    // If the event was within the searchbox bounds and in an inactive empty
+    // search box, enable the search box.
+    SetSearchBoxActive(true);

[xiyuan, 2017/06/27 23:11:10]

This branch is probably not necessary since the events should reach us as
|search_box_| (Textfield) events in OnTextfieldEvent().

[newcomer, 2017/06/30 21:47:23]

It is necessary because there are two regions of the rendered search box, the
textfield and the background(where on mouse hover the mouse does not transition
to the cursor, but the mouse is over the white background of the searchbox).
This branch detects the latter case.

+    located_event->SetHandled();
+  }
+}
+
+bool SearchBoxView::OnTextfieldEvent() {
+  if (!is_search_box_active_) {
+    SetSearchBoxActive(true);
+    return true;
+  } else {
+    return false;
+  }
+}
+
 bool SearchBoxView::OnMouseWheel(const ui::MouseWheelEvent& event) {
   if (contents_view_)
     return contents_view_->OnMouseWheel(event);
@@ -349,6 +428,14 @@ const char* SearchBoxView::GetClassName() const {
   return "SearchBoxView";
 }
 
+void SearchBoxView::OnGestureEvent(ui::GestureEvent* event) {
+  HandleSearchBoxEvent(event->AsLocatedEvent(), event->type());

[oshima, 2017/06/29 16:58:50]

AsLocatedEvent is a utility method to downcast from Event class.

You don't need it here because GestureEvent is already LocatedEvent. (and no
need to pass type separately as the event already have it)

You can just do HandleSearchBoxEvent(event);

[newcomer, 2017/06/30 21:47:23]

Done.

+}
+
+void SearchBoxView::OnMouseEvent(ui::MouseEvent* event) {
+  HandleSearchBoxEvent(event->AsLocatedEvent(), event->type());
+}
+
 void SearchBoxView::UpdateModel() {
   // Temporarily remove from observer to ignore notifications caused by us.
   model_->search_box()->RemoveObserver(this);
@@ -367,8 +454,12 @@ void SearchBoxView::ContentsChanged(views::Textfield* sender,
   UpdateModel();
   view_delegate_->AutoLaunchCanceled();
   NotifyQueryChanged();
-  if (is_fullscreen_app_list_enabled_)
-    app_list_view_->SetStateFromSearchBoxView(search_box_->text().empty());
+  if (is_fullscreen_app_list_enabled_) {
+    if (is_search_box_active_ == search_box_->text().empty()) {
+      SetSearchBoxActive(!search_box_->text().empty());
+      app_list_view_->SetStateFromSearchBoxView(search_box_->text().empty());
+    }
+  }
 }
 
 bool SearchBoxView::HandleKeyEvent(views::Textfield* sender,
@@ -422,6 +513,22 @@ bool SearchBoxView::HandleKeyEvent(views::Textfield* sender,
   return false;
 }
 
+bool SearchBoxView::HandleMouseEvent(views::Textfield* sender,
+                                     const ui::MouseEvent& mouse_event) {
+  if (!is_fullscreen_app_list_enabled_)
+    return false;
+
+  return OnTextfieldEvent();
+}
+
+bool SearchBoxView::HandleGestureEvent(views::Textfield* sender,
+                                       const ui::GestureEvent& gesture_event) {
+  if (!is_fullscreen_app_list_enabled_)
+    return false;
+
+  return OnTextfieldEvent();
+}
+
 void SearchBoxView::ButtonPressed(views::Button* sender,
                                   const ui::Event& event) {
   if (back_button_ && sender == back_button_)