Ignore events in the top of the rounded omnibox popup

Bug: 801583
Change-Id: I7841e175a0aba57020775465cd191083bf8b19a4
Reviewed-on: https://chromium-review.googlesource.com/914984
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537570}

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

@@ -63,6 +63,7 @@ class OmniboxPopupContentsView : public views::View, public OmniboxPopupView {
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
  private:
+  friend class OmniboxPopupContentsViewTest;
   class AutocompletePopupWidget;
 
   // Updates |start_margin_| and |end_margin_| and returns the target popup

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

@@ -11,6 +11,7 @@
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
+#include "chrome/browser/ui/views/omnibox/omnibox_result_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_view_views.h"
 #include "chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
@@ -19,12 +20,31 @@
 #include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/omnibox/browser/omnibox_popup_model.h"
 #include "ui/base/ui_base_switches.h"
+#include "ui/events/test/event_generator.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
 
 enum PopupType { WIDE, NARROW, ROUNDED };
 
+// A View that positions itself over another View to intercept clicks.
+class ClickTrackingOverlayView : public views::View {
+ public:
+  ClickTrackingOverlayView(views::View* over, gfx::Point* last_click)
+      : last_click_(last_click) {
+    SetBoundsRect(over->bounds());
+    over->parent()->AddChildView(this);
+  }
+
+  // views::View:
+  void OnMouseEvent(ui::MouseEvent* event) override {
+    *last_click_ = event->location();
+  }
+
+ private:
+  gfx::Point* last_click_;
+};
+
 std::string PrintPopupType(const testing::TestParamInfo<PopupType>& info) {
   switch (info.param) {
     case WIDE:
@@ -62,15 +82,18 @@ class OmniboxPopupContentsViewTest
     }
   }
 
+  views::Widget* CreatePopupForTestQuery();
   views::Widget* GetPopupWidget() { return popup_view()->GetWidget(); }
+  OmniboxResultView* GetResultViewAt(int index) {
+    return popup_view()->result_view_at(index);
+  }
 
   ToolbarView* toolbar() {
     return BrowserView::GetBrowserViewForBrowser(browser())->toolbar();
   }
   LocationBarView* location_bar() { return toolbar()->location_bar(); }
-  OmniboxEditModel* edit_model() {
-    return location_bar()->omnibox_view()->model();
-  }
+  OmniboxViewViews* omnibox_view() { return location_bar()->omnibox_view(); }
+  OmniboxEditModel* edit_model() { return omnibox_view()->model(); }
   OmniboxPopupModel* popup_model() { return edit_model()->popup_model(); }
   OmniboxPopupContentsView* popup_view() {
     return static_cast<OmniboxPopupContentsView*>(popup_model()->view());
@@ -82,8 +105,10 @@ class OmniboxPopupContentsViewTest
   DISALLOW_COPY_AND_ASSIGN(OmniboxPopupContentsViewTest);
 };
 
-// Tests widget alignment of the different popup types.
-IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, PopupAlignment) {
+// Create an alias to instantiate tests that only care about the rounded style.
+using RoundedOmniboxPopupContentsViewTest = OmniboxPopupContentsViewTest;
+
+views::Widget* OmniboxPopupContentsViewTest::CreatePopupForTestQuery() {
   EXPECT_TRUE(popup_model()->result().empty());
   EXPECT_FALSE(popup_view()->IsOpen());
   EXPECT_FALSE(GetPopupWidget());
@@ -95,6 +120,12 @@ IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, PopupAlignment) {
   EXPECT_TRUE(popup_view()->IsOpen());
   views::Widget* popup = GetPopupWidget();
   EXPECT_TRUE(popup);
+  return popup;
+}
+
+// Tests widget alignment of the different popup types.
+IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, PopupAlignment) {
+  views::Widget* popup = CreatePopupForTestQuery();
 
   if (GetParam() == WIDE) {
     EXPECT_EQ(toolbar()->width(), popup->GetRestoredBounds().width());
@@ -115,7 +146,64 @@ IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, PopupAlignment) {
   }
 }
 
