[omnibox] Narrow condition for resetting selection

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

 // Copyright (c) 2013 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 "chrome/browser/ui/views/omnibox/omnibox_view_views.h"
 
 #include <stddef.h>
 
 #include "base/command_line.h"
 #include "base/macros.h"
@@ skipping 283 matching lines @@
   // Pressing tab to focus the omnibox should select all text.
   while (!ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX)) {
     ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_TAB,
                                                 false, false, false, false));
   }
   EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
   EXPECT_TRUE(omnibox_view->IsSelectAll());
 }
 
 IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, CloseOmniboxPopupOnTextDrag) {
-  OmniboxView* omnibox_view = NULL;
+  OmniboxView* omnibox_view = nullptr;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &omnibox_view));
   OmniboxViewViews* omnibox_view_views =
       static_cast<OmniboxViewViews*>(omnibox_view);
 
   // Populate suggestions for the omnibox popup.
   AutocompleteController* autocomplete_controller =
       omnibox_view->model()->popup_model()->autocomplete_controller();
   AutocompleteResult& results = autocomplete_controller->result_;
   ACMatches matches;
-  AutocompleteMatch match;
+  AutocompleteMatch match(nullptr, 500, false,
+                          AutocompleteMatchType::HISTORY_TITLE);
+  match.contents = base::ASCIIToUTF16("http://autocomplete-result/");
+  match.contents_class.push_back(
+      ACMatchClassification(0, ACMatchClassification::URL));
   match.destination_url = GURL("http://autocomplete-result/");
   match.allowed_to_be_default_match = true;
-  match.type = AutocompleteMatchType::HISTORY_TITLE;
-  match.relevance = 500;
-  matches.push_back(match);
-  match.destination_url = GURL("http://autocomplete-result2/");
   matches.push_back(match);
   const AutocompleteInput input(
       base::ASCIIToUTF16("a"), base::string16::npos, std::string(), GURL(),
       metrics::OmniboxEventProto::INVALID_SPEC, false, false, true, true, false,
       TestSchemeClassifier());
   results.AppendMatches(input, matches);
   results.SortAndCull(
       input, TemplateURLServiceFactory::GetForProfile(browser()->profile()));
 
   // The omnibox popup should open with suggestions displayed.
   omnibox_view->model()->popup_model()->OnResultChanged();
   EXPECT_TRUE(omnibox_view->model()->popup_model()->IsOpen());
 
   // The omnibox text should be selected.
   EXPECT_TRUE(omnibox_view->IsSelectAll());
 
   // Simulate a mouse click before dragging the mouse.
-  gfx::Point point(omnibox_view_views->x(), omnibox_view_views->y());
+  gfx::Point point(omnibox_view_views->origin());
   ui::MouseEvent pressed(ui::ET_MOUSE_PRESSED, point, point,
                          ui::EventTimeForNow(), ui::EF_LEFT_MOUSE_BUTTON,
                          ui::EF_LEFT_MOUSE_BUTTON);
   omnibox_view_views->OnMousePressed(pressed);
   EXPECT_TRUE(omnibox_view->model()->popup_model()->IsOpen());
 
   // Simulate a mouse drag of the omnibox text, and the omnibox should close.
   ui::MouseEvent dragged(ui::ET_MOUSE_DRAGGED, point, point,
                          ui::EventTimeForNow(), ui::EF_LEFT_MOUSE_BUTTON, 0);
   omnibox_view_views->OnMouseDragged(dragged);
 
   EXPECT_FALSE(omnibox_view->model()->popup_model()->IsOpen());
 }
 
+IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, MaintainCursorAfterFocusCycle) {
+  OmniboxView* omnibox_view = nullptr;
+  ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &omnibox_view));
+  OmniboxViewViews* omnibox_view_views =
+      static_cast<OmniboxViewViews*>(omnibox_view);
+
+  // Populate suggestions for the omnibox popup.
+  AutocompleteController* autocomplete_controller =
+      omnibox_view->model()->popup_model()->autocomplete_controller();
+  AutocompleteResult& results = autocomplete_controller->result_;
+  ACMatches matches;
+  AutocompleteMatch match(nullptr, 500, false,
+                          AutocompleteMatchType::HISTORY_TITLE);
+  match.contents = base::ASCIIToUTF16("http://autocomplete-result/");
+  match.contents_class.push_back(
+      ACMatchClassification(0, ACMatchClassification::URL));
+  match.destination_url = GURL("http://autocomplete-result/");
+  match.allowed_to_be_default_match = true;
+  matches.push_back(match);
+  const AutocompleteInput input(
+      base::ASCIIToUTF16("autocomplete-result"), 19, "autocomplete-result",
+      GURL("http://autocomplete-result/"),
+      metrics::OmniboxEventProto::INVALID_SPEC, false, false, true, true, false,
+      TestSchemeClassifier());
+  results.AppendMatches(input, matches);
+  results.SortAndCull(
+      input, TemplateURLServiceFactory::GetForProfile(browser()->profile()));
+
+  // The omnibox popup should open with suggestions displayed.
+  omnibox_view->model()->popup_model()->OnResultChanged();
+  EXPECT_TRUE(omnibox_view->model()->popup_model()->IsOpen());
+
+  // The omnibox text should be selected.
+  EXPECT_TRUE(omnibox_view->IsSelectAll());
+
+  // Simulate a mouse click to trigger focus. Click well right of text.

