Mac pop-up window happier-ness.

BUG=http://crbug.com/15727, http://crbug.com/16063, http://crbug.com/16329

TEST=A few things.
Bookmark bar test (16063):
- open bookmark bar, then open popup.  Popup should not have bmb.
- close bookmark bar, then open popup.  Popup should not have bmb.
- In main window, toggle bmb a few times.  Popup should not change.
Tab at bottom bug (16329):
- Open 'details' popup in gmail (next to 'last acct activity')
- make sure no tab strip at the bottom of the window
General (15727):
- Open a popup.  Make sure no toolbar (back/fwd buttons et al)

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

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

7 files changed, 78 insertions, 2 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index 4cf5d07..678d4ef 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -122,9 +122,15 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
   NSRect webframe = [webContentView_ frame];
   if (apply) {
     superframe.size.height += kBookmarkBarSuperviewHeightAdjustment;
-    superframe.origin.y -= kBookmarkBarSuperviewHeightAdjustment;
+    // TODO(jrg): y=0 if we add the bookmark bar before the parent
+    // view (toolbar) is placed in the view hierarchy.  A different
+    // CL, where the bookmark bar is extracted from the toolbar nib,
+    // may fix this awkwardness.
+    if (superframe.origin.y > 0) {
+      superframe.origin.y -= kBookmarkBarSuperviewHeightAdjustment;
+      webframe.size.height -= kBookmarkBarWebframeHeightAdjustment;
+    }
     frame.size.height += kBookmarkBarHeight;
-    webframe.size.height -= kBookmarkBarWebframeHeightAdjustment;
   } else {
     superframe.size.height -= kBookmarkBarSuperviewHeightAdjustment;
     superframe.origin.y += kBookmarkBarSuperviewHeightAdjustment;
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index e852d90..1103556 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -59,6 +59,12 @@ TEST_F(BookmarkBarControllerTest, ShowHide) {
   EXPECT_FALSE([bar_ isBookmarkBarVisible]);
   EXPECT_TRUE([[bar_ view] isHidden]);
 
+  // Awkwardness to look like we've been installed.
+  [parent_view_ addSubview:[bar_ view]];
+  NSRect frame = [[[bar_ view] superview] frame];
+  frame.origin.y = 100;
+  [[[bar_ view] superview] setFrame:frame];
+
   // Show and hide it by toggling.
   [bar_ toggleBookmarkBar];
   EXPECT_TRUE([bar_ isBookmarkBarVisible]);
diff --git a/chrome/browser/cocoa/browser_window_controller.h b/chrome/browser/cocoa/browser_window_controller.h
index f837ac6..c2c47c1 100644
--- a/chrome/browser/cocoa/browser_window_controller.h
+++ b/chrome/browser/cocoa/browser_window_controller.h
@@ -31,6 +31,7 @@ class TabContents;
 class TabStripModelObserverBridge;
 @class TabStripView;
 @class ToolbarController;
+@class TitlebarController;
 
 @interface BrowserWindowController :
   TabWindowController<NSUserInterfaceValidations,
@@ -51,6 +52,7 @@ class TabStripModelObserverBridge;
   scoped_ptr<TabStripModelObserverBridge> tabObserver_;
   scoped_ptr<BrowserWindowCocoa> windowShim_;
   scoped_nsobject<ToolbarController> toolbarController_;
+  scoped_nsobject<TitlebarController> titlebarController_;
   scoped_nsobject<TabStripController> tabStripController_;
   scoped_nsobject<FindBarCocoaController> findBarCocoaController_;
   scoped_ptr<StatusBubble> statusBubble_;
diff --git a/chrome/browser/cocoa/tab_window_controller.mm b/chrome/browser/cocoa/tab_window_controller.mm
index 35d9f8b..b9c9479 100644
--- a/chrome/browser/cocoa/tab_window_controller.mm
+++ b/chrome/browser/cocoa/tab_window_controller.mm
@@ -24,6 +24,12 @@
     tabFrame.size.height = NSHeight([tabStripView_ frame]);
     [tabStripView_ setFrame:tabFrame];
     [[[[self window] contentView] superview] addSubview:tabStripView_];
+  } else {
+    // No tabstrip so remove the tabContentArea offset.
+    NSRect tabFrame = [tabContentArea_ frame];
+    NSRect contentFrame = [[[self window] contentView] frame];
+    tabFrame.size.height = contentFrame.size.height;
+    [tabContentArea_ setFrame:tabFrame];
   }
 }
 