+// This is only enabled on ChromeOS for now, since it's hard to align an
+// EventGenerator when multiple native windows are involved (the sanity check
+// will fail). TODO(tapted): Enable everywhere.
+#if defined(OS_CHROMEOS)
+#define MAYBE_ClickOmnibox ClickOmnibox
+#else
+#define MAYBE_ClickOmnibox DISABLED_ClickOmnibox
+#endif
+// Test that clicks over the omnibox do not hit the popup.
+IN_PROC_BROWSER_TEST_P(RoundedOmniboxPopupContentsViewTest,
+                       MAYBE_ClickOmnibox) {
+  CreatePopupForTestQuery();
+  ui::test::EventGenerator generator(browser()->window()->GetNativeWindow());
+
+  OmniboxResultView* result = GetResultViewAt(0);
+  ASSERT_TRUE(result);
+
+  // Sanity check: ensure the EventGenerator clicks where we think it should
+  // when clicking on a result (but don't dismiss the popup yet). This will fail
+  // if the WindowTreeHost and EventGenerator coordinate systems do not align.
+  {
+    const gfx::Point expected_point = result->GetLocalBounds().CenterPoint();
+    EXPECT_NE(gfx::Point(), expected_point);
+
+    gfx::Point click;
+    ClickTrackingOverlayView overlay(result, &click);
+    generator.MoveMouseTo(result->GetBoundsInScreen().CenterPoint());
+    generator.ClickLeftButton();
+    EXPECT_EQ(expected_point, click);
+  }
+
+  // Select the text, so that we can test whether a click is received (which
+  // should deselect the text);
+  omnibox_view()->SelectAll(true);
+  views::Textfield* textfield = omnibox_view();
+  EXPECT_EQ(base::ASCIIToUTF16("foo"), textfield->GetSelectedText());
+
+  generator.MoveMouseTo(location_bar()->GetBoundsInScreen().CenterPoint());
+  generator.ClickLeftButton();
+  EXPECT_EQ(base::string16(), textfield->GetSelectedText());
+
+  // Clicking the result should dismiss the popup (asynchronously).
+  generator.MoveMouseTo(result->GetBoundsInScreen().CenterPoint());
+
+  ASSERT_TRUE(GetPopupWidget());
+  EXPECT_FALSE(GetPopupWidget()->IsClosed());
+
+  generator.ClickLeftButton();
+  ASSERT_TRUE(GetPopupWidget());
+  EXPECT_TRUE(GetPopupWidget()->IsClosed());
+}
+
 INSTANTIATE_TEST_CASE_P(,
                         OmniboxPopupContentsViewTest,
                         ::testing::Values(WIDE, NARROW, ROUNDED),
                         &PrintPopupType);
+
+INSTANTIATE_TEST_CASE_P(,
+                        RoundedOmniboxPopupContentsViewTest,
+                        ::testing::Values(ROUNDED),
+                        &PrintPopupType);

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

@@ -12,6 +12,8 @@
 #include "ui/views/painter.h"
 
 #if defined(USE_AURA)
+#include "ui/aura/window.h"
+#include "ui/aura/window_targeter.h"
 #include "ui/wm/core/shadow.h"
 #include "ui/wm/core/shadow_controller.h"
 #endif
@@ -30,6 +32,18 @@ gfx::Insets GetContentInsets(views::View* location_bar) {
          gfx::Insets(location_bar->height(), 0, 0, 0);
 }
 
+#if defined(USE_AURA)
+// A ui::EventTargeter that allows mouse and touch events in the top portion of
+// the Widget to pass through to the omnibox beneath it.
+class ResultsTargeter : public aura::WindowTargeter {
+ public:
+  explicit ResultsTargeter(int top_inset) {
+    const gfx::Insets event_insets(top_inset, 0, 0, 0);
+    SetInsets(event_insets, event_insets);
+  }
+};
+#endif  // USE_AURA
+
 }  // namespace
 
 constexpr gfx::Insets RoundedOmniboxResultsFrame::kLocationBarAlignmentInsets;
@@ -113,6 +127,9 @@ void RoundedOmniboxResultsFrame::Layout() {
 
 void RoundedOmniboxResultsFrame::AddedToWidget() {
 #if defined(USE_AURA)
+  GetWidget()->GetNativeWindow()->SetEventTargeter(
+      std::make_unique<ResultsTargeter>(content_insets_.top()));
+
   // This works well on ChromeOS. On other platforms, some tricks may be needed
   // to ensure the shadow is not clipped to the parent Widget bounds, since it
   // can peek outside the browser bounds. It is possible to just put the shadow

testing/buildbot/filters/mash.browser_tests.filter

@@ -265,3 +265,4 @@
 
 # Access aura window shadow layer. http://crbug.com/811859
 -OmniboxPopupContentsViewTest.PopupAlignment/Rounded
+-RoundedOmniboxPopupContentsViewTest.*

testing/buildbot/filters/mojo.fyi.mash.browser_tests.filter

@@ -265,3 +265,4 @@
 
 # Access aura window shadow layer. http://crbug.com/811859
 -OmniboxPopupContentsViewTest.PopupAlignment/Rounded
+-RoundedOmniboxPopupContentsViewTest.*