[Mac] The autocomplete popup now gets its position from the toolbar controller,

rather than simply growing its width by 2*height.

BUG=None
TEST=The autocomplete popup should continue to appear in the same location.
Review URL: http://codereview.chromium.org/173439

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

11 files changed, 83 insertions, 23 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
index b648e7a..aac879b 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
@@ -19,6 +19,7 @@
 class AutocompleteEditController;
 @class AutocompleteEditHelper;
 class AutocompleteEditModel;
+class AutocompletePopupPositioner;
 class AutocompletePopupViewMac;
 @class AutocompleteTextField;
 class Clipboard;
@@ -32,6 +33,7 @@ class ToolbarModel;
 class AutocompleteEditViewMac : public AutocompleteEditView {
  public:
   AutocompleteEditViewMac(AutocompleteEditController* controller,
+                          AutocompletePopupPositioner* positioner,
                           ToolbarModel* toolbar_model,
                           Profile* profile,
                           CommandUpdater* command_updater,
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
index 6bc065f..d115c88 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
@@ -139,13 +139,14 @@ NSRange ComponentToNSRange(const url_parse::Component& component) {
 // the window |field_| is in.
 AutocompleteEditViewMac::AutocompleteEditViewMac(
     AutocompleteEditController* controller,
+    AutocompletePopupPositioner* positioner,
     ToolbarModel* toolbar_model,
     Profile* profile,
     CommandUpdater* command_updater,
     AutocompleteTextField* field)
     : model_(new AutocompleteEditModel(this, controller, profile)),
-      popup_view_(new AutocompletePopupViewMac(this, model_.get(), profile,
-                                               field)),
+      popup_view_(new AutocompletePopupViewMac(this, model_.get(), positioner,
+                                               profile, field)),
       controller_(controller),
       toolbar_model_(toolbar_model),
       command_updater_(command_updater),
diff --git a/chrome/browser/autocomplete/autocomplete_popup_view.h b/chrome/browser/autocomplete/autocomplete_popup_view.h
index 32cc0df..344ab9c 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_view.h
+++ b/chrome/browser/autocomplete/autocomplete_popup_view.h
@@ -29,6 +29,8 @@ class Profile;
 // bounds for the autocomplete popup view.
 class AutocompletePopupPositioner {
  public:
+  virtual ~AutocompletePopupPositioner() { }
+
   // Returns the bounds at which the popup should be shown, in screen
   // coordinates. The height is ignored, since the popup is sized to its
   // contents automatically.
diff --git a/chrome/browser/autocomplete/autocomplete_popup_view_mac.h b/chrome/browser/autocomplete/autocomplete_popup_view_mac.h
index 7c2b004..d3b7e55 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_view_mac.h
+++ b/chrome/browser/autocomplete/autocomplete_popup_view_mac.h
@@ -33,6 +33,7 @@ class AutocompletePopupViewMac : public AutocompletePopupView {
  public:
   AutocompletePopupViewMac(AutocompleteEditViewMac* edit_view,
                            AutocompleteEditModel* edit_model,
+                           AutocompletePopupPositioner* positioner,
                            Profile* profile,
                            NSTextField* field);
   virtual ~AutocompletePopupViewMac();
@@ -95,7 +96,7 @@ class AutocompletePopupViewMac : public AutocompletePopupView {
 
   scoped_ptr<AutocompletePopupModel> model_;
   AutocompleteEditViewMac* edit_view_;
-
+  AutocompletePopupPositioner* positioner_;  // owned by toolbar controller
   NSTextField* field_;  // owned by tab controller
 
   scoped_nsobject<AutocompleteMatrixTarget> matrix_target_;
diff --git a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
index 9d68b74..9de258f 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
+++ b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
@@ -6,6 +6,7 @@
 
 #include "app/gfx/text_elider.h"
 #include "base/sys_string_conversions.h"
+#include "base/gfx/rect.h"
 #include "chrome/browser/autocomplete/autocomplete_edit.h"
 #include "chrome/browser/autocomplete/autocomplete_edit_view_mac.h"
 #include "chrome/browser/autocomplete/autocomplete_popup_model.h"
@@ -236,10 +237,12 @@ NSAttributedString* AutocompletePopupViewMac::MatchText(
 AutocompletePopupViewMac::AutocompletePopupViewMac(
     AutocompleteEditViewMac* edit_view,
     AutocompleteEditModel* edit_model,
+    AutocompletePopupPositioner* positioner,
     Profile* profile,
     NSTextField* field)
     : model_(new AutocompletePopupModel(this, edit_model, profile)),
       edit_view_(edit_view),
+      positioner_(positioner),
       field_(field),
       matrix_target_([[AutocompleteMatrixTarget alloc] initWithPopupView:this]),
       popup_(nil) {
@@ -316,20 +319,7 @@ void AutocompletePopupViewMac::UpdatePopupAppearance() {
   CreatePopupIfNeeded();
 
   // Layout the popup and size it to land underneath the field.
-  // TODO(shess) Consider refactoring to remove this depenency,
-  // because the popup doesn't need any of the field-like aspects of
-  // field_.  The edit view could expose helper methods for attaching
-  // the window to the field.
-
-  // Locate |field_| on the screen, and pad the left and right sides
-  // by the height to make it wide enough to include the star and go
-  // buttons.
-  // TODO(shess): This assumes that those buttons will be square.
-  // Better to plumb through so that this code can get the rect from
-  // the toolbar controller?
-  NSRect r = [field_ convertRect:[field_ bounds] toView:nil];
-  r.origin.x -= r.size.height;
-  r.size.width += 2 * r.size.height;
+  NSRect r = NSRectFromCGRect(positioner_->GetPopupBounds().ToCGRect());
   r.origin = [[field_ window] convertBaseToScreen:r.origin];
   DCHECK_GT(r.size.width, 0.0);
 
diff --git a/chrome/browser/cocoa/location_bar_view_mac.h b/chrome/browser/cocoa/location_bar_view_mac.h
index 646d31e..6a4099d 100644
--- a/chrome/browser/cocoa/location_bar_view_mac.h
+++ b/chrome/browser/cocoa/location_bar_view_mac.h
@@ -13,6 +13,7 @@
 #include "chrome/browser/autocomplete/autocomplete_edit_view_mac.h"
 #include "chrome/browser/location_bar.h"
 
+class AutocompletePopupPositioner;
 @class AutocompleteTextField;
 class CommandUpdater;
 class Profile;
@@ -27,6 +28,7 @@ class LocationBarViewMac : public AutocompleteEditController,
                            public LocationBarTesting {
  public:
   LocationBarViewMac(AutocompleteTextField* field,
+                     AutocompletePopupPositioner* positioner,
                      CommandUpdater* command_updater,
                      ToolbarModel* toolbar_model,
                      Profile* profile);
diff --git a/chrome/browser/cocoa/location_bar_view_mac.mm b/chrome/browser/cocoa/location_bar_view_mac.mm
index bb23ec8..de3f6ef 100644
--- a/chrome/browser/cocoa/location_bar_view_mac.mm
+++ b/chrome/browser/cocoa/location_bar_view_mac.mm
@@ -46,11 +46,12 @@ std::wstring GetKeywordName(Profile* profile, const std::wstring& keyword) {
 }  // namespace
 
 LocationBarViewMac::LocationBarViewMac(AutocompleteTextField* field,
+                                       AutocompletePopupPositioner* positioner,
                                        CommandUpdater* command_updater,
                                        ToolbarModel* toolbar_model,
                                        Profile* profile)
-  : edit_view_(new AutocompleteEditViewMac(this, toolbar_model, profile,
-                                           command_updater, field)),
+    : edit_view_(new AutocompleteEditViewMac(this, positioner, toolbar_model,
+                                             profile, command_updater, field)),
       command_updater_(command_updater),
       field_(field),
       disposition_(CURRENT_TAB),
diff --git a/chrome/browser/cocoa/location_bar_view_mac_unittest.mm b/chrome/browser/cocoa/location_bar_view_mac_unittest.mm
index cf00ac0..9104001 100644
--- a/chrome/browser/cocoa/location_bar_view_mac_unittest.mm
+++ b/chrome/browser/cocoa/location_bar_view_mac_unittest.mm
@@ -23,7 +23,7 @@ class LocationBarViewMacTest : public testing::Test {
  public:
   LocationBarViewMacTest()
       : field_([[NSTextField alloc] init]),
-        locationBarView_(new LocationBarViewMac(field_, NULL, NULL)) {
+        locationBarView_(new LocationBarViewMac(field_, NULL, NULL, NULL)) {
   }
 
   scoped_nsobject<NSTextField> field_;
diff --git a/chrome/browser/cocoa/toolbar_controller.h b/chrome/browser/cocoa/toolbar_controller.h
index 31aafc4..0e0c8f8 100644
--- a/chrome/browser/cocoa/toolbar_controller.h
+++ b/chrome/browser/cocoa/toolbar_controller.h
@@ -15,6 +15,7 @@
 #import "chrome/browser/cocoa/view_resizer.h"
 #include "chrome/common/pref_member.h"
 
+class AutocompletePopupPositioner;
 @class AutocompleteTextField;
 @class AutocompleteTextFieldEditor;
 @class DelayedMenuButton;
@@ -54,6 +55,8 @@ class ToolbarView;
 
   // Used for monitoring the optional toolbar button prefs.
   scoped_ptr<ToolbarControllerInternal::PrefObserverBridge> prefObserver_;
+  // Used to positioner the omnibox popup view.
+  scoped_ptr<AutocompletePopupPositioner> popupPositioner_;
   BooleanPrefMember showHomeButton_;
   BooleanPrefMember showPageOptionButtons_;
   BOOL hasToolbar_;  // if NO, we only have the location bar.
@@ -140,6 +143,7 @@ class ToolbarView;
 - (NSArray*)toolbarViews;
 - (void)showOptionalHomeButton;
 - (void)showOptionalPageWrenchButtons;
+- (gfx::Rect)autocompletePopupPosition;
 @end
 
 #endif  // CHROME_BROWSER_COCOA_TOOLBAR_CONTROLLER_H_
diff --git a/chrome/browser/cocoa/toolbar_controller.mm b/chrome/browser/cocoa/toolbar_controller.mm
index 1bcf8a4..718eb30 100644
--- a/chrome/browser/cocoa/toolbar_controller.mm
+++ b/chrome/browser/cocoa/toolbar_controller.mm
@@ -6,7 +6,9 @@
 
 #include "base/mac_util.h"
 #include "base/sys_string_conversions.h"
+#include "base/gfx/rect.h"
 #include "chrome/app/chrome_dll_resource.h"
+#include "chrome/browser/autocomplete/autocomplete_popup_view.h"
 #import "chrome/browser/cocoa/autocomplete_text_field.h"
 #import "chrome/browser/cocoa/autocomplete_text_field_editor.h"
 #import "chrome/browser/cocoa/back_forward_menu_controller.h"
@@ -35,6 +37,26 @@ static const float kBookmarkBarOverlap = 5.0;
 - (void)prefChanged:(std::wstring*)prefName;
 @end
 
+namespace {
+
+// A C++ class used to correctly position the autocomplete popup.
+class AutocompletePopupPositionerMac : public AutocompletePopupPositioner {
+ public:
+  AutocompletePopupPositionerMac(ToolbarController* controller)
+      : controller_(controller) { }
+  virtual ~AutocompletePopupPositionerMac() { }
+
+  // Overridden from AutocompletePopupPositioner.
+  virtual gfx::Rect GetPopupBounds() const {
+    return [controller_ autocompletePopupPosition];
+  }
+
+ private:
+  ToolbarController* controller_;  // weak, owns us
+};
+
+}  // namespace
+
 namespace ToolbarControllerInternal {
 
 // A C++ class registered for changes in preferences. Bridges the
@@ -54,7 +76,7 @@ class PrefObserverBridge : public NotificationObserver {
   ToolbarController* controller_;  // weak, owns us
 };
 
-}  // namespace
+}  // namespace ToolbarControllerInternal
 
 @implementation ToolbarController
 
@@ -99,8 +121,11 @@ class PrefObserverBridge : public NotificationObserver {
 // bar and button state.
 - (void)awakeFromNib {
   [self initCommandStatus:commands_];
-  locationBarView_.reset(new LocationBarViewMac(locationBar_, commands_,
-                                                toolbarModel_, profile_));
+  popupPositioner_.reset(new AutocompletePopupPositionerMac(self));
+  locationBarView_.reset(new LocationBarViewMac(locationBar_,
+                                                popupPositioner_.get(),
+                                                commands_, toolbarModel_,
+                                                profile_));
   [locationBar_ setFont:[NSFont systemFontOfSize:[NSFont systemFontSize]]];
 
   // Register pref observers for the optional home and page/options buttons
@@ -378,5 +403,16 @@ class PrefObserverBridge : public NotificationObserver {
                                                 fromView:starButton_];
 }
 
+- (gfx::Rect)autocompletePopupPosition {
+  // The popup should span from the left edge of the star button to the right
+  // edge of the go button.  The returned height is ignored.
+  NSRect locationFrame = [locationBar_ frame];
+  int minX = NSMinX([starButton_ frame]);
+  int maxX = NSMaxX([goButton_ frame]);
+  DCHECK(minX < NSMinX(locationFrame));
+  DCHECK(maxX > NSMaxX(locationFrame));
 
+  NSRect r = NSMakeRect(minX, NSMinY(locationFrame), maxX - minX, 0);
+  return gfx::Rect(NSRectToCGRect([[self view] convertRect:r toView:nil]));
+}
 @end
diff --git a/chrome/browser/cocoa/toolbar_controller_unittest.mm b/chrome/browser/cocoa/toolbar_controller_unittest.mm
index 091bddc..a39481b 100644
--- a/chrome/browser/cocoa/toolbar_controller_unittest.mm
+++ b/chrome/browser/cocoa/toolbar_controller_unittest.mm
@@ -256,5 +256,26 @@ TEST_F(ToolbarControllerTest, StarButtonInWindowCoordinates) {
   EXPECT_TRUE(NSContainsRect(all, star));
 }
 
+TEST_F(ToolbarControllerTest, AutocompletePopupPosition) {
+  NSView* locationBar = [[bar_ toolbarViews] objectAtIndex:kLocationIndex];
+
+  // The window frame (in window base coordinates).
+  NSRect all = [[[bar_ view] window] frame];
+  // The frame of the location bar in window base coordinates.
+  NSRect locationFrame =
+      [locationBar convertRect:[locationBar bounds] toView:nil];
+  // The frame of the popup in window base coordinates.
+  gfx::Rect popupFrame = [bar_ autocompletePopupPosition];
+
+  // Make sure the popup starts to the left of and ends to the right of the
+  // location bar.
+  EXPECT_LT(popupFrame.x(), NSMinX(locationFrame));
+  EXPECT_GT(popupFrame.right(), NSMaxX(locationFrame));
+
+  // Make sure the popup frame is positioned at the bottom of the location bar.
+  EXPECT_EQ(popupFrame.bottom(), NSMinY(locationFrame));
+}
+
+
 
 }  // namespace