[Mac] Mouse clicks in omnibox close popup.

Previously a right-click in omnibox or on a page action would have a
context menu _and_ the omnibox popup.  Clicking a page action or star
would mess up the omnibox selection.  This brings things in line with
other platforms.

Also fix to not change selection if an omnibox decoration handles the click.

BUG=40145
TEST=Clicking star with omnibox popup up should close popup.
TEST=Clicking star should not change omnibox selection.
TEST=Right-click in omnibox with popup up should close popup.

Review URL: http://codereview.chromium.org/2900006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52126 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 45 insertions, 16 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
index c2aea1f..ae9a900 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
@@ -86,7 +86,6 @@ class AutocompleteEditViewMac : public AutocompleteEditView,
   virtual int GetPasteActionStringId();
   virtual void OnPasteAndGo();
   virtual void OnFrameChanged();
-  virtual void OnDidResignKey();  // Closes the popup.
   virtual void OnDidBeginEditing();
   virtual void OnDidChange();
   virtual void OnDidEndEditing();
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
index a360455..736525d 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
@@ -748,10 +748,6 @@ void AutocompleteEditViewMac::OnKillFocus() {
   controller_->OnKillFocus();
 }
 
-void AutocompleteEditViewMac::OnDidResignKey() {
-  ClosePopup();
-}
-
 bool AutocompleteEditViewMac::CanCopy() {
   const NSRange selection = GetSelectedRange();
   return selection.length > 0;
diff --git a/chrome/browser/cocoa/location_bar/autocomplete_text_field.h b/chrome/browser/cocoa/location_bar/autocomplete_text_field.h
index b4d70aa..808f549 100644
--- a/chrome/browser/cocoa/location_bar/autocomplete_text_field.h
+++ b/chrome/browser/cocoa/location_bar/autocomplete_text_field.h
@@ -63,8 +63,9 @@ class AutocompleteTextFieldObserver {
   // Called when the field's frame changes.
   virtual void OnFrameChanged() = 0;
 
-  // Called when the window containing the field loses focus.
-  virtual void OnDidResignKey() = 0;
+  // Called when the popup is no longer appropriate, such as when the
+  // field's window loses focus or a page action is clicked.
+  virtual void ClosePopup() = 0;
 
   // Called when the user begins editing the field, for every edit,
   // and when the user is done editing the field.
diff --git a/chrome/browser/cocoa/location_bar/autocomplete_text_field.mm b/chrome/browser/cocoa/location_bar/autocomplete_text_field.mm
index 873a2d8..bfc6080 100644
--- a/chrome/browser/cocoa/location_bar/autocomplete_text_field.mm
+++ b/chrome/browser/cocoa/location_bar/autocomplete_text_field.mm
@@ -71,6 +71,12 @@
 // a decoration area and get the expected selection behaviour,
 // likewise for multiple clicks in those areas.
 - (void)mouseDown:(NSEvent*)theEvent {
+  // Close the popup before processing the event.  This prevents the
+  // popup from being visible while a right-click context menu or
+  // page-action menu is visible.  Also, it matches other platforms.
+  if (observer_)
+    observer_->ClosePopup();
+
   // If the click was a Control-click, bring up the context menu.
   // |NSTextField| handles these cases inconsistently if the field is
   // not already first responder.
@@ -282,11 +288,12 @@
   return NO;
 }
 
+// When the window resigns, make sure the autocomplete popup is no
+// longer visible, since the user's focus is elsewhere.
 - (void)windowDidResignKey:(NSNotification*)notification {
   DCHECK_EQ([self window], [notification object]);
-  if (observer_) {
-    observer_->OnDidResignKey();
-  }
+  if (observer_)
+    observer_->ClosePopup();
 }
 
 - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
@@ -314,6 +321,22 @@
   }
 }
 
