Mac: Omnibox: Retain the "SelectAll" state after a navigation.

chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm

 // Copyright (c) 2012 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/cocoa/omnibox/omnibox_view_mac.h"
 
 #include <Carbon/Carbon.h>  // kVK_Return
 
 #include "base/mac/foundation_util.h"
 #include "base/metrics/histogram.h"
@@ skipping 201 matching lines @@
 void OmniboxViewMac::ResetTabState(WebContents* web_contents) {
   StoreStateToTab(web_contents, nullptr);
 }
 
 void OmniboxViewMac::Update() {
   if (model()->UpdatePermanentText()) {
     // Something visibly changed.  Re-enable URL replacement.
     controller()->GetToolbarModel()->set_url_replacement_enabled(true);
     model()->UpdatePermanentText();
 
+    const bool was_select_all = IsSelectAll();
+    NSTextView* text_view =
+        base::mac::ObjCCastStrict<NSTextView>([field_ currentEditor]);
+    const bool was_reversed =
+        [text_view selectionAffinity] == NSSelectionAffinityUpstream;
+
     // Restore everything to the baseline look.
     RevertAll();
 
-    // TODO(shess): Figure out how this case is used, to make sure
-    // we're getting the selection and popup right.
+    // Only select all when we have focus.  If we don't have focus, selecting
+    // all is unnecessary since the selection will change on regaining focus,
+    // and can in fact cause artifacts, e.g. if the user is on the NTP and
+    // clicks a link to navigate, causing |was_select_all| to be vacuously true
+    // for the empty omnibox, and we then select all here, leading to the
+    // trailing portion of a long URL being scrolled into view.  We could try
+    // and address cases like this, but it seems better to just not muck with
+    // things when the omnibox isn't focused to begin with.
+    if (was_select_all && model()->has_focus())
+      SelectAll(was_reversed);
   } else {
     // TODO(shess): This corresponds to _win and _gtk, except those
     // guard it with a test for whether the security level changed.
     // But AFAICT, that can only change if the text changed, and that
     // code compares the toolbar model security level with the local
     // security level.  Dig in and figure out why this isn't a no-op
     // that should go away.
     EmphasizeURLComponents();
   }
 }
@@ skipping 99 matching lines @@
     *start = *end = 0;
     return;
   }
 
   const NSRange selected_range = GetSelectedRange();
   *start = static_cast<size_t>(selected_range.location);
   *end = static_cast<size_t>(NSMaxRange(selected_range));
 }
 
 void OmniboxViewMac::SelectAll(bool reversed) {
-  // TODO(shess): Figure out what |reversed| implies.  The gtk version
-  // has it imply inverting the selection front to back, but I don't
-  // even know if that makes sense for Mac.
+  DCHECK(!in_coalesced_update_block_);
+  if (!model()->has_focus())

[Peter Kasting, 2015/05/19 21:32:55]

This line isn't in views -- are you sure it's correct?  I'm wondering when it
will actually trigger.

[tapted, 2015/05/20 00:52:22]

Yeah - this was to keep consistent with the behavior of the SetSelectedRange
call that was here previously. There's a few comments about focus stealing in
here (e.g.  OmniboxViewMac::OnTabChanged), so I thought it best to keep it. (If
it were false, text_view below would be `nil` and the last line would just no-op
in Obj-C's annoying way, but that's a bit too subtle).

I think the difference on Mac is that (for things that aren't WebContents, which
don't use Cocoa's FieldEditor), it's not possible to have text selected without
focus. i.e. selection can only be shown in a FieldEditor, and there is only one
field editor per window (which moves around with focus). Whereas on other
platforms (and inside WebContents), selection is done by RenderText.

I checked all the callers of SelectAll -- they're preceeded by a SetFocus call,
verify that |user_input_in_progress_|, or are
`OmniboxEditModel::OnEscapeKeyPressed()`, so I think focus is generally ensured
here, but on Mac any window control has an opportunity to "refuse" a focus
change away from it, so I'm not sure it can be guaranteed.

+    return;
 
-  // TODO(shess): Verify that we should be stealing focus at this
-  // point.
-  SetSelectedRange(NSMakeRange(0, GetTextLength()));
+  NSTextView* text_view =
+      base::mac::ObjCCastStrict<NSTextView>([field_ currentEditor]);
+  NSSelectionAffinity affinity =
+      reversed ? NSSelectionAffinityUpstream : NSSelectionAffinityDownstream;
+  NSRange range = NSMakeRange(0, GetTextLength());
+
+  [text_view setSelectedRange:range affinity:affinity stillSelecting:NO];
 }
 
 void OmniboxViewMac::RevertAll() {
   OmniboxView::RevertAll();
   [field_ clearUndoChain];
 }
 
 void OmniboxViewMac::UpdatePopup() {
   model()->SetInputInProgress(true);
   if (!model()->has_focus())
@@ skipping 646 matching lines @@
 
 NSUInteger OmniboxViewMac::GetTextLength() const {
   return [field_ currentEditor] ?  [[[field_ currentEditor] string] length] :
                                    [[field_ stringValue] length];
 }
 
 bool OmniboxViewMac::IsCaretAtEnd() const {
   const NSRange selection = GetSelectedRange();
   return NSMaxRange(selection) == GetTextLength();
 }