Revert of Making answer card to behave like other results. (patchset #6...

Revert of Making answer card to behave like other results. (patchset #6 id:100001 of https://codereview.chromium.org/2905523004/ )

Reason for revert:
This broke a build
https://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20x86-generic%20Compile/builds/36279

Original issue's description:
> Making answer card to behave like other results.
>
> Now it lives in a search result container, which can be selected,
> highlighted, navigated with keyboard etc.
>
> Opening the result (i.e. what happens upon clicking on it) is not yet
> implemented.
>
> Unit test for the new result container isn't yet implemented since both
> result opening and accessibility behavior are not yet implemented.
>
> Bug=712331
>
> Review-Url: https://codereview.chromium.org/2905523004
> Cr-Commit-Position: refs/heads/master@{#475782}
> Committed: https://chromium.googlesource.com/chromium/src/+/2420a92712ee82be175bea6750774bec8006c5ce

TBR=xiyuan@chromium.org,jam@chromium.org,sky@chromium.org,vadimt@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Bug=712331

Review-Url: https://codereview.chromium.org/2910173003
Cr-Commit-Position: refs/heads/master@{#475795}

chrome/browser/chromeos/BUILD.gn

@@ -977,6 +977,8 @@ source_set("chromeos") {
     "login/ui/user_adding_screen_input_methods_controller.h",
     "login/ui/web_contents_forced_title.cc",
     "login/ui/web_contents_forced_title.h",
+    "login/ui/web_contents_set_background_color.cc",
+    "login/ui/web_contents_set_background_color.h",
     "login/ui/webui_login_display.cc",
     "login/ui/webui_login_display.h",
     "login/ui/webui_login_view.cc",

ui/views/controls/webview/web_contents_set_background_color.cc → chrome/browser/chromeos/login/ui/web_contents_set_background_color.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
+// Copyright 2016 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.
 
-#include "ui/views/controls/webview/web_contents_set_background_color.h"
+#include "chrome/browser/chromeos/login/ui/web_contents_set_background_color.h"
 
 #include "base/memory/ptr_util.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 
-DEFINE_WEB_CONTENTS_USER_DATA_KEY(views::WebContentsSetBackgroundColor);
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(chromeos::WebContentsSetBackgroundColor);
 
-namespace views {
+namespace chromeos {
 
 // static
 void WebContentsSetBackgroundColor::CreateForWebContentsWithColor(
@@ -54,4 +54,4 @@ void WebContentsSetBackgroundColor::RenderViewHostChanged(
   new_host->GetWidget()->GetView()->SetBackgroundColor(color_);
 }
 
-}  // namespace views
+}  // namespace chromeos

ui/views/controls/webview/web_contents_set_background_color.h → chrome/browser/chromeos/login/ui/web_contents_set_background_color.h

@@ -2,17 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef UI_VIEWS_CONTROLS_WEBVIEW_WEB_CONTENTS_SET_BACKGROUND_COLOR_H_
-#define UI_VIEWS_CONTROLS_WEBVIEW_WEB_CONTENTS_SET_BACKGROUND_COLOR_H_
+#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_UI_WEB_CONTENTS_SET_BACKGROUND_COLOR_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_UI_WEB_CONTENTS_SET_BACKGROUND_COLOR_H_
 
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
-#include "ui/views/controls/webview/webview_export.h"
 
 // Defined in SkColor.h (32-bit ARGB color).
 using SkColor = unsigned int;
 
-namespace views {
+namespace chromeos {
 
 // Ensures that the background color of a given WebContents instance is always
 // set to a given color value.
@@ -20,9 +19,8 @@ class WebContentsSetBackgroundColor
     : public content::WebContentsObserver,
       public content::WebContentsUserData<WebContentsSetBackgroundColor> {
  public:
-  WEBVIEW_EXPORT static void CreateForWebContentsWithColor(
-      content::WebContents* web_contents,
-      SkColor color);
+  static void CreateForWebContentsWithColor(content::WebContents* web_contents,
+                                            SkColor color);
 
   ~WebContentsSetBackgroundColor() override;
 
@@ -41,6 +39,6 @@ class WebContentsSetBackgroundColor
   DISALLOW_COPY_AND_ASSIGN(WebContentsSetBackgroundColor);
 };
 
-}  // namespace views
+}  // namespace chromeos
 
-#endif  // UI_VIEWS_CONTROLS_WEBVIEW_WEB_CONTENTS_SET_BACKGROUND_COLOR_H_
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_UI_WEB_CONTENTS_SET_BACKGROUND_COLOR_H_

chrome/browser/chromeos/login/ui/webui_login_view.cc

@@ -25,6 +25,7 @@
 #include "chrome/browser/chromeos/login/ui/preloaded_web_view_factory.h"
 #include "chrome/browser/chromeos/login/ui/proxy_settings_dialog.h"
 #include "chrome/browser/chromeos/login/ui/web_contents_forced_title.h"
+#include "chrome/browser/chromeos/login/ui/web_contents_set_background_color.h"
 #include "chrome/browser/chromeos/login/ui/webui_login_display.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
@@ -56,7 +57,6 @@
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/keyboard/keyboard_controller.h"
-#include "ui/views/controls/webview/web_contents_set_background_color.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/views/widget/widget.h"
 
@@ -220,7 +220,7 @@ void WebUILoginView::InitializeWebView(views::WebView* web_view,
   if (!title.empty())
     WebContentsForcedTitle::CreateForWebContentsWithTitle(web_contents, title);
 
-  views::WebContentsSetBackgroundColor::CreateForWebContentsWithColor(
+  WebContentsSetBackgroundColor::CreateForWebContentsWithColor(
       web_contents, SK_ColorTRANSPARENT);
 
   // Ensure that the login UI has a tab ID, which will allow the GAIA auth

chrome/browser/ui/app_list/search_answer_web_contents_delegate.cc

@@ -21,7 +21,6 @@
 #include "ui/app_list/app_list_features.h"
 #include "ui/app_list/app_list_model.h"
 #include "ui/app_list/search_box_model.h"
-#include "ui/views/controls/webview/web_contents_set_background_color.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/views/widget/widget.h"
 
@@ -106,11 +105,6 @@ SearchAnswerWebContentsDelegate::SearchAnswerWebContentsDelegate(
     web_view_->SetFocusBehavior(views::View::FocusBehavior::NEVER);
 
   model->AddObserver(this);
-
-  // Make the webview transparent since it's going to be shown on top of a
-  // highlightable button.
-  views::WebContentsSetBackgroundColor::CreateForWebContentsWithColor(
-      web_contents_.get(), SK_ColorTRANSPARENT);
 }
 
 SearchAnswerWebContentsDelegate::~SearchAnswerWebContentsDelegate() {
@@ -174,7 +168,8 @@ void SearchAnswerWebContentsDelegate::UpdatePreferredSize(
       IsCardSizeOk(pref_size) || features::IsAnswerCardDarkRunEnabled();
   model_->SetSearchAnswerAvailable(is_card_size_ok_ && received_answer_ &&
                                    !web_contents_->IsLoading());
-  web_view_->SetPreferredSize(pref_size);
+  if (!features::IsAnswerCardDarkRunEnabled())
+    web_view_->SetPreferredSize(pref_size);
   if (!answer_loaded_time_.is_null()) {
     UMA_HISTOGRAM_TIMES("SearchAnswer.ResizeAfterLoadTime",
                         base::TimeTicks::Now() - answer_loaded_time_);

ui/app_list/BUILD.gn

@@ -140,8 +140,6 @@ component("app_list") {
       "views/search_result_actions_view.cc",
       "views/search_result_actions_view.h",
       "views/search_result_actions_view_delegate.h",
-      "views/search_result_answer_card_view.cc",
-      "views/search_result_answer_card_view.h",
       "views/search_result_container_view.cc",
       "views/search_result_container_view.h",
       "views/search_result_list_view.cc",

ui/app_list/views/contents_view.cc

@@ -17,23 +17,56 @@
 #include "ui/app_list/views/apps_grid_view.h"
 #include "ui/app_list/views/custom_launcher_page_view.h"
 #include "ui/app_list/views/search_box_view.h"
-#include "ui/app_list/views/search_result_answer_card_view.h"
 #include "ui/app_list/views/search_result_list_view.h"
 #include "ui/app_list/views/search_result_page_view.h"
 #include "ui/app_list/views/search_result_tile_item_list_view.h"
 #include "ui/app_list/views/start_page_view.h"
 #include "ui/events/event.h"
+#include "ui/views/layout/box_layout.h"
 #include "ui/views/view_model.h"
 #include "ui/views/widget/widget.h"
 
 namespace app_list {
 
+namespace {
+
+// Container of the search answer view.
+class SearchAnswerContainerView : public views::View {
+ public:
+  explicit SearchAnswerContainerView(views::View* search_results_page_view)
+      : search_results_page_view_(search_results_page_view) {
+    views::BoxLayout* answer_container_layout =
+        new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0);
+    answer_container_layout->set_main_axis_alignment(
+        views::BoxLayout::MAIN_AXIS_ALIGNMENT_CENTER);
+    SetLayoutManager(answer_container_layout);
+  }
+
+  // views::View overrides:
+  void ChildPreferredSizeChanged(View* child) override {
+    if (visible())
+      search_results_page_view_->Layout();
+  }
+
+  const char* GetClassName() const override {
+    return "SearchAnswerContainerView";
+  }
+
+ private:
+  views::View* const search_results_page_view_;
+
+  DISALLOW_COPY_AND_ASSIGN(SearchAnswerContainerView);
+};
+
+}  // namespace
+
 ContentsView::ContentsView(AppListMainView* app_list_main_view)
     : model_(nullptr),
       apps_container_view_(nullptr),
       search_results_page_view_(nullptr),
       start_page_view_(nullptr),
       custom_page_view_(nullptr),
+      search_answer_container_view_(nullptr),
       app_list_main_view_(app_list_main_view),
       page_before_search_(0) {
   pagination_model_.SetTransitionDurations(kPageTransitionDurationInMs,
@@ -43,6 +76,8 @@ ContentsView::ContentsView(AppListMainView* app_list_main_view)
 
 ContentsView::~ContentsView() {
   pagination_model_.RemoveObserver(this);
+  if (model_)
+    model_->RemoveObserver(this);
 }
 
 void ContentsView::Init(AppListModel* model) {
@@ -69,14 +104,14 @@ void ContentsView::Init(AppListModel* model) {
   // Search results UI.
   search_results_page_view_ = new SearchResultPageView();
 
-  // Search result containers.
-  views::View* const search_answer_view =
-      view_delegate->GetSearchAnswerWebView();
-  if (search_answer_view) {
-    search_results_page_view_->AddSearchResultContainerView(
-        nullptr, new SearchResultAnswerCardView(
-                     model_, search_results_page_view_, search_answer_view));
-  }
+  // Search answer container UI.
+  search_answer_container_view_ =
+      new SearchAnswerContainerView(search_results_page_view_);
+  search_answer_container_view_->SetVisible(false);
+  views::View* search_answer_view = view_delegate->GetSearchAnswerWebView();
+  if (search_answer_view)
+    search_answer_container_view_->AddChildView(search_answer_view);
+  search_results_page_view_->AddChildView(search_answer_container_view_);
 
   AppListModel::SearchResults* results = view_delegate->GetModel()->results();
   search_results_page_view_->AddSearchResultContainerView(
@@ -107,6 +142,8 @@ void ContentsView::Init(AppListModel* model) {
   pagination_model_.SelectPage(initial_page_index, false);
 
   ActivePageChanged();
+
+  model_->AddObserver(this);
 }
 
 void ContentsView::CancelDrag() {
@@ -487,4 +524,12 @@ void ContentsView::TransitionChanged() {
   UpdatePageBounds();
 }
 
+void ContentsView::OnSearchAnswerAvailableChanged(bool has_answer) {
+  if (has_answer == search_answer_container_view_->visible())
+    return;
+
+  search_answer_container_view_->SetVisible(has_answer);
+  search_results_page_view_->Layout();
+}
+
 }  // namespace app_list

ui/app_list/views/contents_view.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "ui/app_list/app_list_export.h"
 #include "ui/app_list/app_list_model.h"
+#include "ui/app_list/app_list_model_observer.h"
 #include "ui/app_list/pagination_model.h"
 #include "ui/app_list/pagination_model_observer.h"
 #include "ui/views/view.h"
@@ -42,7 +43,8 @@ class StartPageView;
 // interface for switching between launcher pages, and animates the transition
 // between them.
 class APP_LIST_EXPORT ContentsView : public views::View,
-                                     public PaginationModelObserver {
+                                     public PaginationModelObserver,
+                                     public AppListModelObserver {
  public:
   explicit ContentsView(AppListMainView* app_list_main_view);
   ~ContentsView() override;
@@ -134,6 +136,9 @@ class APP_LIST_EXPORT ContentsView : public views::View,
   void TransitionStarted() override;
   void TransitionChanged() override;
 
+  // Overridden from AppListModelObserver:
+  void OnSearchAnswerAvailableChanged(bool has_answer) override;
+
  private:
   // Sets the active launcher page, accounting for whether the change is for
   // search results.
@@ -185,6 +190,10 @@ class APP_LIST_EXPORT ContentsView : public views::View,
   StartPageView* start_page_view_;
   CustomLauncherPageView* custom_page_view_;
 
+  // Unowned pointer to the container of the search answer web view. This
+  // container view is a sub-view of search_results_page_view_.
+  View* search_answer_container_view_;
+
   // The child page views. Owned by the views hierarchy.
   std::vector<AppListPage*> app_list_pages_;
 

ui/app_list/views/search_result_answer_card_view.cc

-// 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.
-
-#include "ui/app_list/views/search_result_answer_card_view.h"
-
-#include "ui/app_list/app_list_constants.h"
-#include "ui/app_list/app_list_features.h"
-#include "ui/app_list/views/search_result_page_view.h"
-#include "ui/views/background.h"
-#include "ui/views/controls/button/custom_button.h"
-#include "ui/views/layout/box_layout.h"
-#include "ui/views/layout/fill_layout.h"
-
-namespace app_list {
-
-namespace {
-
-// Answer card relevance is high to always have it first.
-constexpr double kSearchAnswerCardRelevance = 100;
-
-// Container of the search answer view.
-class SearchAnswerContainerView : public views::CustomButton {
- public:
-  explicit SearchAnswerContainerView(views::View* search_results_page_view)
-      : CustomButton(nullptr),
-        search_results_page_view_(search_results_page_view) {
-    // Center the card horizontally in the container.
-    views::BoxLayout* answer_container_layout =
-        new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0);
-    answer_container_layout->set_main_axis_alignment(
-        views::BoxLayout::MAIN_AXIS_ALIGNMENT_CENTER);
-    SetLayoutManager(answer_container_layout);
-  }
-
-  void SetSelected(bool selected) {
-    if (selected == selected_)
-      return;
-    selected_ = selected;
-    UpdateBackgroundColor();
-  }
-
-  // views::CustomButton overrides:
-  void ChildPreferredSizeChanged(View* child) override {
-    // Card size changed.
-    if (visible())
-      search_results_page_view_->Layout();
-  }
-
-  int GetHeightForWidth(int w) const override {
-    return visible() ? CustomButton::GetHeightForWidth(w) : 0;
-  }
-
-  const char* GetClassName() const override {
-    return "SearchAnswerContainerView";
-  }
-
-  void StateChanged(ButtonState old_state) override { UpdateBackgroundColor(); }
-
- private:
-  void UpdateBackgroundColor() {
-    views::Background* background = nullptr;
-
-    if (selected_) {
-      background = views::Background::CreateSolidBackground(kSelectedColor);
-    } else if (state() == STATE_HOVERED || state() == STATE_PRESSED) {
-      background = views::Background::CreateSolidBackground(kHighlightedColor);
-    }
-
-    set_background(background);
-    SchedulePaint();
-  }
-
-  views::View* const search_results_page_view_;
-  bool selected_ = false;
-
-  DISALLOW_COPY_AND_ASSIGN(SearchAnswerContainerView);
-};
-
-}  // namespace
-
-SearchResultAnswerCardView::SearchResultAnswerCardView(
-    AppListModel* model,
-    SearchResultPageView* search_results_page_view,
-    views::View* search_answer_view)
-    : model_(model),
-      search_answer_container_view_(
-          new SearchAnswerContainerView(search_results_page_view)) {
-  search_answer_container_view_->SetVisible(false);
-  search_answer_container_view_->AddChildView(search_answer_view);
-  AddChildView(search_answer_container_view_);
-  model->AddObserver(this);
-  SetLayoutManager(new views::FillLayout);
-}
-
-SearchResultAnswerCardView::~SearchResultAnswerCardView() {
-  model_->RemoveObserver(this);
-}
-
-const char* SearchResultAnswerCardView::GetClassName() const {
-  return "SearchResultAnswerCardView";
-}
-
-void SearchResultAnswerCardView::OnContainerSelected(
-    bool from_bottom,
-    bool directional_movement) {
-  if (num_results() == 0)
-    return;
-
-  SetSelectedIndex(0);
-}
-
-int SearchResultAnswerCardView::GetYSize() {
-  return num_results();
-}
-
-int SearchResultAnswerCardView::DoUpdate() {
-  const bool have_result = search_answer_container_view_->visible();
-  set_container_score(have_result ? kSearchAnswerCardRelevance : 0);
-  return have_result ? 1 : 0;
-}
-
-void SearchResultAnswerCardView::UpdateSelectedIndex(int old_selected,
-                                                     int new_selected) {
-  if (new_selected == old_selected)
-    return;
-
-  const bool is_selected = new_selected == 0;
-  search_answer_container_view_->SetSelected(is_selected);
-  if (is_selected)
-    NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true);
-}
-
-void SearchResultAnswerCardView::OnSearchAnswerAvailableChanged(
-    bool has_answer) {
-  const bool visible = has_answer && !features::IsAnswerCardDarkRunEnabled();
-  if (visible == search_answer_container_view_->visible())
-    return;
-
-  search_answer_container_view_->SetVisible(visible);
-  ScheduleUpdate();
-}
-
-}  // namespace app_list

ui/app_list/views/search_result_answer_card_view.h

-// 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.
-
-#ifndef UI_APP_LIST_VIEWS_SEARCH_RESULT_ANSWER_CARD_VIEW_H_
-#define UI_APP_LIST_VIEWS_SEARCH_RESULT_ANSWER_CARD_VIEW_H_
-
-#include "ui/app_list/app_list_model_observer.h"
-#include "ui/app_list/views/search_result_container_view.h"
-
-namespace app_list {
-
-class AppListModel;
-class SearchResultPageView;
-
-namespace {
-class SearchAnswerContainerView;
-}
-
-// Result container for the search answer card.
-class APP_LIST_EXPORT SearchResultAnswerCardView
-    : public SearchResultContainerView,
-      public AppListModelObserver {
- public:
-  SearchResultAnswerCardView(AppListModel* model,
-                             SearchResultPageView* search_results_page_view,
-                             views::View* search_answer_view);
-  ~SearchResultAnswerCardView() override;
-
- private:
-  // Overridden from views::View:
-  const char* GetClassName() const override;
-
-  // Overridden from SearchResultContainerView:
-  void OnContainerSelected(bool from_bottom,
-                           bool directional_movement) override;
-  void NotifyFirstResultYIndex(int y_index) override {}
-  int GetYSize() override;
-  int DoUpdate() override;
-  void UpdateSelectedIndex(int old_selected, int new_selected) override;
-
-  // Overridden from AppListModelObserver
-  void OnSearchAnswerAvailableChanged(bool has_answer) override;
-
-  // Unowned pointer to application list model.
-  AppListModel* const model_;
-
-  // Pointer to the container of the search answer; owned by the view hierarchy.
-  // It's visible iff we have a search answer result.
-  SearchAnswerContainerView* const search_answer_container_view_;
-
-  DISALLOW_COPY_AND_ASSIGN(SearchResultAnswerCardView);
-};
-
-}  // namespace app_list
-
-#endif  // UI_APP_LIST_VIEWS_SEARCH_RESULT_ANSWER_CARD_VIEW_H_

ui/app_list/views/search_result_container_view.h

@@ -84,12 +84,11 @@ class APP_LIST_EXPORT SearchResultContainerView : public views::View,
   virtual void OnContainerSelected(bool from_bottom,
                                    bool directional_movement) = 0;
 
- protected:
+ private:
   // Schedules an Update call using |update_factory_|. Do nothing if there is a
   // pending call.
   void ScheduleUpdate();
 
- private:
   // Updates UI with model. Returns the number of visible results.
   virtual int DoUpdate() = 0;
 

ui/views/controls/webview/BUILD.gn

@@ -8,8 +8,6 @@ component("webview") {
     "unhandled_keyboard_event_handler.h",
     "unhandled_keyboard_event_handler_mac.mm",
     "unhandled_keyboard_event_handler_win.cc",
-    "web_contents_set_background_color.cc",
-    "web_contents_set_background_color.h",
     "web_dialog_view.cc",
     "web_dialog_view.h",
     "webview.cc",