Fix two problems with bookmarking and fullscreen mode:

* The bubble didn't close when toggling in/out of fullscreen mode.  This was because we only ever closed on an activation change, rather than on a window move.  Made the bubble close when the window moves.  This required refactoring the bubble code to have a static instance; once I did that I also cleaned up the rest of the code a bit to not check if the bubble was already showing, and just let the bookmark bubble itself take care of this.
* The bubble positioning was wrong in fullscreen mode.  This was because the toolbar layout was in turn wrong in fullscreen mode.  Made the toolbars able to handle being 0-height.

BUG=534
TEST=Bookmark a page and hit F11.  The bubble should disappear.  Hit ctrl-D.  The bubble should appear at the right spot, not floating in the middle of the screen.  Hit F11 again.  The bubble should disappear, and the toolbar button should be drawn undepressed.
Review URL: http://codereview.chromium.org/21479

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

8 files changed, 43 insertions, 31 deletions

diff --git a/chrome/browser/views/bookmark_bubble_view.cc b/chrome/browser/views/bookmark_bubble_view.cc
index cd6b9c9..890594d5 100644
--- a/chrome/browser/views/bookmark_bubble_view.cc
+++ b/chrome/browser/views/bookmark_bubble_view.cc
@@ -110,6 +110,8 @@ void BookmarkBubbleView::RecentlyUsedFoldersModel::RemoveNode(
 
 // BookmarkBubbleView ---------------------------------------------------------
 
+BookmarkBubbleView* BookmarkBubbleView::bubble_ = NULL;
+
 // static
 void BookmarkBubbleView::Show(HWND parent,
                               const gfx::Rect& bounds,
@@ -117,15 +119,27 @@ void BookmarkBubbleView::Show(HWND parent,
                               Profile* profile,
                               const GURL& url,
                               bool newly_bookmarked) {
-  BookmarkBubbleView* view = new BookmarkBubbleView(delegate, profile, url,
-                                                    newly_bookmarked);
-  InfoBubble::Show(parent, bounds, view, view);
+  if (IsShowing())
+    return;
+
+  bubble_ = new BookmarkBubbleView(delegate, profile, url, newly_bookmarked);
+  InfoBubble::Show(parent, bounds, bubble_, bubble_);
   GURL url_ptr(url);
   NotificationService::current()->Notify(
       NotificationType::BOOKMARK_BUBBLE_SHOWN,
       Source<Profile>(profile->GetOriginalProfile()),
       Details<GURL>(&url_ptr));
-  view->BubbleShown();
+  bubble_->BubbleShown();
+}
+
+// static
+bool BookmarkBubbleView::IsShowing() {
+  return bubble_ != NULL;
+}
+
+void BookmarkBubbleView::Hide() {
+  if (IsShowing())
+    bubble_->Close();
 }
 
 BookmarkBubbleView::~BookmarkBubbleView() {
@@ -317,6 +331,11 @@ void BookmarkBubbleView::InfoBubbleClosing(InfoBubble* info_bubble,
     apply_edits_ = false;
   }
 
+  // We have to reset |bubble_| here, not in our destructor, because we'll be
+  // destroyed asynchronously and the shown state will be checked before then.
+  DCHECK(bubble_ == this);
+  bubble_ = NULL;
+
   if (delegate_)
     delegate_->InfoBubbleClosing(info_bubble, closed_by_escape);
   NotificationService::current()->Notify(
diff --git a/chrome/browser/views/bookmark_bubble_view.h b/chrome/browser/views/bookmark_bubble_view.h
index c067f4a..2f0f7c1 100644
--- a/chrome/browser/views/bookmark_bubble_view.h
+++ b/chrome/browser/views/bookmark_bubble_view.h
@@ -40,6 +40,10 @@ class BookmarkBubbleView : public views::View,
                     const GURL& url,
                     bool newly_bookmarked);
 
+  static bool IsShowing();
+
+  static void Hide();
+
   virtual ~BookmarkBubbleView();
 
   // Overriden to force a layout.
@@ -121,6 +125,9 @@ class BookmarkBubbleView : public views::View,
   // Sets the title and parent of the node.
   void ApplyEdits();
 
+  // The bookmark bubble, if we're showing one.
+  static BookmarkBubbleView* bubble_;
+
   // Delegate for the bubble, may be null.
   InfoBubbleDelegate* delegate_;
 
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 8a8b2cf..829c294 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/view_ids.h"
 #include "chrome/browser/views/about_chrome_view.h"
 #include "chrome/browser/views/bookmark_bar_view.h"
+#include "chrome/browser/views/bookmark_bubble_view.h"
 #include "chrome/browser/views/bookmark_manager_view.h"
 #include "chrome/browser/views/bug_report_view.h"
 #include "chrome/browser/views/clear_browsing_data.h"
@@ -233,6 +234,8 @@ void BrowserView::WindowMoved() {
 
   status_bubble_->Reposition();
 
+  BookmarkBubbleView::Hide();
+
   // Close the omnibox popup, if any.
   if (toolbar_->GetLocationBarView())
     toolbar_->GetLocationBarView()->location_entry()->ClosePopup();
@@ -695,10 +698,6 @@ void BrowserView::ShowBookmarkManager() {
   BookmarkManagerView::Show(browser_->profile());
 }
 
-bool BrowserView::IsBookmarkBubbleVisible() const {
-  return toolbar_->star_button()->is_bubble_showing();
-}
-
 void BrowserView::ShowBookmarkBubble(const GURL& url, bool already_bookmarked) {
   toolbar_->star_button()->ShowStarBubble(url, !already_bookmarked);
 }
diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h
index bc7aad7..dccd54d 100644
--- a/chrome/browser/views/frame/browser_view.h
+++ b/chrome/browser/views/frame/browser_view.h
@@ -182,7 +182,6 @@ class BrowserView : public BrowserWindow,
   virtual void ToggleBookmarkBar();
   virtual void ShowAboutChromeDialog();
   virtual void ShowBookmarkManager();
-  virtual bool IsBookmarkBubbleVisible() const;
   virtual void ShowBookmarkBubble(const GURL& url, bool already_bookmarked);
   virtual void ShowReportBugDialog();
   virtual void ShowClearBrowsingDataDialog();
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index 396883c..0ec20a6 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -211,11 +211,11 @@ void LocationBarView::Paint(ChromeCanvas* canvas) {
 
   const SkBitmap* background =
       popup_window_mode_ ? kPopupBackground : kBackground;
-  int top_offset = TopOffset();
+  int top_offset = std::min(TopOffset(), height());
   canvas->TileImageInt(*background, 0, top_offset, 0, 0, width(), height());
   int top_margin = TopMargin();
   canvas->FillRectInt(bg, 0, top_margin, width(),
-                      height() - top_margin - kVertMargin);
+                      std::max(height() - top_margin - kVertMargin, 0));
 }
 
 bool LocationBarView::CanProcessTabKeyEvents() {
@@ -354,7 +354,7 @@ void LocationBarView::DoLayout(const bool force_layout) {
 
   // TODO(sky): baseline layout.
   int location_y = TopMargin();
-  int location_height = height() - location_y - kVertMargin;
+  int location_height = std::max(height() - location_y - kVertMargin, 0);
   if (info_label_.IsVisible()) {
     info_label_.SetBounds(width() - kEntryPadding - info_label_size.width(),
                           location_y,
@@ -396,7 +396,7 @@ int LocationBarView::TopOffset() const {
 }
 
 int LocationBarView::TopMargin() const {
-  return kVertMargin - TopOffset();
+  return std::min(kVertMargin - TopOffset(), height());
 }
 
 int LocationBarView::TextDisplayWidth() {
diff --git a/chrome/browser/views/toolbar_star_toggle.cc b/chrome/browser/views/toolbar_star_toggle.cc
index e4a017f..420605c 100644
--- a/chrome/browser/views/toolbar_star_toggle.cc
+++ b/chrome/browser/views/toolbar_star_toggle.cc
@@ -29,16 +29,10 @@ static const int64 kDisallowClickMS = 40;
 
 ToolbarStarToggle::ToolbarStarToggle(BrowserToolbarView* host)
     : host_(host),
-      ignore_click_(false),
-      is_bubble_showing_(false) {
+      ignore_click_(false) {
 }
 
 void ToolbarStarToggle::ShowStarBubble(const GURL& url, bool newly_bookmarked) {
-  if (is_bubble_showing_) {
-    // Don't show if we're already showing the bubble.
-    return;
-  }
-
   gfx::Point star_location;
   views::View::ConvertPointToScreen(this, &star_location);
   // Shift the x location by 1 as visually the center of the star appears 1
@@ -50,7 +44,6 @@ void ToolbarStarToggle::ShowStarBubble(const GURL& url, bool newly_bookmarked) {
       reinterpret_cast<HWND>(host_->browser()->window()->GetNativeHandle());
   BookmarkBubbleView::Show(parent_hwnd, star_bounds, this, host_->profile(),
                            url, newly_bookmarked);
-  is_bubble_showing_ = true;
 }
 
 bool ToolbarStarToggle::OnMousePressed(const views::MouseEvent& e) {
@@ -71,12 +64,12 @@ void ToolbarStarToggle::OnDragDone() {
 }
 
 void ToolbarStarToggle::NotifyClick(int mouse_event_flags) {
-  if (!ignore_click_ && !is_bubble_showing_)
+  if (!ignore_click_ && !BookmarkBubbleView::IsShowing())
     ToggleButton::NotifyClick(mouse_event_flags);
 }
 
 SkBitmap ToolbarStarToggle::GetImageToPaint() {
-  if (is_bubble_showing_) {
+  if (BookmarkBubbleView::IsShowing()) {
     ResourceBundle &rb = ResourceBundle::GetSharedInstance();
     return *rb.GetBitmapNamed(IDR_STARRED_P);
   }
@@ -85,7 +78,6 @@ SkBitmap ToolbarStarToggle::GetImageToPaint() {
 
 void ToolbarStarToggle::InfoBubbleClosing(InfoBubble* info_bubble,
                                           bool closed_by_escape) {
-  is_bubble_showing_ = false;
   SchedulePaint();
   bubble_closed_time_ = TimeTicks::Now();
 }
diff --git a/chrome/browser/views/toolbar_star_toggle.h b/chrome/browser/views/toolbar_star_toggle.h
index 0553894..f9b0a9a 100644
--- a/chrome/browser/views/toolbar_star_toggle.h
+++ b/chrome/browser/views/toolbar_star_toggle.h
@@ -24,8 +24,6 @@ class ToolbarStarToggle : public views::ToggleButton,
   // If the bubble isn't showing, shows it.
   void ShowStarBubble(const GURL& url, bool newly_bookmarked);
 
-  bool is_bubble_showing() const { return is_bubble_showing_; }
-
   // Overridden to update ignore_click_ based on whether the mouse was clicked
   // quickly after the bubble was hidden.
   virtual bool OnMousePressed(const views::MouseEvent& e);
@@ -57,9 +55,6 @@ class ToolbarStarToggle : public views::ToggleButton,
   // the amount of time between when the bubble clicked and now.
   bool ignore_click_;
 
-  // Is the bubble showing?
-  bool is_bubble_showing_;
-
   DISALLOW_EVIL_CONSTRUCTORS(ToolbarStarToggle);
 };
 
diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc
index acb64ea..360dc817 100644
--- a/chrome/browser/views/toolbar_view.cc
+++ b/chrome/browser/views/toolbar_view.cc
@@ -265,9 +265,10 @@ void BrowserToolbarView::Layout() {
     return;
   }
 
-  int child_y = kControlVertOffset;
+  int child_y = std::min(kControlVertOffset, height());
   // We assume all child elements are the same height.
-  int child_height = go_->GetPreferredSize().height();
+  int child_height =
+      std::min(go_->GetPreferredSize().height(), height() - child_y);
 
   // If the window is maximized, we extend the back button to the left so that
   // clicking on the left-most pixel will activate the back button.