Compact Navigation prototype

chrome/browser/ui/views/frame/browser_view_layout.cc

Index: chrome/browser/ui/views/frame/browser_view_layout.cc
diff --git a/chrome/browser/ui/views/frame/browser_view_layout.cc b/chrome/browser/ui/views/frame/browser_view_layout.cc
index eae56ab65dd3d88986e25a73dedf60f0c692b0ec..701077c8fd9dd304d9c2bbff9026de1179649c3c 100644
--- a/chrome/browser/ui/views/frame/browser_view_layout.cc
+++ b/chrome/browser/ui/views/frame/browser_view_layout.cc
@@ -34,6 +34,29 @@ namespace {
 const int kTabShadowSize = 2;
 // The vertical overlap between the TabStrip and the Toolbar.
 const int kToolbarTabStripVerticalOverlap = 3;
+// The vertical size of the space between the content area and the tabstrip that
+// is inserted in compact navigation mode. Note that we need to use a height
+// that includes the overlap to get the visible height we want, in order to
+// match how the toolbar overlaps the tabstrip.
+static const int kCompactNavbarSpacerVisibleHeight = 4;

[sky, 2011/05/03 18:38:32]

no need for static since in anonymouse namespace.

[SteveT, 2011/05/06 18:48:43]

Done.

+static const int kCompactNavbarSpacerHeight =
+    kCompactNavbarSpacerVisibleHeight + kToolbarTabStripVerticalOverlap;
+// The size of the padding between the compact navigation bar and the tab strip.
+static const int kCompactNavbarHorizontalPadding = 2;
+// The number of pixels the bookmark bar should overlap the spacer by if the
+// spacer is visible.
+static const int kSpacerBookmarkBarOverlap = 1;
+
+// Combines View::ConvertPointToView and View::HitTest for a given |point|.
+// Converts |point| from |src| to |dst| and hit tests it against |dst|. The
+// converted |point| can then be retrieved and used for additional tests.
+bool ConvertedHitTest(views::View* src, views::View* dst, gfx::Point* point) {
+  DCHECK(src);
+  DCHECK(dst);
+  DCHECK(point);
+  views::View::ConvertPointToView(src, dst, point);
+  return dst->HitTest(*point);
+}
 
 }  // namespace
 
