Extends DBusThreadManager to connect ibus-bus.

chrome/browser/chromeos/input_method/ibus_controller_impl.cc

 // Copyright (c) 2012 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 "chrome/browser/chromeos/input_method/ibus_controller_impl.h"
 
 #include <algorithm>  // for std::reverse.
 #include <cstdio>
 #include <cstring>  // for std::strcmp.
 #include <set>
 #include <sstream>
 #include <stack>
 #include <utility>
 
+#include "base/environment.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/rand_util.h"
 #include "base/stringprintf.h"
 #include "base/string_split.h"
 #include "chrome/browser/chromeos/input_method/input_method_config.h"
 #include "chrome/browser/chromeos/input_method/input_method_property.h"
 #include "chrome/browser/chromeos/input_method/input_method_util.h"
 
 // TODO(nona): Remove libibus dependency from this file. Then, write unit tests
 // for all functions in this file. crbug.com/26334
 #if defined(HAVE_IBUS)
 #include <ibus.h>
@@ skipping 861 matching lines @@
   if (!prop_list.empty()) {
     for (size_t i = 0; i < prop_list.size(); ++i) {
       FindAndUpdateProperty(prop_list[i], &current_property_list_);
     }
     FOR_EACH_OBSERVER(Observer, observers_, PropertyChanged());
   }
 }
 
 bool IBusControllerImpl::MaybeLaunchIBusDaemon() {
   static const char kIBusDaemonPath[] = "/usr/bin/ibus-daemon";
+  const std::string ibus_address = base::StringPrintf(

[Yusuke Sato, 2012/04/24 04:58:37]

nit: move this down to line 908.

[Seigo Nonaka, 2012/04/26 00:14:32]

Done.

+      "unix:abstract=/tmp/ibus-%0lX", base::RandUint64());
 
   if (process_handle_ != base::kNullProcessHandle) {
     DVLOG(1) << "MaybeLaunchIBusDaemon: ibus-daemon is already running.";
     return false;
   }
   if (!should_launch_daemon_)
     return false;
 
   // TODO(zork): Send output to /var/log/ibus.log
   const std::string ibus_daemon_command_line =
-      base::StringPrintf("%s --panel=disable --cache=none --restart --replace",
-                         kIBusDaemonPath);
+      base::StringPrintf("%s --panel=disable --cache=none --restart --replace"
+                         " --address=%s",
+                         kIBusDaemonPath,
+                         ibus_address.c_str());
   if (!LaunchProcess(ibus_daemon_command_line,
                      &process_handle_,
                      reinterpret_cast<GChildWatchFunc>(OnIBusDaemonExit))) {
     DVLOG(1) << "Failed to launch " << ibus_daemon_command_line;
     return false;
   }
+
+  // To avoid reading address file in UI thread, sets environment variable. The

[Yusuke Sato, 2012/04/24 04:58:37]

You can remove 922-927. See http://codereview.chromium.org/10195001/.

[Seigo Nonaka, 2012/04/26 00:14:32]

Done.

+  // libibus uses IBUS_ADDRESS  environment variable at first if available.
+  // https://github.com/ibus/ibus/blob/master/src/ibusshare.c#L188
+  // TODO(nona): Remove environment variable after fixing crosbug.com/26334.
+  scoped_ptr<base::Environment> env(base::Environment::Create());
+  env->SetVar("IBUS_ADDRESS", ibus_address.c_str());
+
+  DBusThreadManager* dbus_thread_manager = DBusThreadManager::Get();
+  dbus_thread_manager->InitializeIBusBus(ibus_address);

[Yusuke Sato, 2012/04/24 04:58:37]

It seems that you code does not support dynamic ibus-daemon shutdown. What
happens with the following scenario?

1. mozc is enabled.
2. MaybeLaunchIBusDaemon()  is called. ibus-daemon is started with --address=X
3. InitializeIBusBus is called with X
4. mozc is disabled.
5. ibus-daemon is killed.
6. mozc is re-enabled.
7. MaybeLaunchIBusDaemon() is called again. ibus-daemon is started with
--address=Y
8. InitializeIBusBus is called with Y, but it fails since ibus_bus_ is already
initialized.

Please fix the code (or remove the dynamic shutdown code if it's super hard to
fix your new client).

[Seigo Nonaka, 2012/04/26 00:14:32]

I think latest patch support suggested case, however I can not reproduce 5.
The ibus-daemon process is alived even I turned off Mozc from input language
configuration.
I tried as follows:

1. Choose English UI on the initial start-up
2. Enable mozc(Click the mozc checkbox on chrome://settings/language)  
3. Turn ON mozc by Alt_L + Shift_L
4. Input something
(I confirmed ibus-daemon is alive here)
5. Disable Mozc (Remove check from chrome://settings/languages)
(I confirmed ibus-daemon is still alive here)
6. Input something (in english)
7. ibus-daemon is still alived.

There are only Japanese and English are shown in chrome://settings/languages.
And the ibus-daemon is not launched after reboot after 7.

Is it bug? or I do something wrong?

On 2012/04/24 04:58:37, Yusuke Sato wrote:

[Yusuke Sato, 2012/04/26 00:46:28]

Ah ok, the dynamic shutdown feature is temporarily disabled to work around
crosbug 26443. See InputMethodManagerImpl::EnableInputMethods() and
InputMethodManagerImpl::RemoveInputMethodExtension().

  if (ContainOnlyKeyboardLayout(active_input_method_ids_)) {
    // Do NOT call ibus_controller_->Stop(); here to work around a crash issue  

    // at crosbug.com/27051.                                                    

    // TODO(yusukes): We can safely call Stop(); here once crosbug.com/26443    

    // is implemented.      
  ...

The plan is to re-enable the feature once your work is done.

On 2012/04/26 00:14:32, nona1 wrote:

[Yusuke Sato, 2012/04/26 00:48:48]

s/crosbug 26443/crosbug 27051/

+
   return true;
 }
 
 bool IBusControllerImpl::LaunchProcess(const std::string& command_line,
                                        base::ProcessHandle* process_handle,
                                        GChildWatchFunc watch_func) {
   std::vector<std::string> argv;
   base::ProcessHandle handle = base::kNullProcessHandle;
 
   base::SplitString(command_line, ' ', &argv);
@@ skipping 53 matching lines @@
 
 // static
 bool IBusControllerImpl::FindAndUpdatePropertyForTesting(
     const chromeos::input_method::InputMethodProperty& new_prop,
     chromeos::input_method::InputMethodPropertyList* prop_list) {
   return FindAndUpdateProperty(new_prop, prop_list);
 }
 
 }  // namespace input_method
 }  // namespace chromeos