[Peter Kasting, 2017/04/17 17:46:39]

This seems a bit fiddly when all you really want (I think) is to get a focused
location bar with not-select-all.

There are various methods that will do that directly, e.g. call
chrome::FocusLocationBar(browser()); and then
omnibox_view->SetWindowTextAndCaretPos().  If you pass true to |update_popup|,
you can probably trigger autocompletion as well, meaning you could skip
basically everything above as this should substitute for it.

Similarly, below when you want to refocus, you could probably just
FocusLocationBar() and then call omnibox_view->UpdatePopup().

The only thing I'd worry about would be that you might need to "really blur" the
popup in between these, as OnBlur() is just the blur handler, and won't result
in the system actually blurring; and who knows whether that means
FocusLocationBar() will early-exit without "properly" refocusing things.

I'm sorry didn't think to suggest this route sooner, as it ought to be a lot
simpler overall.

[Peter Kasting, 2017/04/17 19:57:23]

Note that calling ClickBrowserWindowCenter() would probably accomplish a "true
blur".

+  gfx::Point point(omnibox_view_views->origin());
+  point = gfx::Point(point.x() + 100, point.y() + 1);
+  ui::MouseEvent pressed(ui::ET_MOUSE_PRESSED, point, point,
+                         ui::EventTimeForNow(), ui::EF_LEFT_MOUSE_BUTTON,
+                         ui::EF_LEFT_MOUSE_BUTTON);
+  omnibox_view_views->OnMousePressed(pressed);
+  omnibox_view_views->OnMouseReleased(pressed);
+  EXPECT_TRUE(omnibox_view->model()->popup_model()->IsOpen());
+
+  // Save cursor position, before blur.
+  size_t prev_start, end;
+  omnibox_view->GetSelectionBounds(&prev_start, &end);
+
+  omnibox_view_views->OnBlur();
+  EXPECT_FALSE(omnibox_view->model()->popup_model()->IsOpen());
+
+  // Redo the above.
+  omnibox_view_views->OnFocus();
+  results.AppendMatches(input, matches);
+  results.SortAndCull(
+      input, TemplateURLServiceFactory::GetForProfile(browser()->profile()));
+  omnibox_view->model()->popup_model()->OnResultChanged();
+  EXPECT_TRUE(omnibox_view->model()->popup_model()->IsOpen());
+
+  // Make sure cursor is restored.
+  size_t start = 0;

[Peter Kasting, 2017/04/17 17:46:39]

Nit: No need to init this, but if you do, init the ones above too

[Kevin Bailey, 2017/04/18 19:45:13]

I like to be paranoid and initialize things such that, if the routine leaves
them alone, the test fails, but maybe this is too paranoid. I took out the
initialization.

+  omnibox_view->GetSelectionBounds(&start, &end);
+  EXPECT_EQ(prev_start, start);
+}
+
 IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, BackgroundIsOpaque) {
   // The omnibox text should be rendered on an opaque background. Otherwise, we
   // can't use subpixel rendering.
   OmniboxViewViews* view = BrowserView::GetBrowserViewForBrowser(browser())->
       toolbar()->location_bar()->omnibox_view();
   ASSERT_TRUE(view);
   EXPECT_FALSE(view->GetRenderText()->subpixel_rendering_suppressed());
 }
 
 // Tests if executing a command hides touch editing handles.
@@ skipping 23 matching lines @@
 IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, FocusedTextInputClient) {
   chrome::FocusLocationBar(browser());
   OmniboxView* view = NULL;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &view));
   OmniboxViewViews* omnibox_view_views = static_cast<OmniboxViewViews*>(view);
   ui::InputMethod* input_method =
       omnibox_view_views->GetWidget()->GetInputMethod();
   EXPECT_EQ(static_cast<ui::TextInputClient*>(omnibox_view_views),
             input_method->GetTextInputClient());
 }