Issue 6216002: cleanup: Use the new libcros API StopInputMethodProcess() to terminate ibus-daemon instead of kill()

Issue 6216002: cleanup: Use the new libcros API StopInputMethodProcess() to terminate ibus-daemon instead of kill() (Closed)

Created:
9 years, 11 months ago by Yusuke Sato

Modified:
9 years, 7 months ago

Reviewers:
satorux1

CC:
chromium-reviews, davemoore+watch_chromium.org, pam+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

cleanup: Use the new libcros API StopInputMethodProcess() to terminate ibus-daemon instead of kill() Using the API should be better then kill() because o ibus-daemon could be terminated even if the daemon ignores or temporalily masks SIGTERM for some reason. o libcros could know the connection between the library and the daemon will be terminated shortly, and could avoid using the connection after the API call. This CL depends on http://codereview.chromium.org/6101006/. BUG=chromium-os:9685 TEST=manually Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71026

Patch Set 1 #

Patch Set 2 : review #

Total comments: 4

Patch Set 3 : review fixes #

Patch Set 4 : added cros.DEPS #

Patch Set 5 : style fix #

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+10 lines, -2 lines)			Patch
	M	chrome/browser/chromeos/cros/input_method_library.cc	View	1 2 3 4	1 chunk	+9 lines, -1 line	0 comments	Download
	M	tools/cros.DEPS/DEPS	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

satorux1

LGTM, assuming you'll replace LOG(INFO) with VLOG(1) http://codereview.chromium.org/6216002/diff/2001/chrome/browser/chromeos/cros/input_method_library.cc File chrome/browser/chromeos/cros/input_method_library.cc (right): http://codereview.chromium.org/6216002/diff/2001/chrome/browser/chromeos/cros/input_method_library.cc#newcode586 chrome/browser/chromeos/cros/input_method_library.cc:586: LOG(INFO) << ...

9 years, 11 months ago (2011-01-11 07:02:29 UTC) #2

Yusuke Sato

9 years, 11 months ago (2011-01-11 07:51:01 UTC) #3

http://codereview.chromium.org/6216002/diff/2001/chrome/browser/chromeos/cros...
File chrome/browser/chromeos/cros/input_method_library.cc (right):

http://codereview.chromium.org/6216002/diff/2001/chrome/browser/chromeos/cros...
chrome/browser/chromeos/cros/input_method_library.cc:586: LOG(INFO) <<
"ibus-daemon (PID=" << ibus_daemon_process_id_ << ") is "
On 2011/01/11 07:02:30, satorux1 wrote:
> Use of LOG(INFO) is discouraged. VLOG(1) would be nicer.

Done.

http://codereview.chromium.org/6216002/diff/2001/chrome/browser/chromeos/cros...
chrome/browser/chromeos/cros/input_method_library.cc:592: LOG(INFO) <<
"candidate_window (PID=" << candidate_window_process_id_
On 2011/01/11 07:02:30, satorux1 wrote:
> ditto

Done.

Expand Messages | Collapse Messages