[omnibox] WebUIOmniboxPopup: Remove native result views when flag is on

When the WebUIOmniboxPopup flag is on, this CL doesn't add the native
result views.

This CL is just meant to "hollow out" the OmniboxPopupContentsView
when the flag is on. The next step would be add an actual
views::WebView to take the place of all the child result views.

This CL was deliberately kept to just a bunch of stubs to illustrate
all the integration points we would need with a WebUI popup.

Even when the flag is on, we keep the actual OmniboxPopupContentsView,
because it's responsible for drawing the "beard" and transparent
poke-through that we need even if we render the popup contents in
WebUI.

Bug: 1046561
Change-Id: I01d7a77413b760a290dc823b2a142921121f81cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2042218Reviewed-by: manuk hovanesian <manukh@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739131}

chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc

@@ -8,6 +8,7 @@
 #include <numeric>
 
 #include "base/bind.h"
+#include "base/feature_list.h"
 #include "base/optional.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
@@ -192,6 +193,9 @@ void OmniboxPopupContentsView::UnselectButton() {
 }
 
 OmniboxResultView* OmniboxPopupContentsView::result_view_at(size_t i) {
+  DCHECK(!base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup))
+      << "With the WebUI omnibox popup enabled, the code should not try to "
+         "fetch the child result view.";
   return static_cast<OmniboxResultView*>(children()[i]);
 }
 
@@ -204,6 +208,9 @@ bool OmniboxPopupContentsView::IsOpen() const {
 }
 
 void OmniboxPopupContentsView::InvalidateLine(size_t line) {
+  if (base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup))
+    return;  // TODO(tommycli): Not implemented yet for WebUI.
+
   OmniboxResultView* result = result_view_at(line);
   result->Invalidate();
 
@@ -215,6 +222,9 @@ void OmniboxPopupContentsView::InvalidateLine(size_t line) {
 }
 
 void OmniboxPopupContentsView::OnSelectionStateChanged(size_t line) {
+  if (base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup))
+    return;  // TODO(tommycli): Not implemented yet for WebUI.
+
   result_view_at(line)->OnSelectionStateChanged();
   InvalidateLine(line);
 }
