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

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*