diff --git a/chrome/browser/cocoa/toolbar_controller.h b/chrome/browser/cocoa/toolbar_controller.h
index 31c896e..4f9f3d1 100644
--- a/chrome/browser/cocoa/toolbar_controller.h
+++ b/chrome/browser/cocoa/toolbar_controller.h
@@ -54,6 +54,7 @@ class ToolbarView;
   scoped_ptr<ToolbarControllerInternal::PrefObserverBridge> prefObserver_;
   BooleanPrefMember showHomeButton_;
   BooleanPrefMember showPageOptionButtons_;
+  BOOL hasToolbar_;  // if NO, we only have the location bar.
 
   IBOutlet NSMenu* pageMenu_;
   IBOutlet NSMenu* wrenchMenu_;
@@ -106,6 +107,11 @@ class ToolbarView;
 // state.
 - (void)setIsLoading:(BOOL)isLoading;
 
+// Allow turning off the toolbar (but we keep the location bar
+// around).  This changes the behavior of other methods, like
+// [self view].
+- (void)setHasToolbar:(BOOL)toolbar;
+
 // Return the bookmark bar controller.
 - (BookmarkBarController*)bookmarkBarController;
 
diff --git a/chrome/browser/cocoa/toolbar_controller.mm b/chrome/browser/cocoa/toolbar_controller.mm
index d1a44c6..c16ab73 100644
--- a/chrome/browser/cocoa/toolbar_controller.mm
+++ b/chrome/browser/cocoa/toolbar_controller.mm
@@ -86,6 +86,7 @@ class PrefObserverBridge : public NotificationObserver {
     profile_ = profile;
     bookmarkBarDelegate_ = delegate;
     webContentView_ = webContentView;
+    hasToolbar_ = YES;
 
     // Register for notifications about state changes for the toolbar buttons
     commandObserver_.reset(new CommandObserverBridge(self, commands));
@@ -98,6 +99,14 @@ class PrefObserverBridge : public NotificationObserver {
   return self;
 }
 
+- (void)dealloc {
+  // Make sure any code in the base class which assumes [self view] is
+  // the "parent" view continues to work.
+  hasToolbar_ = YES;
+
+  [super dealloc];
+}
+
 // Called after the view is done loading and the outlets have been hooked up.
 // Now we can hook up bridges that rely on UI objects such as the location
 // bar and button state.
@@ -204,7 +213,31 @@ class PrefObserverBridge : public NotificationObserver {
   [goButton_ setTag:tag];
 }
 
+- (void)setHasToolbar:(BOOL)toolbar {
+  [self view];  // force nib loading
+  hasToolbar_ = toolbar;
+
+  // App mode allows turning off the location bar as well.
+  // TODO(???): add more code here when implementing app mode to allow
+  // turning off both toolbar AND location bar.
+
+  // TODO(jrg): add mode code to make the location bar NOT editable
+  // when in a pop-up.
+}
+
+- (NSView*)view {
+  if (hasToolbar_)
+    return [super view];
+  return locationBar_;
+}
+
 - (BookmarkBarController*)bookmarkBarController {
+  // Browser has a FEATURE_BOOKMARKBAR but it is ignored by Safari
+  // when using window.open(); the logic seems to be "if no toolbar,
+  // no bookmark bar".
+  // TODO(jrg): investigate non-Mac Chrome behavior and possibly expand this.
+  if (hasToolbar_ == NO)
+    return nil;
   return bookmarkBarController_.get();
 }
 
diff --git a/chrome/browser/cocoa/toolbar_controller_unittest.mm b/chrome/browser/cocoa/toolbar_controller_unittest.mm
index ed3cf89..58493a2 100644
--- a/chrome/browser/cocoa/toolbar_controller_unittest.mm
+++ b/chrome/browser/cocoa/toolbar_controller_unittest.mm
@@ -74,6 +74,23 @@ TEST_F(ToolbarControllerTest, AwakeFromNibCreatesBMBController) {
   EXPECT_TRUE([bar_ bookmarkBarController]);
 }
 
+// Make sure a "titlebar only" toolbar works
+TEST_F(ToolbarControllerTest, TitlebarOnly) {
+  NSView* view = [bar_ view];
+  EXPECT_TRUE([bar_ bookmarkBarController]);
+
+  [bar_ setHasToolbar:NO];
+  EXPECT_NE(view, [bar_ view]);
+  EXPECT_FALSE([bar_ bookmarkBarController]);
+
+  [bar_ setHasToolbar:YES];
+  EXPECT_EQ(view, [bar_ view]);
+  EXPECT_TRUE([bar_ bookmarkBarController]);
+
+  // Leave it off to make sure that's fine
+  [bar_ setHasToolbar:NO];
+}
+
 
 // Make some changes to the enabled state of a few of the buttons and ensure
 // that we're still in sync.