@@ -267,26 +277,31 @@ void OmniboxPopupContentsView::UpdatePopupAppearance() {
   // Update the match cached by each row, in the process of doing so make sure
   // we have enough row views.
   const size_t result_size = model_->result().size();
-  for (size_t i = 0; i < result_size; ++i) {
-    // Create child views lazily.  Since especially the first result view may be
-    // expensive to create due to loading font data, this saves time and memory
-    // during browser startup.
-    if (children().size() == i) {
-      AddChildView(std::make_unique<OmniboxResultView>(this, i));
+  if (base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup)) {
+    // TODO(tommycli): Not implemented yet for WebUI.
+  } else {
+    for (size_t i = 0; i < result_size; ++i) {
+      // Create child views lazily.  Since especially the first result view may
+      // be expensive to create due to loading font data, this saves time and
+      // memory during browser startup.
+      if (children().size() == i) {
+        AddChildView(std::make_unique<OmniboxResultView>(this, i));
+      }
+
+      OmniboxResultView* view = result_view_at(i);
+      const AutocompleteMatch& match = GetMatchAtIndex(i);
+      view->SetMatch(match);
+      view->SetVisible(true);
+      const SkBitmap* bitmap = model_->RichSuggestionBitmapAt(i);
+      if (bitmap)
+        view->SetRichSuggestionImage(
+            gfx::ImageSkia::CreateFrom1xBitmap(*bitmap));
     }
 
-    OmniboxResultView* view = result_view_at(i);
-    const AutocompleteMatch& match = GetMatchAtIndex(i);
-    view->SetMatch(match);
-    view->SetVisible(true);
-    const SkBitmap* bitmap = model_->RichSuggestionBitmapAt(i);
-    if (bitmap)
-      view->SetRichSuggestionImage(gfx::ImageSkia::CreateFrom1xBitmap(*bitmap));
+    for (auto i = children().begin() + result_size; i != children().end(); ++i)
+      (*i)->SetVisible(false);
   }
 
-  for (auto i = children().begin() + result_size; i != children().end(); ++i)
-    (*i)->SetVisible(false);
-
   popup_->SetTargetBounds(GetTargetBounds());
 
   if (popup_created) {
@@ -294,7 +309,8 @@ void OmniboxPopupContentsView::UpdatePopupAppearance() {
 
     // Popup is now expanded and first item will be selected.
     NotifyAccessibilityEvent(ax::mojom::Event::kExpandedChanged, true);
-    if (result_view_at(0)) {
+    if (!base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup) &&
+        result_view_at(0)) {
       result_view_at(0)->NotifyAccessibilityEvent(ax::mojom::Event::kSelection,
                                                   true);
     }
@@ -303,10 +319,16 @@ void OmniboxPopupContentsView::UpdatePopupAppearance() {
 }
 
 void OmniboxPopupContentsView::ProvideButtonFocusHint(size_t line) {
+  if (base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup))
+    return;  // TODO(tommycli): Not implemented yet for WebUI.
+
   result_view_at(line)->ProvideButtonFocusHint();
 }
 
 void OmniboxPopupContentsView::OnMatchIconUpdated(size_t match_index) {
+  if (base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup))
+    return;  // TODO(tommycli): Not implemented yet for WebUI.
+
   result_view_at(match_index)->OnMatchIconUpdated();
 }
 
@@ -315,6 +337,9 @@ void OmniboxPopupContentsView::OnDragCanceled() {
 }
 
 bool OmniboxPopupContentsView::OnMouseDragged(const ui::MouseEvent& event) {
+  if (base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup))
+    return true;  // TODO(tommycli): Not implemented yet for WebUI.
+
   size_t index = GetIndexForPoint(event.location());
 
   // If the drag event is over the bounds of one of the result views, pass
@@ -362,12 +387,19 @@ void OmniboxPopupContentsView::OnWidgetBoundsChanged(
 }
 
 gfx::Rect OmniboxPopupContentsView::GetTargetBounds() {
-  DCHECK_GE(children().size(), model_->result().size());
-  int popup_height = std::accumulate(
-      children().cbegin(), children().cbegin() + model_->result().size(), 0,
-      [](int height, const auto* v) {
-        return height + v->GetPreferredSize().height();
-      });
+  int popup_height = 0;
+
+  if (base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup)) {
+    // TODO(tommycli): Not implemented yet for WebUI.
+  } else {
+    DCHECK_GE(children().size(), model_->result().size());
+    popup_height = std::accumulate(
+        children().cbegin(), children().cbegin() + model_->result().size(), 0,
+        [](int height, const auto* v) {
+          return height + v->GetPreferredSize().height();
+        });
+  }
+
   // Add enough space on the top and bottom so it looks like there is the same
   // amount of space between the text and the popup border as there is in the
   // interior between each row of text.

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

@@ -719,6 +719,11 @@ bool OmniboxViewViews::TextAndUIDirectionMatch() const {
 }
 
 views::Button* OmniboxViewViews::GetSecondaryButtonForSelectedLine() const {
+  // TODO(tommycli): If we have a WebUI omnibox popup, we should move the
+  // secondary button logic out of the View and into the OmniboxPopupModel.
+  if (base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup))
+    return nullptr;
+
   OmniboxPopupModel* popup_model = model()->popup_model();
   if (!popup_model)
     return nullptr;
@@ -758,6 +763,11 @@ bool OmniboxViewViews::MaybeUnfocusSecondaryButton() {
 }
 
 bool OmniboxViewViews::MaybeTriggerSecondaryButton(const ui::KeyEvent& event) {
+  // TODO(tommycli): If we have a WebUI omnibox popup, we should move the
+  // secondary button logic out of the View and into the OmniboxPopupModel.
+  if (base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup))
+    return false;
+
   if (model()->popup_model()->selected_line_state() !=
       OmniboxPopupModel::BUTTON_FOCUSED)
     return false;
@@ -867,9 +877,13 @@ void OmniboxViewViews::ClearAccessibilityLabel() {
 void OmniboxViewViews::SetAccessibilityLabel(const base::string16& display_text,
                                              const AutocompleteMatch& match) {
   size_t selected_line = model()->popup_model()->selected_line();
-  if (selected_line != OmniboxPopupModel::kNoMatch && popup_view_) {
+  if (selected_line != OmniboxPopupModel::kNoMatch && popup_view_ &&
+      !base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup)) {
     // Although it feels bad to ask a whole different view for the accessibility
     // text, only the OmniboxResultView knows which secondary button is shown.
+    //
+    // TODO(tommycli): If we have a WebUI omnibox popup, we should move the
+    // secondary button logic out of the View and into the OmniboxPopupModel.
     OmniboxResultView* result_view = popup_view_->result_view_at(selected_line);
     friendly_suggestion_text_ =
         result_view->ToAccessibilityLabelWithSecondaryButton(