summaryrefslogtreecommitdiffstats
path: root/chrome/browser/gtk
diff options
context:
space:
mode:
authorthakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-10-02 03:24:13 +0000
committerthakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-10-02 03:24:13 +0000
commit3cafc369b174594f0f7e873890aff267233d2209 (patch)
tree3def7c63b322ae96439fc13c0103ab7407ccd4f0 /chrome/browser/gtk
parent6de328fddf450413595d75c2fe3cf2c34f7cee4f (diff)
downloadchromium_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.cc7
-rw-r--r--chrome/browser/gtk/gtk_theme_provider_unittest.cc4
-rw-r--r--chrome/browser/gtk/owned_widget_gtk.cc39
-rw-r--r--chrome/browser/gtk/reload_button_gtk_unittest.cc4
-rw-r--r--chrome/browser/gtk/tabs/tab_gtk.cc7
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_);