@@ -138,20 +161,29 @@ int BrowserViewLayout::NonClientHitTest(
   gfx::Point point_in_browser_view_coords(point);
   views::View::ConvertPointToView(
       parent, browser_view_, &point_in_browser_view_coords);
+  gfx::Point test_point(point);
 
   // Determine if the TabStrip exists and is capable of being clicked on. We
   // might be a popup window without a TabStrip.
   if (browser_view_->IsTabStripVisible()) {
     // See if the mouse pointer is within the bounds of the TabStrip.
-    gfx::Point point_in_tabstrip_coords(point);
-    views::View::ConvertPointToView(parent, tabstrip_,
-                                    &point_in_tabstrip_coords);
-    if (tabstrip_->HitTest(point_in_tabstrip_coords)) {
-      if (tabstrip_->IsPositionInWindowCaption(point_in_tabstrip_coords))
+    if (ConvertedHitTest(parent, tabstrip_, &test_point)) {
+      if (tabstrip_->IsPositionInWindowCaption(test_point))
         return HTCAPTION;
       return HTCLIENT;
     }
 
+    // If the tabstrip is visible and we are in compact navigation mode, test
+    // against the compact navigation and option bars.
+    if (browser_view_->UseCompactNavigationBar()) {
+      test_point = point;
+      if (ConvertedHitTest(parent, compact_navigation_bar_, &test_point))
+        return HTCLIENT;
+      test_point = point;
+      if (ConvertedHitTest(parent, compact_options_bar_, &test_point))
+        return HTCLIENT;
+    }
+
     // The top few pixels of the TabStrip are a drop-shadow - as we're pretty
     // starved of dragable area, let's give it to window dragging (this also
     // makes sense visually).
@@ -204,6 +236,9 @@ void BrowserViewLayout::Installed(views::View* host) {
   download_shelf_ = NULL;
   active_bookmark_bar_ = NULL;
   tabstrip_ = NULL;
+  compact_navigation_bar_ = NULL;
+  compact_options_bar_ = NULL;
+  spacer_ = NULL;
   browser_view_ = static_cast<BrowserView*>(host);
 }
 
@@ -234,6 +269,15 @@ void BrowserViewLayout::ViewAdded(views::View* host, views::View* view) {
     case VIEW_ID_TAB_STRIP:
       tabstrip_ = static_cast<AbstractTabStripView*>(view);
       break;
+    case VIEW_ID_SPACER:
+      spacer_ = view;
+      break;
+    case VIEW_ID_COMPACT_NAV_BAR:
+      compact_navigation_bar_ = view;
+      break;
+    case VIEW_ID_COMPACT_OPT_BAR:
+      compact_options_bar_ = view;
+      break;
   }
 }
 
@@ -247,7 +291,7 @@ void BrowserViewLayout::ViewRemoved(views::View* host, views::View* view) {
 
 void BrowserViewLayout::Layout(views::View* host) {
   vertical_layout_rect_ = browser_view_->GetLocalBounds();
-  int top = LayoutTabStrip();
+  int top = LayoutTabStripRegion();
   if (browser_view_->IsTabStripVisible() && !browser_view_->UseVerticalTabs()) {
     tabstrip_->SetBackgroundOffset(gfx::Point(
         tabstrip_->GetMirroredX() + browser_view_->GetMirroredX(),
@@ -268,6 +312,11 @@ void BrowserViewLayout::Layout(views::View* host) {
     browser()->GetFindBarController()->find_bar()->MoveWindowIfNecessary(
         gfx::Rect(), true);
   }
+  if (browser()->UseCompactNavigationBar()) {
+    DCHECK(browser_view_->compact_location_bar_view_host());
+    browser_view_->compact_location_bar_view_host()->MoveWindowIfNecessary(
+        gfx::Rect(), true);
+  }
 }
 
 // Return the preferred size which is the size required to give each
@@ -287,20 +336,52 @@ const Browser* BrowserViewLayout::browser() const {
   return browser_view_->browser();
 }
 
-int BrowserViewLayout::LayoutTabStrip() {
+int BrowserViewLayout::LayoutTabStripRegion() {
   if (!browser_view_->IsTabStripVisible()) {
+    compact_navigation_bar_->SetVisible(false);

[sky, 2011/05/03 18:38:32]

Are we really always creating the compact nav classes? We should only create it
if needed.

[SteveT, 2011/05/06 18:48:43]

I agree with you here, though I am having troubles accomplishing this. Maybe you
can shed some light on this. Our compact navigation classes are instantiated in
BrowserView::Init along side the rest of the views placed in the BrowserView.

It appears that at this point in the code, the browser prefs have not yet been
loaded. So even if I know my Chrome has the cnav flag set to true, it is
initially false when I try to read it in BrowserView::Init. So the classes can't
be instantiated. Later when we Toggle cnav mode on, the browser will crash
because it expects to be able to use these classes, but they're not there.

Any idea how we can get to these prefs early on in BrowserView::Init? Or is
there somewhere else where it's appropriate to instantiate views that are owned
by BrowserView?

[sky, 2011/05/06 19:56:06]

Are you sure about that? The code for choosing side tab should looks nearly
identical, and as far as I know doesn't have problems. Where do you end up if in
Init you try to call UseCompactNavigationBar?

[SteveT, 2011/05/06 22:13:40]

Ah, this was my bad - I was checking the pref (context menu switch) rather than
the actual switch (from about:flags). So we had states where the switch was on,
but the pref was off. I fixed this now.

So what we do is if the about:flags value is Enabled, we instantiate the class.
And then we only show them (checking carefully for nullness) if the tab context
menu item is checked.

I've also renamed the TabMenuModel accessor to the switch to
UseCompactNavigationMODE, for extra clarity.

Thanks for the vigilance on this one.

+    compact_options_bar_->SetVisible(false);
     tabstrip_->SetVisible(false);
     tabstrip_->SetBounds(0, 0, 0, 0);
     return 0;
   }
 
+  // This retrieves the bounds for the tab strip based on whether or not we show
+  // anything to the left of it, like the incognito avatar.
   gfx::Rect tabstrip_bounds(
       browser_view_->frame()->GetBoundsForTabStrip(tabstrip_));
   gfx::Point tabstrip_origin(tabstrip_bounds.origin());
   views::View::ConvertPointToView(browser_view_->parent(), browser_view_,
-                                  &tabstrip_origin);

[sky, 2011/05/03 18:38:32]

This spacing is preferable; leave it.

[SteveT, 2011/05/06 18:48:43]

Done.

+      &tabstrip_origin);
   tabstrip_bounds.set_origin(tabstrip_origin);
 
+  // If we are in compact nav mode, we want to reduce the tab strip bounds from
+  // both ends enough to lay out the compact navigation and options bars.
+  compact_navigation_bar_->SetVisible(browser_view_->UseCompactNavigationBar());
+  compact_options_bar_->SetVisible(browser_view_->UseCompactNavigationBar());
+  if (compact_navigation_bar_->IsVisible()) {
+    gfx::Rect cnav_bar_bounds;
+    gfx::Size cnav_bar_size = compact_navigation_bar_->GetPreferredSize();
+    cnav_bar_bounds.set_origin(tabstrip_bounds.origin());
+    cnav_bar_bounds.set_size(cnav_bar_size);
+    compact_navigation_bar_->SetBoundsRect(cnav_bar_bounds);
+
+    // The options bar is flush right of the tab strip region.
+    gfx::Rect copt_bar_bounds;
+    gfx::Size copt_bar_size = compact_options_bar_->GetPreferredSize();
+    copt_bar_bounds.set_x(std::max(0, tabstrip_bounds.right() -
+                          copt_bar_size.width()));
+    copt_bar_bounds.set_y(tabstrip_origin.y());
+    copt_bar_bounds.set_size(copt_bar_size);
+    compact_options_bar_->SetBoundsRect(copt_bar_bounds);
+
+    // Reduce the bounds of the tab strip accordingly.
+    tabstrip_bounds.set_x(tabstrip_bounds.x() + cnav_bar_size.width() +
+        kCompactNavbarHorizontalPadding);
+    tabstrip_bounds.set_width(std::max(0, tabstrip_bounds.width() -
+        cnav_bar_size.width() - copt_bar_size.width() -
+        kCompactNavbarHorizontalPadding * 2));
+  }
+
   if (browser_view_->UseVerticalTabs())
     vertical_layout_rect_.Inset(tabstrip_bounds.width(), 0, 0, 0);
 
@@ -312,16 +393,26 @@ int BrowserViewLayout::LayoutTabStrip() {
 
 int BrowserViewLayout::LayoutToolbar(int top) {
   int browser_view_width = vertical_layout_rect_.width();
-  bool visible = browser_view_->IsToolbarVisible();
-  toolbar_->location_bar()->SetFocusable(visible);
+  bool toolbar_visible = browser_view_->IsToolbarVisible();
+  toolbar_->location_bar()->SetFocusable(toolbar_visible);
   int y = top;
   if (!browser_view_->UseVerticalTabs()) {
-    y -= ((visible && browser_view_->IsTabStripVisible()) ?
-          kToolbarTabStripVerticalOverlap : 0);
+    y -= ((toolbar_visible || browser_view_->UseCompactNavigationBar()) &&
+           browser_view_->IsTabStripVisible()) ?
+           kToolbarTabStripVerticalOverlap : 0;
   }
-  int height = visible ? toolbar_->GetPreferredSize().height() : 0;
-  toolbar_->SetVisible(visible);
+  int height = toolbar_visible ? toolbar_->GetPreferredSize().height() : 0;
+  toolbar_->SetVisible(toolbar_visible);
   toolbar_->SetBounds(vertical_layout_rect_.x(), y, browser_view_width, height);
+
+  // The spacer essentially replaces the toolbar when in compact mode.
+  if (browser_view_->UseCompactNavigationBar()) {
+    spacer_->SetVisible(!toolbar_visible);
+    spacer_->SetBounds(vertical_layout_rect_.x(), y, browser_view_width,
+                       toolbar_visible ? 0 : kCompactNavbarSpacerHeight);
+    height = kCompactNavbarSpacerHeight;
+  }
+
   return y + height;
 }
 
@@ -351,8 +442,12 @@ int BrowserViewLayout::LayoutBookmarkBar(int top) {
 
   active_bookmark_bar_->set_infobar_visible(InfobarVisible());
   int bookmark_bar_height = active_bookmark_bar_->GetPreferredSize().height();
-  y -= views::NonClientFrameView::kClientEdgeThickness +
-      active_bookmark_bar_->GetToolbarOverlap(false);
+  if (!browser_view_->UseCompactNavigationBar()) {
+    y -= views::NonClientFrameView::kClientEdgeThickness +
+        active_bookmark_bar_->GetToolbarOverlap(false);
+  } else {
+    y -= kSpacerBookmarkBarOverlap;
+  }
   active_bookmark_bar_->SetVisible(true);
   active_bookmark_bar_->SetBounds(vertical_layout_rect_.x(), y,
                                   vertical_layout_rect_.width(),