+// NSTextField becomes first responder by installing a "field editor"
+// subview.  Clicks outside the field editor (such as a decoration)
+// will attempt to make the field the first-responder again, which
+// causes a select-all, even if the decoration handles the click.  If
+// the field editor is already in place, don't accept first responder
+// again.  This allows the selection to be unmodified if the click is
+// handled by a decoration or context menu (|-mouseDown:| will still
+// change it if appropriate).
+- (BOOL)acceptsFirstResponder {
+  if ([self currentEditor]) {
+    DCHECK_EQ([self currentEditor], [[self window] firstResponder]);
+    return NO;
+  }
+  return [super acceptsFirstResponder];
+}
+
 // (Overridden from NSResponder)
 - (BOOL)becomeFirstResponder {
   BOOL doAccept = [super becomeFirstResponder];
diff --git a/chrome/browser/cocoa/location_bar/autocomplete_text_field_editor.mm b/chrome/browser/cocoa/location_bar/autocomplete_text_field_editor.mm
index eb3e077..890d07e 100644
--- a/chrome/browser/cocoa/location_bar/autocomplete_text_field_editor.mm
+++ b/chrome/browser/cocoa/location_bar/autocomplete_text_field_editor.mm
@@ -204,8 +204,9 @@ class Extension;
     [[BrowserWindowController browserWindowControllerForView:field]
         releaseBarVisibilityForOwner:field withAnimation:YES delay:YES];
 
-    if ([field observer])
-      [field observer]->OnKillFocus();
+    AutocompleteTextFieldObserver* observer = [self observer];
+    if (observer)
+      observer->OnKillFocus();
   }
   return doResign;
 }
@@ -314,4 +315,13 @@ class Extension;
   textChangedByKeyEvents_ = NO;
 }
 
+- (void)mouseDown:(NSEvent*)theEvent {
+  // Close the popup before processing the event.
+  AutocompleteTextFieldObserver* observer = [self observer];
+  if (observer)
+    observer->ClosePopup();
+
+  [super mouseDown:theEvent];
+}
+
 @end
diff --git a/chrome/browser/cocoa/location_bar/autocomplete_text_field_unittest.mm b/chrome/browser/cocoa/location_bar/autocomplete_text_field_unittest.mm
index a58fd76..cc78e9f 100644
--- a/chrome/browser/cocoa/location_bar/autocomplete_text_field_unittest.mm
+++ b/chrome/browser/cocoa/location_bar/autocomplete_text_field_unittest.mm
@@ -862,8 +862,8 @@ TEST_F(AutocompleteTextFieldObserverTest, SendsEditingMessages) {
 // in and out of the window.
 // TODO(shess): Should this test the key window for realz?  That would
 // be really annoying to whoever is running the tests.
-TEST_F(AutocompleteTextFieldObserverTest, SendsOnResignKey) {
-  EXPECT_CALL(field_observer_, OnDidResignKey());
+TEST_F(AutocompleteTextFieldObserverTest, ClosePopupOnResignKey) {
+  EXPECT_CALL(field_observer_, ClosePopup());
   [test_window() resignKeyWindow];
 
   scoped_nsobject<AutocompleteTextField> pin([field_ retain]);
@@ -871,7 +871,7 @@ TEST_F(AutocompleteTextFieldObserverTest, SendsOnResignKey) {
   [test_window() resignKeyWindow];
 
   [[test_window() contentView] addSubview:field_];
-  EXPECT_CALL(field_observer_, OnDidResignKey());
+  EXPECT_CALL(field_observer_, ClosePopup());
   [test_window() resignKeyWindow];
 }
 
diff --git a/chrome/browser/cocoa/location_bar/autocomplete_text_field_unittest_helper.h b/chrome/browser/cocoa/location_bar/autocomplete_text_field_unittest_helper.h
index 3483417..3b96e08 100644
--- a/chrome/browser/cocoa/location_bar/autocomplete_text_field_unittest_helper.h
+++ b/chrome/browser/cocoa/location_bar/autocomplete_text_field_unittest_helper.h
@@ -42,7 +42,7 @@ class MockAutocompleteTextFieldObserver : public AutocompleteTextFieldObserver {
   MOCK_METHOD0(GetPasteActionStringId, int());
   MOCK_METHOD0(OnPasteAndGo, void());
   MOCK_METHOD0(OnFrameChanged, void());
-  MOCK_METHOD0(OnDidResignKey, void());
+  MOCK_METHOD0(ClosePopup, void());
   MOCK_METHOD0(OnDidBeginEditing, void());
   MOCK_METHOD0(OnDidChange, void());
   MOCK_METHOD0(OnDidEndEditing, void());