Move a bunch of our GTK code that was previous manually handling references (and often doing it incorrect / leaking) to use an OwnedWidgetGtk.

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


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

7 files changed, 44 insertions, 40 deletions

diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index 59bd60c..56745bd 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -170,8 +170,8 @@ void BrowserWindowGtk::Init() {
   find_bar_controller_.reset(new FindBarController(find_bar_gtk));
   find_bar_gtk->set_find_bar_controller(find_bar_controller_.get());
 
-  contents_container_.reset(new TabContentsContainerGtk(
-      find_bar_gtk->gtk_widget()));
+  contents_container_.reset(
+      new TabContentsContainerGtk(find_bar_gtk->widget()));
 
   contents_container_->AddContainerToBox(vbox_);
 
@@ -201,14 +201,14 @@ void BrowserWindowGtk::SetBounds(const gfx::Rect& bounds) {
 }
 
 void BrowserWindowGtk::Close() {
-  if (!window_)
-    return;
-
   // TODO(tc): Once the tab strip model is hooked up, this call can be removed.
   // It should get called by TabDetachedAt when the window is being closed, but
   // we don't have a TabStripModel yet.
   find_bar_controller_->ChangeWebContents(NULL);
 
+  if (!window_)
+    return;
+
   GtkWidget* window = GTK_WIDGET(window_);
   // To help catch bugs in any event handlers that might get fired during the
   // destruction, set window_ to NULL before any handlers will run.
@@ -248,6 +248,9 @@ void BrowserWindowGtk::SelectedTabToolbarSizeChanged(bool is_animating) {
 }
 
 void BrowserWindowGtk::UpdateTitleBar() {
+  if (!window_)
+    return;
+
   std::wstring title = browser_->GetCurrentPageTitle();
   gtk_window_set_title(window_, WideToUTF8(title).c_str());
   if (browser_->SupportsWindowFeature(Browser::FEATURE_TITLEBAR)) {
diff --git a/chrome/browser/gtk/find_bar_gtk.cc b/chrome/browser/gtk/find_bar_gtk.cc
index 2398770..f4dc247 100644
--- a/chrome/browser/gtk/find_bar_gtk.cc
+++ b/chrome/browser/gtk/find_bar_gtk.cc
@@ -28,11 +28,12 @@ gboolean KeyPressEvent(GtkWindow* window, GdkEventKey* event,
 }
 
 FindBarGtk::FindBarGtk() {
+  // TODO(tc): Pull out widget creation into an Init() method.
   find_text_ = gtk_entry_new();
   gtk_widget_show(find_text_);
 
-  container_ = gtk_hbox_new(false, 2);
-  gtk_box_pack_end(GTK_BOX(container_), find_text_, FALSE, FALSE, 0);
+  container_.Own(gtk_hbox_new(false, 2));
+  gtk_box_pack_end(GTK_BOX(container_.get()), find_text_, FALSE, FALSE, 0);
 
   g_signal_connect(G_OBJECT(find_text_), "changed", 
                    G_CALLBACK(EntryContentsChanged), this);
@@ -40,15 +41,19 @@ FindBarGtk::FindBarGtk() {
                    G_CALLBACK(KeyPressEvent), this);
 }
 
+FindBarGtk::~FindBarGtk() {
+  container_.Destroy();
+}
+
 void FindBarGtk::Show() {
   // TODO(tc): This should be an animated slide in.
-  gtk_widget_show(container_);
+  gtk_widget_show(container_.get());
   gtk_widget_grab_focus(find_text_);
 }
 
 void FindBarGtk::Hide(bool animate) {
   // TODO(tc): Animated slide away.
-  gtk_widget_hide(container_);
+  gtk_widget_hide(container_.get());
 }
 
 void FindBarGtk::SetFocusAndSelection() {
@@ -76,7 +81,7 @@ void FindBarGtk::SetDialogPosition(const gfx::Rect& new_pos, bool no_redraw) {
 }
 
 bool FindBarGtk::IsFindBarVisible() {
-  return true;
+  return GTK_WIDGET_VISIBLE(container_.get());
 }
 
 void FindBarGtk::RestoreSavedFocus() {
diff --git a/chrome/browser/gtk/find_bar_gtk.h b/chrome/browser/gtk/find_bar_gtk.h
index 11439fb..a9f0ca7 100644
--- a/chrome/browser/gtk/find_bar_gtk.h
+++ b/chrome/browser/gtk/find_bar_gtk.h
@@ -5,13 +5,14 @@
 #ifndef CHROME_BROWSER_GTK_FIND_BAR_GTK_H_
 #define CHROME_BROWSER_GTK_FIND_BAR_GTK_H_
 
-#include "base/basictypes.h"
-#include "chrome/browser/find_bar.h"
-
 #include <gtk/gtk.h>
 
 #include <string>
 
+#include "base/basictypes.h"
+#include "chrome/browser/find_bar.h"
+#include "chrome/common/owned_widget_gtk.h"
+
 class FindBarController;
 class TabContentsContainerGtk;
 class WebContents;
@@ -21,7 +22,7 @@ class WebContents;
 class FindBarGtk : public FindBar {
  public:
   FindBarGtk();
-  virtual ~FindBarGtk() { }
+  virtual ~FindBarGtk();
 
   void set_find_bar_controller(FindBarController* find_bar_controller) {
     find_bar_controller_ = find_bar_controller;
@@ -33,7 +34,7 @@ class FindBarGtk : public FindBar {
   // Callback when Escape is pressed.
   void EscapePressed();
 
-  GtkWidget* gtk_widget() const { return container_; }
+  GtkWidget* widget() const { return container_.get(); }
 
   // Methods from FindBar.
   virtual void Show();
@@ -51,7 +52,7 @@ class FindBarGtk : public FindBar {
 
  private:
   // GtkHBox containing the find bar widgets.
-  GtkWidget* container_;
+  OwnedWidgetGtk container_;
 
   // The widget where text is entered.
   GtkWidget* find_text_;
diff --git a/chrome/browser/gtk/location_bar_view_gtk.cc b/chrome/browser/gtk/location_bar_view_gtk.cc
index 4f6d623..9aa1bb6 100644
--- a/chrome/browser/gtk/location_bar_view_gtk.cc
+++ b/chrome/browser/gtk/location_bar_view_gtk.cc
@@ -33,8 +33,7 @@ const GdkColor kBorderColor = GDK_COLOR_RGB(0xbe, 0xc8, 0xd4);
 
 LocationBarViewGtk::LocationBarViewGtk(CommandUpdater* command_updater,
                                        ToolbarModel* toolbar_model)
-    : outer_bin_(NULL),
-      inner_vbox_(NULL),
+    : inner_vbox_(NULL),
       profile_(NULL),
       command_updater_(command_updater),
       toolbar_model_(toolbar_model),
@@ -43,7 +42,8 @@ LocationBarViewGtk::LocationBarViewGtk(CommandUpdater* command_updater,
 }
 
 LocationBarViewGtk::~LocationBarViewGtk() {
-  gtk_widget_destroy(outer_bin_);
+  // All of our widgets should have be children of / owned by the outer bin.
+  outer_bin_.Destroy();
 }
 
 void LocationBarViewGtk::Init() {
@@ -62,15 +62,12 @@ void LocationBarViewGtk::Init() {
                      TRUE, TRUE, 0);
 
   // Use an alignment to position our bordered location entry exactly.
-  outer_bin_ = gtk_alignment_new(0, 0, 1, 1);
-  gtk_alignment_set_padding(GTK_ALIGNMENT(outer_bin_),
+  outer_bin_.Own(gtk_alignment_new(0, 0, 1, 1));
+  gtk_alignment_set_padding(GTK_ALIGNMENT(outer_bin_.get()),
                             kTopPadding, kBottomPadding, 0, 0);
   gtk_container_add(
-      GTK_CONTAINER(outer_bin_),
+      GTK_CONTAINER(outer_bin_.get()),
       gfx::CreateGtkBorderBin(inner_vbox_, &kBorderColor, 1, 1, 0, 0));
-
-  // Sink the ref so that we own the widget, and will destroy on destruction.
-  g_object_ref_sink(outer_bin_);
 }
 
 void LocationBarViewGtk::SetProfile(Profile* profile) {
diff --git a/chrome/browser/gtk/location_bar_view_gtk.h b/chrome/browser/gtk/location_bar_view_gtk.h
index eb7bbf2..2fc5b0b 100644
--- a/chrome/browser/gtk/location_bar_view_gtk.h
+++ b/chrome/browser/gtk/location_bar_view_gtk.h
@@ -13,6 +13,7 @@
 #include "base/scoped_ptr.h"
 #include "chrome/browser/autocomplete/autocomplete_edit.h"
 #include "chrome/browser/location_bar.h"
+#include "chrome/common/owned_widget_gtk.h"
 #include "chrome/common/page_transition_types.h"
 #include "webkit/glue/window_open_disposition.h"
 
@@ -35,7 +36,7 @@ class LocationBarViewGtk : public AutocompleteEditController,
   void SetProfile(Profile* profile);
 
   // Returns the widget the caller should host.  You must call Init() first.
-  GtkWidget* widget() { return outer_bin_; }
+  GtkWidget* widget() { return outer_bin_.get(); }
 
   // Updates the location bar.  We also reset the bar's permanent text and
   // security style, and, if |tab_for_state_restoring| is non-NULL, also
@@ -65,7 +66,7 @@ class LocationBarViewGtk : public AutocompleteEditController,
 
  private:
   // The outermost widget we want to be hosted.
-  GtkWidget* outer_bin_;
+  OwnedWidgetGtk outer_bin_;
 
   // This is the widget you probably care about, our inner vbox (inside the
   // the border) which holds the elements inside the location bar.
diff --git a/chrome/browser/gtk/menu_gtk.cc b/chrome/browser/gtk/menu_gtk.cc
index 3f8f7a9..fc349cd 100644
--- a/chrome/browser/gtk/menu_gtk.cc
+++ b/chrome/browser/gtk/menu_gtk.cc
@@ -40,19 +40,17 @@ MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
     : delegate_(delegate),
       accel_group_(accel_group),
       menu_(gtk_menu_new()) {
-  g_object_ref_sink(menu_);
-  BuildMenuIn(menu_, menu_data);
+  BuildMenuIn(menu_.get(), menu_data);
 }
 
 MenuGtk::MenuGtk(MenuGtk::Delegate* delegate)
     : delegate_(delegate),
       menu_(gtk_menu_new()) {
-  g_object_ref_sink(menu_);
   BuildMenuFromDelegate();
 }
 
 MenuGtk::~MenuGtk() {
-  g_object_unref(menu_);
+  menu_.Destroy();
 }
 
 void MenuGtk::Popup(GtkWidget* widget, GdkEvent* event) {
@@ -64,20 +62,20 @@ void MenuGtk::Popup(GtkWidget* widget, GdkEvent* event) {
 }
 
 void MenuGtk::Popup(GtkWidget* widget, gint button_type, guint32 timestamp) {
-  gtk_container_foreach(GTK_CONTAINER(menu_), SetMenuItemInfo, this);
+  gtk_container_foreach(GTK_CONTAINER(menu_.get()), SetMenuItemInfo, this);
 
-  gtk_menu_popup(GTK_MENU(menu_), NULL, NULL,
+  gtk_menu_popup(GTK_MENU(menu_.get()), NULL, NULL,
                  MenuPositionFunc,
                  widget,
                  button_type, timestamp);
 }
 
 void MenuGtk::PopupAsContext() {
-  gtk_container_foreach(GTK_CONTAINER(menu_), SetMenuItemInfo, this);
+  gtk_container_foreach(GTK_CONTAINER(menu_.get()), SetMenuItemInfo, this);
 
   // TODO(estade): |button| value of 0 (6th argument) is not strictly true,
   // but does it matter?
-  gtk_menu_popup(GTK_MENU(menu_), NULL, NULL, NULL, NULL, 0,
+  gtk_menu_popup(GTK_MENU(menu_.get()), NULL, NULL, NULL, NULL, 0,
                  gtk_get_current_event_time());
 }
 
@@ -180,7 +178,8 @@ void MenuGtk::BuildMenuFromDelegate() {
                      G_CALLBACK(OnMenuItemActivatedById), this);
 
     gtk_widget_show(menu_item);
-    gtk_menu_append(menu_, menu_item);
+    // TODO(estade): gtk_menu_append is deprecated.
+    gtk_menu_append(menu_.get(), menu_item);
   }
 }
 
diff --git a/chrome/browser/gtk/menu_gtk.h b/chrome/browser/gtk/menu_gtk.h
index 01bdc78..216478a 100644
--- a/chrome/browser/gtk/menu_gtk.h
+++ b/chrome/browser/gtk/menu_gtk.h
@@ -9,6 +9,7 @@
 #include <string>
 
 #include "chrome/browser/gtk/standard_menus.h"
+#include "chrome/common/owned_widget_gtk.h"
 
 class SkBitmap;
 
@@ -93,12 +94,9 @@ class MenuGtk {
   // Accelerator group to add keyboard accelerators to.
   GtkAccelGroup* accel_group_;
 
-  // The window this menu is attached to.
-  GtkWindow* window_;
-
   // gtk_menu_popup() does not appear to take ownership of popup menus, so
   // MenuGtk explicitly manages the lifetime of the menu.
-  GtkWidget* menu_;
+  OwnedWidgetGtk menu_;
 };
 
 #endif  // CHROME_BROWSER_GTK_MENU_GTK_H_