From 3e5f6017799f7f88e850d747ff4fea0234027b13 Mon Sep 17 00:00:00 2001
From: "satorux@chromium.org"
<satorux@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Mon, 12 Sep 2011 18:17:29 +0000
Subject: Fix a memory leak caused by IBusController
IBusController used to be a singleton, but this change lets
IBusController be owned by InputMethodManager.
FWIW, here's the command line to run a test in the browser_tests:
% sh tools/valgrind/chrome_tests.sh --build_dir=out/Release -t browser --gtest_filter=ExtensionApiTest.InputMethodApiBasic
BUG=chromium:86146
TEST=ran a browser test locally and the leak is gone after the change. Alos tested on the device to confirm that input methods worked as before.
Review URL: http://codereview.chromium.org/7864023
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@100723 0039d316-1c4b-4281-b951-d872f2087c98
---
.../chromeos/input_method/ibus_controller.cc | 76 +++++++++++++---------
.../chromeos/input_method/input_method_manager.cc | 8 +--
tools/valgrind/memcheck/suppressions.txt | 7 --
3 files changed, 50 insertions(+), 41 deletions(-)
diff --git a/chrome/browser/chromeos/input_method/ibus_controller.cc b/chrome/browser/chromeos/input_method/ibus_controller.cc
index 93b63d1..a776d2c 100644
--- a/chrome/browser/chromeos/input_method/ibus_controller.cc
+++ b/chrome/browser/chromeos/input_method/ibus_controller.cc
@@ -537,10 +537,51 @@ std::string PrintPropList(IBusPropList *prop_list, int tree_level) {
// The real implementation of the IBusController.
class IBusControllerImpl : public IBusController {
public:
- // TODO(satorux,yusukes): Remove use of singleton here.
- static IBusControllerImpl* GetInstance() {
- return Singleton<IBusControllerImpl,
- LeakySingletonTraits<IBusControllerImpl> >::get();
+ IBusControllerImpl()
+ : ibus_(NULL),
+ ibus_config_(NULL) {
+ }
+
+ ~IBusControllerImpl() {
+ // Disconnect signals so the handler functions will not be called with
+ // |this| which is already freed.
+ if (ibus_) {
+ g_signal_handlers_disconnect_by_func(
+ ibus_,
+ reinterpret_cast<gpointer>(G_CALLBACK(IBusBusConnectedThunk)),
+ this);
+ g_signal_handlers_disconnect_by_func(
+ ibus_,
+ reinterpret_cast<gpointer>(G_CALLBACK(IBusBusDisconnectedThunk)),
+ this);
+ g_signal_handlers_disconnect_by_func(
+ ibus_,
+ reinterpret_cast<gpointer>(
+ G_CALLBACK(IBusBusGlobalEngineChangedThunk)),
+ this);
+ g_signal_handlers_disconnect_by_func(
+ ibus_,
+ reinterpret_cast<gpointer>(G_CALLBACK(IBusBusNameOwnerChangedThunk)),
+ this);
+
+ // Disconnect signals for the panel service as well.
+ IBusPanelService* ibus_panel_service = IBUS_PANEL_SERVICE(
+ g_object_get_data(G_OBJECT(ibus_), kPanelObjectKey));
+ if (ibus_panel_service) {
+ g_signal_handlers_disconnect_by_func(
+ ibus_panel_service,
+ reinterpret_cast<gpointer>(G_CALLBACK(FocusInThunk)),
+ this);
+ g_signal_handlers_disconnect_by_func(
+ ibus_panel_service,
+ reinterpret_cast<gpointer>(G_CALLBACK(RegisterPropertiesThunk)),
+ this);
+ g_signal_handlers_disconnect_by_func(
+ ibus_panel_service,
+ reinterpret_cast<gpointer>(G_CALLBACK(UpdatePropertyThunk)),
+ this);
+ }
+ }
}
virtual void Connect() {
@@ -801,31 +842,6 @@ class IBusControllerImpl : public IBusController {
->UpdateProperty(sender, ibus_prop);
}
- friend struct DefaultSingletonTraits<IBusControllerImpl>;
- IBusControllerImpl()
- : ibus_(NULL),
- ibus_config_(NULL) {
- }
-
- ~IBusControllerImpl() {
- // Since the class is used as a leaky singleton, this destructor is never
- // called. However, if you would delete an instance of this class, you have
- // to disconnect all signals so the handler functions will not be called
- // with |this| which is already freed.
- //
- // if (ibus_) {
- // g_signal_handlers_disconnect_by_func(
- // ibus_,
- // reinterpret_cast<gpointer>(
- // G_CALLBACK(IBusBusConnectedCallback)),
- // this);
- // ...
- // }
- // if (ibus_panel_service_) {
- // g_signal_handlers_disconnect_by_func(
- // ...
- }
-
// Checks if |ibus_| and |ibus_config_| connections are alive.
bool IBusConnectionsAreAlive() {
return ibus_ && ibus_bus_is_connected(ibus_) && ibus_config_;
@@ -1246,7 +1262,7 @@ class IBusControllerStubImpl : public IBusController {
IBusController* IBusController::Create() {
#if defined(HAVE_IBUS)
- return IBusControllerImpl::GetInstance();
+ return new IBusControllerImpl;
#else
return new IBusControllerStubImpl;
#endif
diff --git a/chrome/browser/chromeos/input_method/input_method_manager.cc b/chrome/browser/chromeos/input_method/input_method_manager.cc
index 0b7c183..f9859be 100644
--- a/chrome/browser/chromeos/input_method/input_method_manager.cc
+++ b/chrome/browser/chromeos/input_method/input_method_manager.cc
@@ -153,7 +153,7 @@ class InputMethodManagerImpl : public HotkeyManager::Observer,
notification_registrar_.Add(this, content::NOTIFICATION_APP_TERMINATING,
NotificationService::AllSources());
- ibus_controller_ = IBusController::Create();
+ ibus_controller_.reset(IBusController::Create());
// The observer should be added before Connect() so we can capture the
// initial connection change.
ibus_controller_->AddObserver(this);
@@ -1182,9 +1182,9 @@ class InputMethodManagerImpl : public HotkeyManager::Observer,
ChangeInputMethod(*iter);
}
- // A reference to the language api, to allow callbacks when the input method
- // status changes.
- IBusController* ibus_controller_;
+ // The IBus controller is used to control the input method status and
+ // allow allow callbacks when the input method status changes.
+ scoped_ptr<IBusController> ibus_controller_;
ObserverList<InputMethodManager::Observer> observers_;
ObserverList<VirtualKeyboardObserver> virtual_keyboard_observers_;
diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt
index 6a8debb..fa7db47 100644
--- a/tools/valgrind/memcheck/suppressions.txt
+++ b/tools/valgrind/memcheck/suppressions.txt
@@ -4155,13 +4155,6 @@
fun:_ZN5views6Widget4InitERKNS0_10InitParamsE
}
{
- bug_86146
- Memcheck:Leak
- fun:_Znw*
- fun:_ZN8chromeos12input_method14IBusController6CreateEv
- fun:_ZN8chromeos22InputMethodLibraryImpl4InitEv
-}
-{
bug_86481
Memcheck:Leak
fun:_Znw*
--
cgit v1.1