Removing DeleteSoon() from WigetGtk so it behaves like WidgetWin

This is a retry of http://codereview.chromium.org/7002029/ after it has been reverted because of the failing test BrowserTest.CloseWithAppMenuOpen (failed only on buildbot)

BUG=chromium-os:15129
TEST=tests


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

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

3 files changed, 179 insertions, 2 deletions

diff --git a/ui/base/gtk/g_object_destructor_filo.cc b/ui/base/gtk/g_object_destructor_filo.cc
new file mode 100644
index 0000000..3e37f60
--- /dev/null
+++ b/ui/base/gtk/g_object_destructor_filo.cc
@@ -0,0 +1,85 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/base/gtk/g_object_destructor_filo.h"
+
+#include <glib-object.h>
+#include "base/logging.h"
+
+namespace ui {
+
+GObjectDestructorFILO::GObjectDestructorFILO() {
+}
+
+GObjectDestructorFILO::~GObjectDestructorFILO() {
+  // Probably CHECK(handler_map_.empty()) would look natural here. But
+  // some tests (some views_unittests) violate this assertion.
+}
+
+// static
+GObjectDestructorFILO* GObjectDestructorFILO::GetInstance() {
+  return Singleton<GObjectDestructorFILO>::get();
+}
+
+void GObjectDestructorFILO::Connect(
+    GObject* object, DestructorHook callback, void* context) {
+  const Hook hook(object, callback, context);
+  HandlerMap::iterator iter = handler_map_.find(object);
+  if (iter == handler_map_.end()) {
+    g_object_weak_ref(object, WeakNotifyThunk, this);
+    handler_map_[object].push_front(hook);
+  } else {
+    iter->second.push_front(hook);
+  }
+}
+
+void GObjectDestructorFILO::Disconnect(
+    GObject* object, DestructorHook callback, void* context) {
+  HandlerMap::iterator iter = handler_map_.find(object);
+  if (iter == handler_map_.end()) {
+    LOG(DFATAL) << "Unable to disconnect destructor hook for object " << object
+                << ": hook not found (" << callback << ", " << context << ").";
+    return;
+  }
+  HandlerList& dtors = iter->second;
+  if (dtors.empty()) {
+    LOG(DFATAL) << "Destructor list is empty for specified object " << object
+                << " Maybe it is being executed?";
+    return;
+  }
+  if (!dtors.front().equal(object, callback, context)) {
+    LOG(WARNING) << "Destructors should be unregistered the reverse order they "
+                 << "were registered. But for object " << object << " "
+                 << "deleted hook is "<< context << ", the last queued hook is "
+                 << dtors.front().context;
+  }
+  for (HandlerList::iterator i = dtors.begin(); i != dtors.end(); ++i) {
+    if (i->equal(object, callback, context)) {
+      dtors.erase(i);
+      break;
+    }
+  }
+  if (dtors.empty()) {
+    g_object_weak_unref(object, WeakNotifyThunk, this);
+    handler_map_.erase(iter);
+  }
+}
+
+void GObjectDestructorFILO::WeakNotify(GObject* where_the_object_was) {
+  HandlerMap::iterator iter = handler_map_.find(where_the_object_was);
+  DCHECK(iter != handler_map_.end());
+  DCHECK(!iter->second.empty());
+
+  // Save destructor list for given object into local copy to avoid reentrancy
+  // problem: if callee wants to modify the caller list.
+  HandlerList dtors;
+  iter->second.swap(dtors);
+  handler_map_.erase(iter);
+
+  // Execute hooks in local list in FILO order.
+  for (HandlerList::iterator i = dtors.begin(); i != dtors.end(); ++i)
+    i->callback(i->context, where_the_object_was);
+}
+
+}  // napespace ui
diff --git a/ui/base/gtk/g_object_destructor_filo.h b/ui/base/gtk/g_object_destructor_filo.h
new file mode 100644
index 0000000..90e7597
--- /dev/null
+++ b/ui/base/gtk/g_object_destructor_filo.h
@@ -0,0 +1,89 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_BASE_GTK_G_OBJECT_DESTRUCTOR_FILO_H_
+#define UI_BASE_GTK_G_OBJECT_DESTRUCTOR_FILO_H_
+#pragma once
+
+#include <glib.h>
+#include <map>
+#include <list>
+
+#include "base/memory/singleton.h"
+
+typedef struct _GObject GObject;
+
+namespace ui {
+
+// This class hooks calls to g_object_weak_ref()/unref() and executes them in
+// FILO order. This is important if there are several hooks to the single object
+// (set up at different levels of class hierarchy) and the lowest hook (set up
+// first) is deleting self - it must be called last (among hooks for the given
+// object). Unfortunately Glib does not provide this guarantee.
+//
+// Use it as follows:
+//
+// static void OnDestroyedThunk(gpointer data, GObject *where_the_object_was) {
+//   reinterpret_cast<MyClass*>(data)->OnDestroyed(where_the_object_was);
+// }
+// void MyClass::OnDestroyed(GObject *where_the_object_was) {
+//   destroyed_ = true;
+//   delete this;
+// }
+// MyClass::Init() {
+//   ...
+//   ui::GObjectDestructorFILO::GetInstance()->Connect(
+//       G_OBJECT(my_widget), &OnDestroyedThunk, this);
+// }
+// MyClass::~MyClass() {
+//   if (!destroyed_) {
+//     ui::GObjectDestructorFILO::GetInstance()->Disconnect(
+//         G_OBJECT(my_widget), &OnDestroyedThunk, this);
+//   }
+// }
+//
+// TODO(glotov): Probably worth adding ScopedGObjectDtor<T>.
+//
+// This class is a singleton. Not thread safe. Must be called within UI thread.
+class GObjectDestructorFILO {
+ public:
+  typedef void (*DestructorHook)(void* context, GObject* where_the_object_was);
+
+  static GObjectDestructorFILO* GetInstance();
+  void Connect(GObject* object, DestructorHook callback, void* context);
+  void Disconnect(GObject* object, DestructorHook callback, void* context);
+
+ private:
+  struct Hook {
+    Hook(GObject* o, DestructorHook cb, void* ctx)
+        : object(o), callback(cb), context(ctx) {
+    }
+    bool equal(GObject* o, DestructorHook cb, void* ctx) const {
+      return object == o && callback == cb && context == ctx;
+    }
+    GObject* object;
+    DestructorHook callback;
+    void* context;
+  };
+  typedef std::list<Hook> HandlerList;
+  typedef std::map<GObject*, HandlerList> HandlerMap;
+
+  GObjectDestructorFILO();
+  ~GObjectDestructorFILO();
+  friend struct DefaultSingletonTraits<GObjectDestructorFILO>;
+
+  void WeakNotify(GObject* where_the_object_was);
+  static void WeakNotifyThunk(gpointer data, GObject* where_the_object_was) {
+    reinterpret_cast<GObjectDestructorFILO*>(data)->WeakNotify(
+        where_the_object_was);
+  }
+
+  HandlerMap handler_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(GObjectDestructorFILO);
+};
+
+}  // namespace ui
+
+#endif  // UI_BASE_GTK_G_OBJECT_DESTRUCTOR_FILO_H_
diff --git a/ui/base/gtk/gtk_signal_registrar.cc b/ui/base/gtk/gtk_signal_registrar.cc
index c6b170f..9a5846e 100644
--- a/ui/base/gtk/gtk_signal_registrar.cc
+++ b/ui/base/gtk/gtk_signal_registrar.cc
@@ -7,6 +7,7 @@
 #include <glib-object.h>
 
 #include "base/logging.h"
+#include "ui/base/gtk/g_object_destructor_filo.h"
 
 namespace ui {
 
@@ -17,7 +18,8 @@ GtkSignalRegistrar::~GtkSignalRegistrar() {
   for (HandlerMap::iterator list_iter = handler_lists_.begin();
        list_iter != handler_lists_.end(); ++list_iter) {
     GObject* object = list_iter->first;
-    g_object_weak_unref(object, WeakNotifyThunk, this);
+    GObjectDestructorFILO::GetInstance()->Disconnect(
+        object, WeakNotifyThunk, this);
 
     HandlerList& handlers = list_iter->second;
     for (HandlerList::iterator ids_iter = handlers.begin();
@@ -51,7 +53,8 @@ glong GtkSignalRegistrar::ConnectInternal(gpointer instance,
 
   HandlerMap::iterator iter = handler_lists_.find(object);
   if (iter == handler_lists_.end()) {
-    g_object_weak_ref(object, WeakNotifyThunk, this);
+    GObjectDestructorFILO::GetInstance()->Connect(
+        object, WeakNotifyThunk, this);
     handler_lists_[object] = HandlerList();
     iter = handler_lists_.find(object);
   }