4 files changed, 197 insertions, 26 deletions

diff --git a/chrome/browser/gtk/bookmark_bubble_gtk.cc b/chrome/browser/gtk/bookmark_bubble_gtk.cc
index 3ea2197..91b4c8d 100644
--- a/chrome/browser/gtk/bookmark_bubble_gtk.cc
+++ b/chrome/browser/gtk/bookmark_bubble_gtk.cc
@@ -10,17 +10,82 @@
 #include "base/logging.h"
 #include "chrome/browser/gtk/info_bubble_gtk.h"
 
+namespace {
+
+// We basically have a singleton, since a bubble is sort of app-modal.  This
+// keeps track of the currently open bubble, or NULL if none is open.
+BookmarkBubbleGtk* g_bubble = NULL;
+
+// TODO(deanm): Just a temporary state to keep track of the last entry in the
+// combo box.  This makes sure we are catching the closing events right and
+// saving the state.
+gint g_last_active = 0;
+
+}  // namespace
+
 // static
 void BookmarkBubbleGtk::Show(const gfx::Rect& rect,
                              Profile* profile,
                              const GURL& url,
                              bool newly_bookmarked) {
+  // TODO(deanm): The Views code deals with the possibility of a bubble already
+  // being open, and then it just does nothing.  I am not sure how this could
+  // happen with the style of our GTK bubble since it has a grab.  I would also
+  // think that closing the previous bubble and opening the new one would make
+  // more sense, but I guess then you would commit the bubble's changes.
+  DCHECK(!g_bubble);
+  g_bubble = new BookmarkBubbleGtk(rect, profile, url, newly_bookmarked);
+}
+
+void BookmarkBubbleGtk::InfoBubbleClosing(InfoBubbleGtk* info_bubble,
+                                          bool closed_by_escape) {
+  // Possibly commit any bookmark changes here...
+  g_last_active = gtk_combo_box_get_active(GTK_COMBO_BOX(combo_));
+}
+
+BookmarkBubbleGtk::BookmarkBubbleGtk(const gfx::Rect& rect,
+                                     Profile* profile,
+                                     const GURL& url,
+                                     bool newly_bookmarked)
+    : profile_(profile),
+      newly_bookmarked_(newly_bookmarked),
+      content_(NULL),
+      combo_(NULL) {
   // TODO(deanm): Implement the real bookmark bubble.  For now we just have
   // a placeholder for testing that input and focus works correctly.
   GtkWidget* content = gtk_vbox_new(FALSE, 5);
   gtk_box_pack_start(GTK_BOX(content), gtk_label_new("Hej!"), TRUE, TRUE, 0);
   gtk_box_pack_start(GTK_BOX(content), gtk_entry_new(), TRUE, TRUE, 0);
-  gtk_box_pack_start(GTK_BOX(content), gtk_entry_new(), TRUE, TRUE, 0);
-  InfoBubbleGtk* bubble = InfoBubbleGtk::Show(rect, content);
-  DCHECK(bubble);
+  // Use a combo box just to make sure popup windows work in the bubble content
+  // and we're not fighting with the bubble for the grab.
+  combo_ = gtk_combo_box_new_text();
+  gtk_combo_box_append_text(GTK_COMBO_BOX(combo_), "entry 1");
+  gtk_combo_box_append_text(GTK_COMBO_BOX(combo_), "entry 2");
+  gtk_combo_box_append_text(GTK_COMBO_BOX(combo_), "entry 3");
+  gtk_combo_box_set_active(GTK_COMBO_BOX(combo_), g_last_active);
+  gtk_box_pack_start(GTK_BOX(content), combo_, TRUE, TRUE, 0);
+
+  g_signal_connect(content, "destroy",
+                   G_CALLBACK(&HandleDestroyThunk), this);
+
+  // TODO(deanm): In the future we might want to hang on to the returned
+  // InfoBubble so that we can call Close() on it.
+  if (!InfoBubbleGtk::Show(rect, content, this)) {
+    NOTREACHED();
+  }
+}
+
+BookmarkBubbleGtk::~BookmarkBubbleGtk() {
+  DCHECK(!content_);  // |content_| should have already been destroyed.
+
+  DCHECK(g_bubble);
+  g_bubble = NULL;
+}
+
+gboolean BookmarkBubbleGtk::HandleDestroy() {
+  // We are self deleting, we have a destroy signal setup to catch when we
+  // destroyed (via the InfoBubble being destroyed), and delete ourself.
+  content_ = NULL;  // We are being destroyed.
+  delete this;
+  return FALSE;  // Propagate.
 }
diff --git a/chrome/browser/gtk/bookmark_bubble_gtk.h b/chrome/browser/gtk/bookmark_bubble_gtk.h
index f456ba1..123ff81 100644
--- a/chrome/browser/gtk/bookmark_bubble_gtk.h
+++ b/chrome/browser/gtk/bookmark_bubble_gtk.h
@@ -2,25 +2,71 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// This is the GTK implementation of the bookmark bubble, the dialog box
+// presented to create or edit a bookmark.  There can only ever be a single
+// bubble open, so the class presents only static methods, and handles the
+// singleton behavior for you.  It also handles the object and widget
+// lifetimes, destroying everything and possibly committing any changes when
+// the bubble is closed.
+
 #ifndef CHROME_BROWSER_GTK_BOOKMARK_BUBBLE_GTK_H_
 #define CHROME_BROWSER_GTK_BOOKMARK_BUBBLE_GTK_H_
 
