Reduce CPU usage for input method switching.

Reduce CPU usage for input method switching.

- Move GetNumActiveInputMethods call from input_method_menu_button.cc to input_method_library.cc so that the IBus function is called only once even when multiple Chrome windows are available.
- Improved InputMethodMenu::InputMethodChanged so that only the first input method button would update Preferences.
- Remove ImePropertiesChanged callback. It's obsolete.

BUG=chromium-os:8553
TEST=manually.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67973

[Yusuke Sato, 2010-11-30 05:43:34]

Satoru, can you review this?

[satorux1, 2010-12-01 06:40:18]

sorry for the belated response.

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/input_method_library.cc (right):

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/input_method_library.cc:25: #define
NOTIFY_FIRST_OBSERVER(ObserverType, observer_list, func)  \
The macro seems to be only used once. Probably not necessary to define this.

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/input_method_library.cc:409:
NOTIFY_FIRST_OBSERVER(Observer, observers_,
Please add a comment about why you notify this to the first observer only.

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/input_method_menu.h (right):

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/input_method_menu.h:116: virtual void
UpdateUI(const std::string& id,  // e.g. "xkb:us::eng", "mozc"
id is not descriptive. id -> input_method_id ?

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/input_method_menu_button.cc (right):

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/input_method_menu_button.cc:89: // In general, we
should not call an ibus-daemon API in the input method
ibus-daemon -> input method ?

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/input_method_menu_button.h (right):

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/input_method_menu_button.h:32: virtual void
UpdateUI(const std::string& id,
id -> input_method_id?

[Yusuke Sato, 2010-12-01 07:36:41]

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/input_method_library.cc (right):

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/input_method_library.cc:25: #define
NOTIFY_FIRST_OBSERVER(ObserverType, observer_list, func)  \
On 2010/12/01 06:40:18, satorux1 wrote:
> The macro seems to be only used once. Probably not necessary to define this.

Done.

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/input_method_library.cc:409:
NOTIFY_FIRST_OBSERVER(Observer, observers_,
On 2010/12/01 06:40:18, satorux1 wrote:
> Please add a comment about why you notify this to the first observer only.

Done.

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/input_method_menu.h (right):

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/input_method_menu.h:116: virtual void
UpdateUI(const std::string& id,  // e.g. "xkb:us::eng", "mozc"
On 2010/12/01 06:40:18, satorux1 wrote:
> id is not descriptive. id -> input_method_id ?

Done.

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/input_method_menu_button.cc (right):

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/input_method_menu_button.cc:89: // In general, we
should not call an ibus-daemon API in the input method
On 2010/12/01 06:40:18, satorux1 wrote:
> ibus-daemon -> input method ?

Done.

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/input_method_menu_button.h (right):

http://codereview.chromium.org/4162002/diff/10001/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/input_method_menu_button.h:32: virtual void
UpdateUI(const std::string& id,
On 2010/12/01 06:40:18, satorux1 wrote:
> id -> input_method_id?

Done.

[satorux1, 2010-12-01 07:56:47]

LGTM

http://codereview.chromium.org/4162002/diff/19001/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/input_method_library.cc (right):

http://codereview.chromium.org/4162002/diff/19001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/input_method_library.cc:402: // observers to do so
for performance reasons.
You could elaborate a bit more:

Ask the first observer to update preferences. We should not ask every observer
to do so. Otherwise, we'll end up updating preferences many times, when many
observers are attached (ex. many windows are opened), which is unnecessary and
expensive.

http://codereview.chromium.org/4162002/diff/19001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/input_method_library.cc:404: Observer*
first_observer;
Uninitialized pointers annoy me. :) The following would be a bit nicer.

Observer* first_observer = it.GetNext();
if (first_observer != NULL) {
  ...

[Yusuke Sato, 2010-12-01 09:21:25]

http://codereview.chromium.org/4162002/diff/19001/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/input_method_library.cc (right):

http://codereview.chromium.org/4162002/diff/19001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/input_method_library.cc:402: // observers to do so
for performance reasons.
On 2010/12/01 07:56:47, satorux1 wrote:
> You could elaborate a bit more:
> 
> Ask the first observer to update preferences. We should not ask every observer
> to do so. Otherwise, we'll end up updating preferences many times, when many
> observers are attached (ex. many windows are opened), which is unnecessary and
> expensive.

Done.

http://codereview.chromium.org/4162002/diff/19001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/input_method_library.cc:404: Observer*
first_observer;
On 2010/12/01 07:56:47, satorux1 wrote:
> Uninitialized pointers annoy me. :) The following would be a bit nicer.
> 
> Observer* first_observer = it.GetNext();
> if (first_observer != NULL) {
>   ...

Done.