Compact Navigation prototype

chrome/browser/ui/views/compact_nav/compact_location_bar_view_host.cc

+// Copyright (c) 2011 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/views/compact_nav/compact_location_bar_view_host.h"
+
+#include <algorithm>
+
+#if defined(OS_LINUX)

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

TOOLKIT_USES_GTK

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

Done.

+#include <gtk/gtk.h>
+#endif
+
+#include "base/i18n/rtl.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/platform_util.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
+#include "chrome/browser/ui/view_ids.h"
+#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
+#include "chrome/browser/ui/views/compact_nav/compact_location_bar_view.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/tabs/base_tab_strip.h"
+#include "content/browser/renderer_host/render_view_host.h"
+#include "content/browser/tab_contents/tab_contents.h"
+#include "content/browser/tab_contents/tab_contents_view.h"
+#include "content/common/notification_source.h"
+#include "ui/base/animation/slide_animation.h"
+#include "ui/base/keycodes/keyboard_codes.h"
+#include "views/controls/scrollbar/native_scroll_bar.h"
+#include "views/events/event.h"
+#include "views/focus/external_focus_tracker.h"
+#include "views/focus/view_storage.h"
+#include "views/widget/root_view.h"
+#include "views/widget/widget.h"
+
+const int kHideTimeoutInSeconds = 2;

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

either use static for these variables, or put in an anonymous namespace

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

Done.

+
+// TODO(stevet): Share this with CompactLocationBarView. This is the actual
+// height, without the overlap added.
+const int kCompactNavbarSpacerHeight = 4;
+
+const int kBookmarkBarLocationBarOverlap = 2;
+const int kSpacerLocationbarOverlap = 1;
+
+#if !defined(OS_MACOSX)  // MessageLoopForUI::Observer isn't available on Mac.

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

views doesn't run on osx, so I don't see any point in this #ifdef.

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

Removed.