+#include <gtk/gtk.h>
+
 #include "base/basictypes.h"
+#include "chrome/browser/gtk/info_bubble_gtk.h"
 #include "googleurl/src/gurl.h"
 
-typedef struct _GtkWidget GtkWidget;
-
 class Profile;
 namespace gfx {
 class Rect;
 }
 
-class BookmarkBubbleGtk {
+class BookmarkBubbleGtk : public InfoBubbleGtkDelegate {
  public:
+  // Shows the bookmark bubble, pointing at |rect|.
   static void Show(const gfx::Rect& rect,
                    Profile* profile,
                    const GURL& url,
                    bool newly_bookmarked);
+
+  // Implements the InfoBubbleGtkDelegate.  We are notified when the bubble
+  // is about to be closed, so we have a chance to save any state / input in
+  // our widgets before they are destroyed.
+  virtual void InfoBubbleClosing(InfoBubbleGtk* info_bubble,
+                                 bool closed_by_escape);
+
+ private:
+  BookmarkBubbleGtk(const gfx::Rect& rect,
+                    Profile* profile,
+                    const GURL& url,
+                    bool newly_bookmarked);
+  ~BookmarkBubbleGtk();
+
+  static gboolean HandleDestroyThunk(GtkWidget* widget,
+                                     gpointer userdata) {
+    return reinterpret_cast<BookmarkBubbleGtk*>(userdata)->
+        HandleDestroy();
+  }
+  // Notified when |content_| is destroyed so we can delete our instance.
+  gboolean HandleDestroy();
+  
+  // The URL of the bookmark.
+  GURL url_;
+  // Our current profile (used to access the bookmark system).
+  Profile* profile_;
+  // Whether the bubble is creating or editing an existing bookmark.
+  bool newly_bookmarked_;
+
+  // We let the InfoBubble own our content, and then we delete ourself
+  // when the widget is destroyed (when the InfoBubble is destroyed).
+  GtkWidget* content_;
+
+  // The combo box for selecting the bookmark folder.
+  GtkWidget* combo_;
+
+  DISALLOW_COPY_AND_ASSIGN(BookmarkBubbleGtk);
 };
 
 #endif  // CHROME_BROWSER_GTK_BOOKMARK_BUBBLE_GTK_H_
diff --git a/chrome/browser/gtk/info_bubble_gtk.cc b/chrome/browser/gtk/info_bubble_gtk.cc
index d01bd55..cff694b 100644
--- a/chrome/browser/gtk/info_bubble_gtk.cc
+++ b/chrome/browser/gtk/info_bubble_gtk.cc
@@ -116,17 +116,21 @@ gboolean HandleExpose(GtkWidget* widget,
 }  // namespace
 
 // static
-InfoBubbleGtk* InfoBubbleGtk::Show(const gfx::Rect& rect, GtkWidget* content) {
+InfoBubbleGtk* InfoBubbleGtk::Show(const gfx::Rect& rect,
+                                   GtkWidget* content,
+                                   InfoBubbleGtkDelegate* delegate) {
   InfoBubbleGtk* bubble = new InfoBubbleGtk();
   bubble->Init(rect, content);
+  bubble->set_delegate(delegate);
   return bubble;
 }
 
 InfoBubbleGtk::InfoBubbleGtk()
-    : window_(NULL),
+    : delegate_(NULL),
+      window_(NULL),
       screen_x_(0),
-      screen_y_(0),
-      closed_(false) {
+      screen_y_(0) {
+      
 }
 
 InfoBubbleGtk::~InfoBubbleGtk() {
@@ -173,18 +177,30 @@ void InfoBubbleGtk::Init(const gfx::Rect& rect, GtkWidget* content) {
                    G_CALLBACK(&HandleConfigureThunk), this);
   g_signal_connect(window_, "button-press-event",
                    G_CALLBACK(&HandleButtonPressThunk), this);
+  g_signal_connect(window_, "destroy",
+                   G_CALLBACK(&HandleDestroyThunk), this);
 
   gtk_widget_show_all(window_);
+  // Make sure our window has focus, is brought to the top, etc.
   gtk_window_present(GTK_WINDOW(window_));
+  // We add a GTK (application level) grab.  This means we will get all
+  // keyboard and mouse events for our application, even if they were delivered
+  // on another window.  This allows us to close when the user clicks outside
+  // of the info bubble.  We don't use an X grab since that would steal
+  // keystrokes from your window manager, prevent you from interacting with
+  // other applications, etc.
   gtk_grab_add(window_);
 }
 
-void InfoBubbleGtk::Close() {
-  DCHECK(!closed_);
+void InfoBubbleGtk::Close(bool closed_by_escape) {
+  // Notify the delegate that we're about to close.  This gives the chance
+  // to save state / etc from the hosted widget before it's destroyed.
+  if (delegate_)
+    delegate_->InfoBubbleClosing(this, closed_by_escape);
+
   DCHECK(window_);
   gtk_widget_destroy(window_);
-  window_ = NULL;
-  closed_ = true;
+  // |this| has been deleted, see HandleDestroy.
 }
 
 gboolean InfoBubbleGtk::HandleConfigure(GdkEventConfigure* event) {
@@ -206,3 +222,11 @@ gboolean InfoBubbleGtk::HandleButtonPress(GdkEventButton* event) {
   Close();
   return TRUE;
 }
+
+gboolean InfoBubbleGtk::HandleDestroy() {
+  // We are self deleting, we have a destroy signal setup to catch when we
+  // destroy the widget manually, or the window was closed via X.  This will
+  // delete the InfoBubbleGtk object.
+  delete this;
+  return FALSE;  // Propagate.
+}
diff --git a/chrome/browser/gtk/info_bubble_gtk.h b/chrome/browser/gtk/info_bubble_gtk.h
index d5a3b7b..e88946c 100644
--- a/chrome/browser/gtk/info_bubble_gtk.h
+++ b/chrome/browser/gtk/info_bubble_gtk.h
@@ -2,34 +2,62 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// This is the GTK implementation of InfoBubbles.  InfoBubbles are like
+// dialogs, but they point to a given element on the screen.  You should call
+// InfoBubbleGtk::Show, which will create and display a bubble.  The object is
+// self deleting, when the bubble is closed, you will be notified via
+// InfoBubbleGtkDelegate::InfoBubbleClosing().  Then the widgets and the
+// underlying object will be destroyed.  You can also close and destroy the
+// bubble by calling Close().
+
 #ifndef CHROME_BROWSER_GTK_INFO_BUBBLE_GTK_H_
 #define CHROME_BROWSER_GTK_INFO_BUBBLE_GTK_H_
 
-#include "base/basictypes.h"
-
 #include <gtk/gtk.h>
 
+#include "base/basictypes.h"
+
+class InfoBubbleGtk;
 namespace gfx {
 class Rect;
 }
 
+class InfoBubbleGtkDelegate {
+ public:
+  // Called when the InfoBubble is closing and is about to be deleted.
+  // |closed_by_escape| is true if the close is the result of pressing escape.
+  virtual void InfoBubbleClosing(InfoBubbleGtk* info_bubble,
+                                 bool closed_by_escape) = 0;
+
+  // NOTE: The Views interface has CloseOnEscape, except I can't find a place
+  // where it ever returns false, so we always allow you to close via escape.
+};
+
 class InfoBubbleGtk {
  public:
   // Show an InfoBubble, pointing at the area |rect| (in screen coordinates).
   // An infobubble will try to fit on the screen, so it can point to any edge
-  // of |rect|.  The bubble will host |widget| as the content.
-  static InfoBubbleGtk* Show(const gfx::Rect& rect, GtkWidget* content);
+  // of |rect|.  The bubble will host the |content| widget.  The |delegate|
+  // will be notified when things like closing are happening.
+  static InfoBubbleGtk* Show(const gfx::Rect& rect,
+                             GtkWidget* content,
+                             InfoBubbleGtkDelegate* delegate);
+
+  // Close the bubble if it's open.  This will delete the widgets and object,
+  // so you shouldn't hold a InfoBubbleGtk pointer after calling Close().
+  void Close() { Close(false); }
 
+ private:
   InfoBubbleGtk();
   virtual ~InfoBubbleGtk();
 
-  void Close();
-
- private:
   // Creates the InfoBubble.
   void Init(const gfx::Rect& rect, GtkWidget* content);
 
-  // Closes the window notifying the delegate. |closed_by_escape| is true if
+  // Sets the delegate.
+  void set_delegate(InfoBubbleGtkDelegate* delegate) { delegate_ = delegate; }
+
+  // Closes the window and notifies the delegate. |closed_by_escape| is true if
   // the close is the result of pressing escape.
   void Close(bool closed_by_escape);
 
@@ -56,16 +84,24 @@ class InfoBubbleGtk {
   }
   gboolean HandleButtonRelease(GdkEventButton* event);
 
-  // Our GtkWindow popup window.
+  static gboolean HandleDestroyThunk(GtkWidget* widget,
+                                     gpointer userdata) {
+    return reinterpret_cast<InfoBubbleGtk*>(userdata)->
+        HandleDestroy();
+  }
+  gboolean HandleDestroy();
+
+  // The caller supplied delegate, can be NULL.
+  InfoBubbleGtkDelegate* delegate_;
+
+  // Our GtkWindow popup window, we don't technically "own" the widget, since
+  // it deletes us when it is destroyed.
   GtkWidget* window_;
 
   // Where we want our window to be positioned on the screen.
   int screen_x_;
   int screen_y_;
 
-  // Have we been closed?
-  bool closed_;
-
   DISALLOW_COPY_AND_ASSIGN(InfoBubbleGtk);
 };