cros: Reorder functions in search_box_view

This cl reorders the functions in search_box_view to make the
readability better.

Bug: 828603
Test: manual.
Change-Id: Ic2fa5c7bb0852504a4b1056d66f60ed7800c9c66
Reviewed-on: https://chromium-review.googlesource.com/994134Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547964}

ash/components/shortcut_viewer/views/ksv_search_box_view.h

@@ -33,8 +33,8 @@ class KSVSearchBoxView : public search_box::SearchBoxViewBase {
 
  private:
   // search_box::SearchBoxViewBase:
-  void ModelChanged() override {}
   void UpdateBackgroundColor(SkColor color) override;
+  void ModelChanged() override {}
   void UpdateKeyboardVisibility() override {}
   void UpdateModel(bool initiated_by_user) override {}
   void UpdateSearchIcon() override {}

ui/app_list/views/search_box_view.cc

@@ -88,6 +88,28 @@ SearchBoxView::~SearchBoxView() {
   view_delegate_->RemoveObserver(this);
 }
 
+void SearchBoxView::ClearSearch() {
+  search_box::SearchBoxViewBase::ClearSearch();
+  app_list_view_->SetStateFromSearchBoxView(true);
+}
+
+views::View* SearchBoxView::GetSelectedViewInContentsView() {
+  if (!contents_view())
+    return nullptr;
+  return static_cast<ContentsView*>(contents_view())->GetSelectedView();
+}
+
+void SearchBoxView::HandleSearchBoxEvent(ui::LocatedEvent* located_event) {
+  if (located_event->type() == ui::ET_MOUSEWHEEL) {
+    if (!app_list_view_->HandleScroll(
+            located_event->AsMouseWheelEvent()->offset().y(),
+            ui::ET_MOUSEWHEEL)) {
+      return;
+    }
+  }
+  search_box::SearchBoxViewBase::HandleSearchBoxEvent(located_event);
+}
+
 void SearchBoxView::ModelChanged() {
   if (search_model_)
     search_model_->search_box()->RemoveObserver(this);
@@ -101,11 +123,6 @@ void SearchBoxView::ModelChanged() {
   OnWallpaperColorsChanged();
 }
 
-void SearchBoxView::ClearSearch() {
-  search_box::SearchBoxViewBase::ClearSearch();
-  app_list_view_->SetStateFromSearchBoxView(true);
-}
-
 void SearchBoxView::UpdateKeyboardVisibility() {
   if (!is_tablet_mode())
     return;
@@ -126,15 +143,65 @@ void SearchBoxView::UpdateKeyboardVisibility() {
       keyboard::KeyboardController::HIDE_REASON_MANUAL);
 }
 
-void SearchBoxView::HandleSearchBoxEvent(ui::LocatedEvent* located_event) {
-  if (located_event->type() == ui::ET_MOUSEWHEEL) {
-    if (!app_list_view_->HandleScroll(
-            located_event->AsMouseWheelEvent()->offset().y(),
-            ui::ET_MOUSEWHEEL)) {
-      return;
-    }
+void SearchBoxView::UpdateModel(bool initiated_by_user) {
+  // Temporarily remove from observer to ignore notifications caused by us.
+  search_model_->search_box()->RemoveObserver(this);
+  search_model_->search_box()->Update(search_box()->text(), initiated_by_user);
+  search_model_->search_box()->SetSelectionModel(
+      search_box()->GetSelectionModel());
+  search_model_->search_box()->AddObserver(this);
+}
+
+void SearchBoxView::UpdateSearchIcon() {
+  const gfx::VectorIcon& google_icon =
+      is_search_box_active() ? kIcGoogleColorIcon : kIcGoogleBlackIcon;
+  const gfx::VectorIcon& icon = search_model_->search_engine_is_google()
+                                    ? google_icon
+                                    : kIcSearchEngineNotGoogleIcon;
+  SetSearchIconImage(gfx::CreateVectorIcon(icon, search_box::kSearchIconSize,
+                                           search_box_color()));
+}
+
+void SearchBoxView::UpdateSearchBoxBorder() {
+  if (search_box()->HasFocus() && !is_search_box_active()) {
+    // Show a gray ring around search box to indicate that the search box is
+    // selected. Do not show it when search box is active, because blinking
+    // cursor already indicates that.
+    SetBorder(views::CreateRoundedRectBorder(kSearchBoxBorderWidth,
+                                             kSearchBoxFocusBorderCornerRadius,
+                                             kSearchBoxBorderColor));
+    return;
   }
-  search_box::SearchBoxViewBase::HandleSearchBoxEvent(located_event);
+
+  // Creates an empty border as a placeholder for colored border so that
+  // re-layout won't move views below the search box.
+  SetBorder(
+      views::CreateEmptyBorder(kSearchBoxBorderWidth, kSearchBoxBorderWidth,
+                               kSearchBoxBorderWidth, kSearchBoxBorderWidth));
+}
+
+void SearchBoxView::SetupCloseButton() {
+  views::ImageButton* close = close_button();
+  close->SetImage(views::ImageButton::STATE_NORMAL,
+                  gfx::CreateVectorIcon(views::kIcCloseIcon, kCloseIconSize,
+                                        search_box_color()));
+  close->SetVisible(false);
+  close->SetAccessibleName(
+      l10n_util::GetStringUTF16(IDS_APP_LIST_CLEAR_SEARCHBOX));
+}
+
+void SearchBoxView::SetupBackButton() {
+  views::ImageButton* back = back_button();
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  back->SetImage(views::ImageButton::STATE_NORMAL,
+                 rb.GetImageSkiaNamed(IDR_APP_LIST_FOLDER_BACK_NORMAL));
+  back->SetImageAlignment(views::ImageButton::ALIGN_CENTER,
+                          views::ImageButton::ALIGN_MIDDLE);
+  back->SetVisible(false);
+  base::string16 back_button_label(
+      l10n_util::GetStringUTF16(IDS_APP_LIST_BACK));
+  back->SetAccessibleName(back_button_label);
+  back->SetTooltipText(back_button_label);
 }
 
 void SearchBoxView::OnKeyEvent(ui::KeyEvent* event) {
@@ -227,36 +294,32 @@ void SearchBoxView::UpdateOpacity() {
       should_restore_opacity ? 1.0f : opacity);
 }
 
-views::View* SearchBoxView::GetSelectedViewInContentsView() {
-  if (!contents_view())
-    return nullptr;
-  return static_cast<ContentsView*>(contents_view())->GetSelectedView();
-}
-
-void SearchBoxView::UpdateModel(bool initiated_by_user) {
-  // Temporarily remove from observer to ignore notifications caused by us.
-  search_model_->search_box()->RemoveObserver(this);
-  search_model_->search_box()->Update(search_box()->text(), initiated_by_user);
-  search_model_->search_box()->SetSelectionModel(
-      search_box()->GetSelectionModel());
-  search_model_->search_box()->AddObserver(this);
-}
-
-void SearchBoxView::UpdateSearchIcon() {
-  const gfx::VectorIcon& google_icon =
-      is_search_box_active() ? kIcGoogleColorIcon : kIcGoogleBlackIcon;
-  const gfx::VectorIcon& icon = search_model_->search_engine_is_google()
-                                    ? google_icon
-                                    : kIcSearchEngineNotGoogleIcon;
-  SetSearchIconImage(gfx::CreateVectorIcon(icon, search_box::kSearchIconSize,
-                                           search_box_color()));
-}
-
 void SearchBoxView::GetWallpaperProminentColors(
     AppListViewDelegate::GetWallpaperProminentColorsCallback callback) {
   view_delegate_->GetWallpaperProminentColors(std::move(callback));
 }
 
+void SearchBoxView::OnWallpaperProminentColorsReceived(
+    const std::vector<SkColor>& prominent_colors) {
+  if (prominent_colors.empty())
+    return;
+  DCHECK_EQ(static_cast<size_t>(ColorProfileType::NUM_OF_COLOR_PROFILES),
+            prominent_colors.size());
+
+  SetSearchBoxColor(
+      prominent_colors[static_cast<int>(ColorProfileType::DARK_MUTED)]);
+  SetBackgroundColor(
+      prominent_colors[static_cast<int>(ColorProfileType::LIGHT_VIBRANT)]);
+  UpdateSearchIcon();
+  close_button()->SetImage(
+      views::Button::STATE_NORMAL,
+      gfx::CreateVectorIcon(views::kIcCloseIcon, kCloseIconSize,
+                            search_box_color()));
+  search_box()->set_placeholder_text_color(search_box_color());
+  UpdateBackgroundColor(background_color());
+  SchedulePaint();
+}
+
 void SearchBoxView::ContentsChanged(views::Textfield* sender,
                                     const base::string16& new_contents) {
   search_box::SearchBoxViewBase::ContentsChanged(sender, new_contents);
@@ -323,73 +386,10 @@ void SearchBoxView::SearchEngineChanged() {
   UpdateSearchIcon();
 }
 
-void SearchBoxView::OnWallpaperProminentColorsReceived(
-    const std::vector<SkColor>& prominent_colors) {
-  if (prominent_colors.empty())
-    return;
-  DCHECK_EQ(static_cast<size_t>(ColorProfileType::NUM_OF_COLOR_PROFILES),
-            prominent_colors.size());
-
-  SetSearchBoxColor(
-      prominent_colors[static_cast<int>(ColorProfileType::DARK_MUTED)]);
-  SetBackgroundColor(
-      prominent_colors[static_cast<int>(ColorProfileType::LIGHT_VIBRANT)]);
-  UpdateSearchIcon();
-  close_button()->SetImage(
-      views::Button::STATE_NORMAL,
-      gfx::CreateVectorIcon(views::kIcCloseIcon, kCloseIconSize,
-                            search_box_color()));
-  search_box()->set_placeholder_text_color(search_box_color());
-  UpdateBackgroundColor(background_color());
-  SchedulePaint();
-}
-
 void SearchBoxView::OnWallpaperColorsChanged() {
   GetWallpaperProminentColors(
       base::BindOnce(&SearchBoxView::OnWallpaperProminentColorsReceived,
                      weak_ptr_factory_.GetWeakPtr()));
 }
 
-void SearchBoxView::UpdateSearchBoxBorder() {
-  if (search_box()->HasFocus() && !is_search_box_active()) {
-    // Show a gray ring around search box to indicate that the search box is
-    // selected. Do not show it when search box is active, because blinking
-    // cursor already indicates that.
-    SetBorder(views::CreateRoundedRectBorder(kSearchBoxBorderWidth,
-                                             kSearchBoxFocusBorderCornerRadius,
-                                             kSearchBoxBorderColor));
-    return;
-  }
-
-  // Creates an empty border as a placeholder for colored border so that
-  // re-layout won't move views below the search box.
-  SetBorder(
-      views::CreateEmptyBorder(kSearchBoxBorderWidth, kSearchBoxBorderWidth,
-                               kSearchBoxBorderWidth, kSearchBoxBorderWidth));
-}
-
-void SearchBoxView::SetupBackButton() {
-  views::ImageButton* back = back_button();
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  back->SetImage(views::ImageButton::STATE_NORMAL,
-                 rb.GetImageSkiaNamed(IDR_APP_LIST_FOLDER_BACK_NORMAL));
-  back->SetImageAlignment(views::ImageButton::ALIGN_CENTER,
-                          views::ImageButton::ALIGN_MIDDLE);
-  back->SetVisible(false);
-  base::string16 back_button_label(
-      l10n_util::GetStringUTF16(IDS_APP_LIST_BACK));
-  back->SetAccessibleName(back_button_label);
-  back->SetTooltipText(back_button_label);
-}
-
-void SearchBoxView::SetupCloseButton() {
-  views::ImageButton* close = close_button();
-  close->SetImage(views::ImageButton::STATE_NORMAL,
-                  gfx::CreateVectorIcon(views::kIcCloseIcon, kCloseIconSize,
-                                        search_box_color()));
-  close->SetVisible(false);
-  close->SetAccessibleName(
-      l10n_util::GetStringUTF16(IDS_APP_LIST_CLEAR_SEARCHBOX));
-}
-
 }  // namespace app_list

ui/app_list/views/search_box_view.h

@@ -38,10 +38,16 @@ class APP_LIST_EXPORT SearchBoxView : public search_box::SearchBoxViewBase,
   ~SearchBoxView() override;
 
   // Overridden from search_box::SearchBoxViewBase:
-  void ModelChanged() override;
   void ClearSearch() override;
-  void UpdateKeyboardVisibility() override;
+  views::View* GetSelectedViewInContentsView() override;
   void HandleSearchBoxEvent(ui::LocatedEvent* located_event) override;
+  void ModelChanged() override;
+  void UpdateKeyboardVisibility() override;
+  void UpdateModel(bool initiated_by_user) override;
+  void UpdateSearchIcon() override;
+  void UpdateSearchBoxBorder() override;
+  void SetupCloseButton() override;
+  void SetupBackButton() override;
 
   // Overridden from views::View:
   void OnKeyEvent(ui::KeyEvent* evetn) override;
@@ -66,14 +72,7 @@ class APP_LIST_EXPORT SearchBoxView : public search_box::SearchBoxViewBase,
   // Updates the opacity of the searchbox.
   void UpdateOpacity();
 
-  // Overridden from search_box::SearchBoxViewBase:
-  views::View* GetSelectedViewInContentsView() override;
-
  private:
-  // Overridden from search_box::SearchBoxViewBase:
-  void UpdateModel(bool initiated_by_user) override;
-  void UpdateSearchIcon() override;
-
   // Gets the wallpaper prominent colors.
   void GetWallpaperProminentColors(
       AppListViewDelegate::GetWallpaperProminentColorsCallback callback);
@@ -100,11 +99,6 @@ class APP_LIST_EXPORT SearchBoxView : public search_box::SearchBoxViewBase,
   // Overridden from AppListViewDelegateObserver:
   void OnWallpaperColorsChanged() override;
 
-  // Overridden from search_box::SearchBoxViewBase:
-  void UpdateSearchBoxBorder() override;
-  void SetupBackButton() override;
-  void SetupCloseButton() override;
-
   AppListViewDelegate* view_delegate_;  // Not owned.
   SearchModel* search_model_ = nullptr;  // Owned by the profile-keyed service.
 

ui/chromeos/search_box/search_box_view_base.cc

@@ -227,15 +227,6 @@ bool SearchBoxViewBase::HasSearch() const {
   return !search_box_->text().empty();
 }
 
-void SearchBoxViewBase::ClearSearch() {
-  search_box_->SetText(base::string16());
-  UpdateCloseButtonVisisbility();
-  // Updates model and fires query changed manually because SetText() above
-  // does not generate ContentsChanged() notification.
-  UpdateModel(false);
-  NotifyQueryChanged();
-}
-
 gfx::Rect SearchBoxViewBase::GetViewBoundsForSearchBoxContentsBounds(
     const gfx::Rect& rect) const {
   gfx::Rect view_bounds = rect;
@@ -286,22 +277,6 @@ void SearchBoxViewBase::SetSearchBoxActive(bool active) {
   SchedulePaint();
 }
 
-void SearchBoxViewBase::HandleSearchBoxEvent(ui::LocatedEvent* located_event) {
-  if (located_event->type() == ui::ET_MOUSE_PRESSED ||
-      located_event->type() == ui::ET_GESTURE_TAP) {
-    bool event_is_in_searchbox_bounds =
-        GetWidget()->GetWindowBoundsInScreen().Contains(
-            located_event->root_location());
-    if (is_search_box_active_ || !event_is_in_searchbox_bounds ||
-        !search_box_->text().empty())
-      return;
-    // If the event was within the searchbox bounds and in an inactive empty
-    // search box, enable the search box.
-    SetSearchBoxActive(true);
-  }
-  located_event->SetHandled();
-}
-
 bool SearchBoxViewBase::OnTextfieldEvent() {
   if (is_search_box_active_)
     return false;
@@ -366,10 +341,6 @@ void SearchBoxViewBase::OnTabletModeChanged(bool started) {
   UpdateKeyboardVisibility();
 }
 
-views::View* SearchBoxViewBase::GetSelectedViewInContentsView() {
-  return nullptr;
-}
-
 void SearchBoxViewBase::OnOnSearchBoxFocusedChanged() {
   UpdateSearchBoxBorder();
   Layout();
@@ -383,6 +354,19 @@ bool SearchBoxViewBase::IsSearchBoxTrimmedQueryEmpty() const {
   return trimmed_query.empty();
 }
 
+void SearchBoxViewBase::ClearSearch() {
+  search_box_->SetText(base::string16());
+  UpdateCloseButtonVisisbility();
+  // Updates model and fires query changed manually because SetText() above
+  // does not generate ContentsChanged() notification.
+  UpdateModel(false);
+  NotifyQueryChanged();
+}
+
+views::View* SearchBoxViewBase::GetSelectedViewInContentsView() {
+  return nullptr;
+}
+
 void SearchBoxViewBase::NotifyQueryChanged() {
   DCHECK(delegate_);
   delegate_->QueryChanged(this);
@@ -407,14 +391,6 @@ void SearchBoxViewBase::SetSearchBoxColor(SkColor color) {
       SK_ColorTRANSPARENT == color ? kDefaultSearchboxColor : color;
 }
 
-// TODO(crbug.com/755219): Unify this with SetBackgroundColor.
-void SearchBoxViewBase::UpdateBackgroundColor(SkColor color) {
-  if (is_search_box_active_)
-    color = kSearchBoxBackgroundDefault;
-  GetSearchBoxBackground()->set_color(color);
-  search_box_->SetBackgroundColor(color);
-}
-
 void SearchBoxViewBase::UpdateCloseButtonVisisbility() {
   if (!close_button_)
     return;
@@ -458,6 +434,30 @@ void SearchBoxViewBase::SetSearchIconImage(gfx::ImageSkia image) {
   search_icon_->SetImage(image);
 }
 
+void SearchBoxViewBase::HandleSearchBoxEvent(ui::LocatedEvent* located_event) {
+  if (located_event->type() == ui::ET_MOUSE_PRESSED ||
+      located_event->type() == ui::ET_GESTURE_TAP) {
+    bool event_is_in_searchbox_bounds =
+        GetWidget()->GetWindowBoundsInScreen().Contains(
+            located_event->root_location());
+    if (is_search_box_active_ || !event_is_in_searchbox_bounds ||
+        !search_box_->text().empty())
+      return;
+    // If the event was within the searchbox bounds and in an inactive empty
+    // search box, enable the search box.
+    SetSearchBoxActive(true);
+  }
+  located_event->SetHandled();
+}
+
+// TODO(crbug.com/755219): Unify this with SetBackgroundColor.
+void SearchBoxViewBase::UpdateBackgroundColor(SkColor color) {
+  if (is_search_box_active_)
+    color = kSearchBoxBackgroundDefault;
+  GetSearchBoxBackground()->set_color(color);
+  search_box_->SetBackgroundColor(color);
+}
+
 SearchBoxBackground* SearchBoxViewBase::GetSearchBoxBackground() const {
   return static_cast<SearchBoxBackground*>(content_container_->background());
 }

ui/chromeos/search_box/search_box_view_base.h

@@ -48,9 +48,7 @@ class SEARCH_BOX_EXPORT SearchBoxViewBase : public views::WidgetDelegateView,
 
   void Init();
 
-  virtual void ModelChanged() = 0;
   bool HasSearch() const;
-  virtual void ClearSearch();
 
   // Returns the bounds to use for the view (including the shadow) given the
   // desired bounds of the search box contents.
@@ -74,13 +72,6 @@ class SEARCH_BOX_EXPORT SearchBoxViewBase : public views::WidgetDelegateView,
   // disables cursor blink.
   void SetSearchBoxActive(bool active);
 
-  // Shows/hides the virtual keyboard if the search box is active.
-  virtual void UpdateKeyboardVisibility() = 0;
-
-  // Detects |ET_MOUSE_PRESSED| and |ET_GESTURE_TAP| events on the white
-  // background of the search box.
-  virtual void HandleSearchBoxEvent(ui::LocatedEvent* located_event);
-
   // Handles Gesture and Mouse Events sent from |search_box_|.
   bool OnTextfieldEvent();
 
@@ -108,14 +99,16 @@ class SEARCH_BOX_EXPORT SearchBoxViewBase : public views::WidgetDelegateView,
   // Whether the search box is active.
   bool is_search_box_active() const { return is_search_box_active_; }
 
-  // Returns selected view in contents view.
-  virtual views::View* GetSelectedViewInContentsView();
-
   void OnOnSearchBoxFocusedChanged();
 
   // Whether the trimmed query in the search box is empty.
   bool IsSearchBoxTrimmedQueryEmpty() const;
 
+  virtual void ClearSearch();
+
+  // Returns selected view in contents view.
+  virtual views::View* GetSelectedViewInContentsView();
+
  protected:
   // Fires query change notification.
   void NotifyQueryChanged();
@@ -131,9 +124,6 @@ class SEARCH_BOX_EXPORT SearchBoxViewBase : public views::WidgetDelegateView,
   void SetSearchBoxColor(SkColor color);
   SkColor search_box_color() const { return search_box_color_; }
 
-  // Updates the search box's background color.
-  virtual void UpdateBackgroundColor(SkColor color);
-
   // Updates the visibility of close button.
   void UpdateCloseButtonVisisbility();
 
@@ -159,16 +149,25 @@ class SEARCH_BOX_EXPORT SearchBoxViewBase : public views::WidgetDelegateView,
 
   void SetSearchIconImage(gfx::ImageSkia image);
 
+  // Detects |ET_MOUSE_PRESSED| and |ET_GESTURE_TAP| events on the white
+  // background of the search box.
+  virtual void HandleSearchBoxEvent(ui::LocatedEvent* located_event);
+
+  // Updates the search box's background color.
+  virtual void UpdateBackgroundColor(SkColor color);
+
  private:
+  virtual void ModelChanged() = 0;
+
+  // Shows/hides the virtual keyboard if the search box is active.
+  virtual void UpdateKeyboardVisibility() = 0;
+
   // Updates model text and selection model with current Textfield info.
   virtual void UpdateModel(bool initiated_by_user) = 0;
 
   // Updates the search icon.
   virtual void UpdateSearchIcon() = 0;
 
-  // Gets the search box background.
-  SearchBoxBackground* GetSearchBoxBackground() const;
-
   // Update search box border based on whether the search box is activated.
   virtual void UpdateSearchBoxBorder() = 0;
 
@@ -176,6 +175,9 @@ class SEARCH_BOX_EXPORT SearchBoxViewBase : public views::WidgetDelegateView,
   virtual void SetupCloseButton() = 0;
   virtual void SetupBackButton() = 0;
 
+  // Gets the search box background.
+  SearchBoxBackground* GetSearchBoxBackground() const;
+
   SearchBoxViewDelegate* delegate_;  // Not owned.
 
   // Owned by views hierarchy.