+// An mouse event observer to detect a mouse click on
+// BrowserView's content area and hide the location bar.
+class MouseObserver : public MessageLoopForUI::Observer {

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

Don't inline all the methods of this class.

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

Done - Moved definitions out and below.

+ public:
+  MouseObserver(CompactLocationBarViewHost* host, ::BrowserView* view)
+      : host_(host),
+        browser_view_(view),
+        observing_(false) {
+    top_level_window_ = browser_view_->GetWidget()->GetNativeView();
+  }
+

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

Destructor should invoke StopObservering.

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

Done.

+  // MessageLoopForUI::Observer overrides.
+#if defined(OS_WIN)
+  virtual void WillProcessMessage(const MSG& native_event) {}

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

OVERRIDE on all these

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

Done.

+  virtual void DidProcessMessage(const MSG& native_event) {
+#elif defined(OS_LINUX)
+  virtual void WillProcessEvent(GdkEvent* native_event) {}
+  virtual void DidProcessEvent(GdkEvent* native_event) {
+#endif
+    // Hide the location bar iff the mouse is pressed on the
+    // BrowserView's content area.
+    if (!IsMouseEvent(native_event))
+      return;
+    views::MouseEvent event(native_event);
+    if (event.type() == ui::ET_MOUSE_PRESSED &&
+        IsSameTopLevelWindow(native_event) &&
+        HitContentArea(event.x(), event.y())) {
+      host_->Hide(true);

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

What if the user alt-tabs or the active window changes in some other way?

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

In these cases, the drop down will remain in the same state (for example, shown
and with focus). Switching back puts you back in the same state, so the user can
continue doing whatever they were doing with the drop down. We believe that this
is fine.

+    }
+  }
+
+  void Observe(MessageLoopForUI* loop) {
+    if (!observing_) {
+      loop->AddObserver(this);
+      observing_ = true;
+    }
+  }
+
+  void StopObserving(MessageLoopForUI* loop) {
+    if (observing_) {
+      loop->RemoveObserver(this);
+      observing_ = false;
+    }
+  }
+
+ private:
+  // TODO(mad): would be nice to have this on the views::Event class.
+  bool IsMouseEvent(views::NativeEvent native_event) {

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

Can this take a const NativeEvent& ?

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

Sure can!!

+#if defined(OS_WIN)
+    // Copied from views\events\event_win.cc.
+    return native_event.message == WM_MOUSELEAVE ||
+      native_event.message == WM_MOUSEHOVER ||

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

indentation is off, and refactor so you aren't copying it all.

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

Fixed. I've fixed the helpers in event_win.cc to take a const NativeEvent& as
well, and made them accessible.

+      (native_event.message >= WM_MOUSEFIRST &&
+      native_event.message <= WM_MOUSELAST) ||
+      native_event.message == WM_NCMOUSELEAVE ||
+      native_event.message == WM_NCMOUSEHOVER ||
+      (native_event.message >= WM_NCMOUSEMOVE &&
+      native_event.message <= WM_NCXBUTTONDBLCLK);
+#elif defined(OS_LINUX)
+    return native_event->type == GDK_MOTION_NOTIFY ||
+           native_event->type == GDK_BUTTON_PRESS ||
+           native_event->type == GDK_2BUTTON_PRESS ||
+           native_event->type == GDK_3BUTTON_PRESS ||
+           native_event->type == GDK_BUTTON_RELEASE;
+#endif
+  }
+  // TODO(mad): Would be nice to have a NativeEvent -> NativeWindow mapping.

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

newline between 111 and 112. And feel free to add the mapping.

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

Haven't figured this mapping out yet... we'll take another shot when we do the
Gtk implementation.

+  // Then, with a a GetTopLevel receiving a NativeWindow, we could do this
+  // in a platform independent way.
+  bool IsSameTopLevelWindow(views::NativeEvent native_event) {
+#if defined(OS_WIN)
+    return platform_util::GetTopLevel(native_event.hwnd) == top_level_window_;
+#elif defined(OS_LINUX)
+    return gdk_window_get_toplevel(((GdkEventAny*)native_event)->window) ==

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

Use C++ casts

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

Done.

+        top_level_window_->window;
+#endif
+  }
+  // Tests if the event occurred on the content area, using

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

newline bewtween 122 and 123.

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

Done.

+  // root window's coordinates.
+  bool HitContentArea(int x, int y) {
+    gfx::Point p(x, y);
+    // First, exclude the location bar as it's shown on top of
+    // content area.
+    if (HitOnScreen(host_->view(), p)) {
+      return false;
+    }
+    // Treat the bookmark as a content area when it in detached mode.
+    if (browser_view_->GetBookmarkBarView()->IsDetached() &&
+        browser_view_->IsBookmarkBarVisible() &&
+        HitOnScreen(browser_view_->GetBookmarkBarView(), p)) {
+      return true;
+    }
+    if (HitOnScreen(browser_view_->GetContentsView(),

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

put all on one line (don't wrap).

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

Done.

+                    p)) {
+      return true;
+    }
+    return false;
+  }
+
+  // Tests if |p| in the root window's coordinate is within the |view|'s bound.
+  bool HitOnScreen(const views::View* view, const gfx::Point& p) {
+    gfx::Point origin(0, 0);
+    views::View::ConvertPointToScreen(view, &origin);
+    gfx::Rect new_bounds(origin, view->size());
+    return new_bounds.Contains(p);
+  }
+
+  CompactLocationBarViewHost* host_;
+  BrowserView* browser_view_;
+  gfx::NativeView top_level_window_;
+  bool observing_;
+
+  DISALLOW_COPY_AND_ASSIGN(MouseObserver);
+};
+#endif  // !defined(OS_MACOSX)
+
+////////////////////////////////////////////////////////////////////////////////
+// CompactLocationBarViewHost, public:
+
+CompactLocationBarViewHost::CompactLocationBarViewHost(
+      BrowserView* browser_view)

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

indentation is off

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

Done.

+    : DropdownBarHost(browser_view),
+      current_tab_model_index_(-1) {
+  auto_hide_timer_.reset(new base::OneShotTimer<CompactLocationBarViewHost>());
+  mouse_observer_.reset(new MouseObserver(this, browser_view));
+  CompactLocationBarView* clbv = new CompactLocationBarView(this);

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

document who owns clbv

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

Done.

+  Init(clbv, clbv);
+}
+
+CompactLocationBarViewHost::~CompactLocationBarViewHost() {
+  // This may happen if we are destroyed during cleanup.
+  if (browser_view() &&  browser_view()->browser() &&

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

'&&  browser_view' -> '&& browser_view'

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

Done.

+      browser_view()->browser()->tabstrip_model()) {
+    browser_view()->browser()->tabstrip_model()->RemoveObserver(this);
+  }
+  MessageLoopForUI::current()->RemoveObserver(mouse_observer_.get());

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

Remove this and have MouseObservers destructor do it.

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

Done.

+}
+
+void CompactLocationBarViewHost::StartAutoHideTimer() {
+  if (!IsVisible() || HasFocus())
+    return;
+
+  if (auto_hide_timer_->IsRunning()) {
+    // Restart the timer.
+    auto_hide_timer_->Reset();
+  } else {
+    auto_hide_timer_->Start(base::TimeDelta::FromSeconds(kHideTimeoutInSeconds),
+                            this, &CompactLocationBarViewHost::HideCallback);
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// LocationBarView::Delegate implementation:
+TabContentsWrapper* CompactLocationBarViewHost::GetTabContentsWrapper() const {
+  return browser_view()->browser()->GetSelectedTabContentsWrapper();
+}
+
+InstantController* CompactLocationBarViewHost::GetInstant() {
+  return browser_view()->browser()->instant();
+}
+
+void CompactLocationBarViewHost::OnInputInProgress(bool in_progress) {
+

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

remove empty line

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

Done.

+}
+
+////////////////////////////////////////////////////////////////////////////////
+// CompactLocationBarViewHost, views::AcceleratorTarget implementation:
+
+bool CompactLocationBarViewHost::AcceleratorPressed(
+    const views::Accelerator& accelerator) {
+  if (HasFocus()) {
+    DCHECK(view() != NULL);
+    views::FocusManager* focus_manager = view()->GetFocusManager();
+    if (focus_manager)
+      focus_manager->ClearFocus();
+  }
+  Hide(true);
+  return false;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// CompactLocationBarViewHost, views::DropdownBarHost implementation:
+
+gfx::Rect CompactLocationBarViewHost::GetDialogPosition(
+    gfx::Rect avoid_overlapping_rect) {
+  if (!browser_view() || !browser_view()->browser() ||
+    !browser_view()->browser()->tabstrip_model()) {

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

indentation is off

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

Done.

+        return gfx::Rect();
+  }
+  DCHECK_GE(current_tab_model_index_, 0);
+  if (!browser_view()->browser()->tabstrip_model()->ContainsIndex(
+      current_tab_model_index_)) {
+    return gfx::Rect();
+  }
+
+  gfx::Rect new_pos = GetBoundsUnderTab(current_tab_model_index_);
+
+  if (animation_offset() > 0)
+    new_pos.Offset(0, std::min(0, -animation_offset()));
+  return new_pos;
+}
+
+void CompactLocationBarViewHost::SetDialogPosition(const gfx::Rect& new_pos,
+                                                   bool no_redraw) {
+  if (new_pos.IsEmpty())
+    return;
+
+  // Make sure the window edges are clipped to just the visible region. We need
+  // to do this before changing position, so that when we animate the closure
+  // of it it doesn't look like the window crumbles into the toolbar.
+  UpdateWindowEdges(new_pos);
+
+  // TODO(oshima): Animate the window clipping like find-bar.
+  SetWidgetPositionNative(new_pos, no_redraw);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// CompactLocationBarViewHost, views::TabStripModelObserver implementation:
+
+void CompactLocationBarViewHost::TabInsertedAt(TabContentsWrapper* contents,
+                                               int index,
+                                               bool foreground) {
+  if (foreground) {
+    Update(index, true);

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

This case (inserting a tab with foregroudn == true) also sends out
TabSelectedAt. Do you really need to do this here and not in TabSelectedAt? If
you do really need it here, add a comment as to why.

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

Thanks for the pointer - no, we can just call it in TabSelectedAt and we're
fine. Removed this and the header entry.

+    StartAutoHideTimer();
+  }
+}
+
+void CompactLocationBarViewHost::TabClosingAt(TabStripModel* tab_strip_model,
+                                              TabContentsWrapper* contents,
+                                              int index) {
+  // TODO(oshima): We need to relocate the compact navigation bar if,
+  // the removed tab is not the one we are currently under
+  // but the tabstrip does not have the ideal location yet
+  // because the tabs are animating at this time. Need to investigate
+  // the best way to handle this case.
+  Hide(false);

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

Are you sure you want to hide when any tab is closed?

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

The problem with hiding only when it's the current tab (the ideal behaviour) is
noted in Oshima's original TODO, above, which I've reassigned to me. We'll
address this later once we have a better idea of how to handle this.

For now I will avoid hiding it without being able to shift it, since it looks
strange if the active tab shifts to the left.

+}
+
+void CompactLocationBarViewHost::TabSelectedAt(TabContentsWrapper* old_contents,
+                                               TabContentsWrapper* new_contents,
+                                               int index,
+                                               bool user_gesture) {
+  current_tab_model_index_ = index;

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

This can be invoked with the same index. Do you need to handle that case? Maybe
always show then?

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

No, I don't think we need to handle that case with always show. One example of
where it is an issue is when you have two tabs open and you close the left tab.
This actually calls TabSelectedAt on the right tab, with
current_tab_model_index_ == index. This then shows the CLB, which isn't
necessarily what we want to do unless the page in the right tab was still
loading.

If you can think of a scenario where it's invoked with the same index and we
want to always show, let us know.

+  if (new_contents && new_contents->tab_contents()->is_loading()) {
+    Show(false);
+  } else {
+    Hide(false);
+  }
+}
+
+void CompactLocationBarViewHost::TabMoved(TabContentsWrapper* contents,
+                                          int from_index,
+                                          int to_index) {
+  Update(to_index, false);

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

Don't you only care if from_index == current_tab_model_index_ ?

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

Hm, is that expression not always true? It seems to always be true since in
order to move the tab from from_index to to_index, you need to select the tab
first, forcing current_tab_model_index_ to be equal to from_index in the
TabMoved call. Correct me if I am wrong?

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

It's currently true, but I don't think you should assume its always the case.

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

Done.

+  StartAutoHideTimer();
+}
+
+void CompactLocationBarViewHost::TabChangedAt(TabContentsWrapper* contents,
+                                              int index,
+                                              TabChangeType change_type) {
+    if (IsCurrentTabIndex(index) && change_type ==
+        TabStripModelObserver::LOADING_ONLY) {
+    bool was_not_visible = !IsVisible();

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

indentation is off.

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

Done.

+    TabContents* tab_contents = contents->tab_contents();
+    Update(tab_contents, false);
+    if (was_not_visible) {
+      if (tab_contents->is_loading()) {
+        // Register to NavigationController LOAD_STOP so that we can autohide
+        // when loading is done.
+        if (!registrar_.IsRegistered(this, NotificationType::LOAD_STOP,
+            Source<NavigationController>(&tab_contents->controller()))) {
+          registrar_.Add(this, NotificationType::LOAD_STOP,

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

I don't see you removing the observer any where. Don't you need to remove it
when the selection changes or the tab is replaced?

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

We remove the observe below in CompactLocationBarViewHost::Observe, once we
receive the notification we are listening for.

+              Source<NavigationController>(&tab_contents->controller()));
+        }
+      } else {
+        StartAutoHideTimer();
+      }
+    }
+  }
+}
+

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

You should override TabReplacedAt. You may want a similar implementation to that
of TabChangedAt.

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

TabReplacedAt is called when Instant replaces the current tab contents. Not sure
if we want to change the state of the compact location bar at that point.

If we were to call the same code as TabChangedAt, we wouldn't do anything useful
since it just tries to start a timer (to close the location bar). The timer will
either not start because the user is still typing their query in the omnibox, or
it conflicts with the timer that starts if they choose to navigate to a page.

+void CompactLocationBarViewHost::TabReselected(int index) {
+  Update(index, true);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// CompactLocationBarViewHost, NotificationObserver implementation:
+
+void CompactLocationBarViewHost::Observe(NotificationType type,
+                                         const NotificationSource& source,
+                                         const NotificationDetails& details) {
+  switch (type.value) {
+    case NotificationType::LOAD_STOP: {
+      StartAutoHideTimer();
+      // This is one shot deal...
+      registrar_.Remove(this, NotificationType::LOAD_STOP, source);
+      break;
+    }
+    default:
+      NOTREACHED();
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// CompactLocationBarViewHost public:
+
+gfx::Rect CompactLocationBarViewHost::GetBoundsUnderTab(int model_index) const {
+  // Get the position of the left-bottom corner of the tab on the
+  // widget.  The widget of the tab is same as the widget of the
+  // BrowserView which is the parent of the host.
+  BaseTabStrip* tabstrip =

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

What does this code do if the user has enabled side tabs? I suspect we don't
want to allow both at the same time.

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

We have some code in browser.cc that prevents side tabs and cnav mode from being
enabled at the same time. I've added a DCHECK to assert this here.

+      static_cast<BaseTabStrip*>(browser_view()->tabstrip());
+  gfx::Rect tab_bounds =
+      tabstrip->ideal_bounds(tabstrip->ModelIndexToTabIndex(model_index));
+  gfx::Rect navbar_bounds(gfx::Point(tab_bounds.x(), tab_bounds.height()),
+                          view()->GetPreferredSize());
+
+  // For RTL case x() defines tab right corner.
+  if (base::i18n::IsRTL())
+    navbar_bounds.Offset(tab_bounds.width(), 0);
+  navbar_bounds.Offset(tabstrip->x(), tabstrip->y());

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

I'm not sure what coordinates system you want this in. Use convertpointtoXXX to
avoid dependencies upon the view hierarchy. This may also be why you have the
ifdef on 373.

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

Yes, good suggestion. This removes the dependency on browser_view()->y(). We'll
fix this for GTK later if it turns out that their dropdown bars ARE drawn
relative to the browser view.

+
+  // The compact location bar must be smaller than browser_width.
+  int width = std::min(browser_view()->width(),
+                       view()->GetPreferredSize().width());
+
+#if defined(OS_WIN)
+  // The y-coordinate of the browser view is the distance from the top of the
+  // browser frame to the browser view. We need to offset the returned bounds by
+  // this amount since the drop down bar is drawn relative to the frame in
+  // Windows.
+  navbar_bounds.Offset(0, browser_view()->y());
+#endif
+
+  // Try to center around the tab.
+  navbar_bounds.set_x(browser_view()->GetMirroredXInView(
+      navbar_bounds.x()) - ((width - tab_bounds.width()) / 2));
+
+  // Adjust the location to create the illusion that the compact location bar
+  // is a part of the spacer or bookmark bar, depending on which is showing.
+  if (browser_view()->IsBookmarkBarVisible() &&
+      !browser_view()->GetBookmarkBarView()->IsDetached()) {
+    // TODO(stevet): Compact location bar does not have right background image
+    // yet, so kBookmarkBarLocationBarOverlap is tentative. Fix this once UI is
+    // settled. This may be entirely replaced by a popup CLB, anyway.
+    navbar_bounds.Offset(0,
+        browser_view()->GetBookmarkBarView()->bounds().height() +
+        kCompactNavbarSpacerHeight - kBookmarkBarLocationBarOverlap);
+  } else {
+    // TODO(stevet): kSpacerLocationbarOverlap is tentative as well, as above.
+    navbar_bounds.Offset(0, kCompactNavbarSpacerHeight -
+                         kSpacerLocationbarOverlap);
+  }
+
+  // TODO(stevet): Adjust to the right if there is an info bar visible.
+  return navbar_bounds.AdjustToFit(browser_view()->bounds());
+}
+
+void CompactLocationBarViewHost::Update(TabContents* contents, bool animate) {
+  // Don't animate if the bar is already shown.
+  bool showing_in_same_tab = animation()->IsShowing() && IsCurrentTab(contents);
+  current_tab_model_index_ = browser_view()->browser()->active_index();
+  Hide(false);
+  GetCompactLocationBarView()->Update(contents);
+  Show(animate && !showing_in_same_tab);
+  // If the tab is loading, we must wait for the notification that it is done.
+  if (contents && !contents->is_loading()) {
+    // This will be a NOOP if we have focus.
+    // We never want to stay up, unless we have focus.
+    StartAutoHideTimer();
+  }
+}
+
+void CompactLocationBarViewHost::Update(int model_index, bool animate) {

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

Name these methods differently, especially since this one does nothing if
model_index is the current tab.

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

Second method:
Update -> UpdateOnTabChange

+  DCHECK_GE(model_index, 0);
+  if (IsCurrentTabIndex(model_index) && animation()->IsShowing()) {
+    return;
+  }
+  current_tab_model_index_ = model_index;
+  Update(browser_view()->browser()->tabstrip_model()->
+      GetTabContentsAt(model_index)->tab_contents(), animate);
+}
+
+void CompactLocationBarViewHost::CancelAutoHideTimer() {
+  auto_hide_timer_->Stop();
+}
+
+void CompactLocationBarViewHost::SetEnabled(bool enabled) {
+  if (enabled) {

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

What if enabled is already true?

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

Currently we rely on the caller to behave correctly - which is does since
SetEnabled is called by a toggled switch (so we only get true/false
alternating). We do get a DCHECK in AddObserver if we are added to it twice.

If you insist, we can add an additional is_observed_ flag here as an additional
check before we call AddObserver.

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

Yes please.

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

Done.

+    browser_view()->browser()->tabstrip_model()->AddObserver(this);
+  } else {
+    browser_view()->browser()->tabstrip_model()->RemoveObserver(this);
+  }
+}
+
+void CompactLocationBarViewHost::Show(bool animate) {
+  CancelAutoHideTimer();
+  mouse_observer_->Observe(MessageLoopForUI::current());
+  DropdownBarHost::Show(animate);
+  host()->Show();
+}
+
+void CompactLocationBarViewHost::Hide(bool animate) {
+  CancelAutoHideTimer();
+  mouse_observer_->StopObserving(MessageLoopForUI::current());
+  host()->Hide();
+  DropdownBarHost::Hide(animate);
+}
+
+CompactLocationBarView* CompactLocationBarViewHost::
+    GetCompactLocationBarView() {
+  return static_cast<CompactLocationBarView*>(view());
+}
+
+void CompactLocationBarViewHost::MoveWindowIfNecessary(
+    const gfx::Rect& selection_rect,
+    bool no_redraw) {
+  // We only move the window if one is currently shown. If we don't check this,
+  // then SetWidgetPosition below will end up making the Location Bar visible.
+  if (!IsVisible())
+    return;
+
+  gfx::Rect new_pos = GetDialogPosition(selection_rect);
+  SetDialogPosition(new_pos, no_redraw);
+
+  // May need to redraw our frame to accommodate bookmark bar styles.
+  view()->SchedulePaint();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// CompactLocationBarViewHost private:
+bool CompactLocationBarViewHost::HasFocus() {
+  DCHECK(view() != NULL);
+  views::FocusManager* focus_manager = view()->GetFocusManager();
+  return focus_manager && view()->Contains(focus_manager->GetFocusedView());
+}
+
+void CompactLocationBarViewHost::HideCallback() {
+  if (IsVisible() && !HasFocus())
+    Hide(true);
+}
+
+bool CompactLocationBarViewHost::IsCurrentTabIndex(int index) {
+  return current_tab_model_index_ == index;
+}
+
+bool CompactLocationBarViewHost::IsCurrentTab(TabContents* contents) {
+  TabStripModel* tab_strip_model = browser_view()->browser()->tabstrip_model();
+  return tab_strip_model->ContainsIndex(current_tab_model_index_) &&
+      tab_strip_model->GetTabContentsAt(current_tab_model_index_)->
+          tab_contents() == contents;
+}