diff options
author | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-02 03:24:13 +0000 |
---|---|---|
committer | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-02 03:24:13 +0000 |
commit | 3cafc369b174594f0f7e873890aff267233d2209 (patch) | |
tree | 3def7c63b322ae96439fc13c0103ab7407ccd4f0 /chrome/browser/gtk | |
parent | 6de328fddf450413595d75c2fe3cf2c34f7cee4f (diff) | |
download | chromium_src-3cafc369b174594f0f7e873890aff267233d2209.zip chromium_src-3cafc369b174594f0f7e873890aff267233d2209.tar.gz chromium_src-3cafc369b174594f0f7e873890aff267233d2209.tar.bz2 |
Revert 61269 (broke lots of browser and ui tests, like on your try run) - [GTK] delay reference count check in OwnedWidgetGtk
It's a common problem that a GtkWidget will be referenced by an event that's currently on the callstack. These ref holders ignore the "destroy" signal, so when we go to destroy the owned widget, the refcount == 1 DCHECK fails. This change delays that DCHECK until the current stack has unwound, so we don't have to keep running into this.
BUG=none
TEST=in debug builds, Escape in a constrained window doesn't DCHECK, dragging a tab doesn't cause a DCHECK, pressing the custom frame [x] in lucid doesn't DCHECK; in general, no additional DCHECKs
Review URL: http://codereview.chromium.org/3416032
TBR=estade@chromium.org
Review URL: http://codereview.chromium.org/3564007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@61283 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/gtk')
-rw-r--r-- | chrome/browser/gtk/constrained_window_gtk.cc | 7 | ||||
-rw-r--r-- | chrome/browser/gtk/gtk_theme_provider_unittest.cc | 4 | ||||
-rw-r--r-- | chrome/browser/gtk/owned_widget_gtk.cc | 39 | ||||
-rw-r--r-- | chrome/browser/gtk/reload_button_gtk_unittest.cc | 4 | ||||
-rw-r--r-- | chrome/browser/gtk/tabs/tab_gtk.cc | 7 |
5 files changed, 14 insertions, 47 deletions
diff --git a/chrome/browser/gtk/constrained_window_gtk.cc b/chrome/browser/gtk/constrained_window_gtk.cc index bb53e68..b901083 100644 --- a/chrome/browser/gtk/constrained_window_gtk.cc +++ b/chrome/browser/gtk/constrained_window_gtk.cc @@ -70,8 +70,11 @@ TabContentsViewGtk* ConstrainedWindowGtk::ContainingView() { gboolean ConstrainedWindowGtk::OnKeyPress(GtkWidget* sender, GdkEventKey* key) { if (key->keyval == GDK_Escape) { - // This will delete us! - CloseConstrainedWindow(); + // Let the stack unwind so the event handler can release its ref + // on widget(). + MessageLoop::current()->PostTask(FROM_HERE, + factory_.NewRunnableMethod( + &ConstrainedWindowGtk::CloseConstrainedWindow)); return TRUE; } diff --git a/chrome/browser/gtk/gtk_theme_provider_unittest.cc b/chrome/browser/gtk/gtk_theme_provider_unittest.cc index 4779063..1836b80 100644 --- a/chrome/browser/gtk/gtk_theme_provider_unittest.cc +++ b/chrome/browser/gtk/gtk_theme_provider_unittest.cc @@ -4,7 +4,6 @@ #include <gtk/gtk.h> -#include "base/message_loop.h" #include "chrome/browser/gtk/gtk_theme_provider.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profile.h" @@ -37,9 +36,6 @@ class GtkThemeProviderTest : public testing::Test { provider_ = GtkThemeProvider::GetFrom(&profile_); } - private: - MessageLoop message_loop_; - protected: TestingProfile profile_; diff --git a/chrome/browser/gtk/owned_widget_gtk.cc b/chrome/browser/gtk/owned_widget_gtk.cc index c646cd7..2315371 100644 --- a/chrome/browser/gtk/owned_widget_gtk.cc +++ b/chrome/browser/gtk/owned_widget_gtk.cc @@ -6,32 +6,7 @@ #include <gtk/gtk.h> -#include "base/debug_util.h" #include "base/logging.h" -#include "base/message_loop.h" - -namespace { - -class CheckWidgetRefCountTask : public Task { - public: - CheckWidgetRefCountTask(GtkWidget* widget, const std::string& stack) - : widget_(widget), stack_(stack) {} - - virtual ~CheckWidgetRefCountTask() {} - - virtual void Run() { - // NOTE: Assumes some implementation details about glib internals. - DCHECK_EQ(G_OBJECT(widget_)->ref_count, 1U) << - " with Destroy() stack: \n" << stack_; - g_object_unref(widget_); - } - - private: - GtkWidget* widget_; - std::string stack_; -}; - -} // namespace OwnedWidgetGtk::~OwnedWidgetGtk() { DCHECK(!widget_) << "You must explicitly call OwnerWidgetGtk::Destroy()."; @@ -59,16 +34,8 @@ void OwnedWidgetGtk::Destroy() { widget_ = NULL; gtk_widget_destroy(widget); -#ifndef NDEBUG DCHECK(!g_object_is_floating(widget)); - - std::string stack(StackTrace().AsString()); -#else - std::string stack; -#endif - - // Make sure all other ref holders let go of their references (but delay - // the check because some ref holders won't let go immediately). - MessageLoop::current()->PostTask( - FROM_HERE, new CheckWidgetRefCountTask(widget, stack)); + // NOTE: Assumes some implementation details about glib internals. + DCHECK_EQ(G_OBJECT(widget)->ref_count, 1U); + g_object_unref(widget); } diff --git a/chrome/browser/gtk/reload_button_gtk_unittest.cc b/chrome/browser/gtk/reload_button_gtk_unittest.cc index 13c152d..12384b6 100644 --- a/chrome/browser/gtk/reload_button_gtk_unittest.cc +++ b/chrome/browser/gtk/reload_button_gtk_unittest.cc @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/message_loop.h" #include "chrome/browser/gtk/reload_button_gtk.h" #include "testing/gtest/include/gtest/gtest.h" @@ -52,9 +51,6 @@ class ReloadButtonGtkTest : public testing::Test { protected: ReloadButtonGtkTest() : reload_(NULL, NULL), peer_(&reload_) { } - private: - MessageLoop message_loop_; - protected: ReloadButtonGtk reload_; ReloadButtonGtkPeer peer_; diff --git a/chrome/browser/gtk/tabs/tab_gtk.cc b/chrome/browser/gtk/tabs/tab_gtk.cc index 13e6dbb..148e7ea 100644 --- a/chrome/browser/gtk/tabs/tab_gtk.cc +++ b/chrome/browser/gtk/tabs/tab_gtk.cc @@ -388,7 +388,12 @@ void TabGtk::EndDrag(bool canceled) { // Make sure we only run EndDrag once by canceling any tasks that want // to call EndDrag. drag_end_factory_.RevokeAll(); - DestroyDragWidget(); + + // We must let gtk clean up after we handle the drag operation, otherwise + // there will be outstanding references to the drag widget when we try to + // destroy it. + MessageLoop::current()->PostTask(FROM_HERE, + destroy_factory_.NewRunnableMethod(&TabGtk::DestroyDragWidget)); if (last_mouse_down_) { gdk_event_free(last_mouse_down_); |