Support IME properties in uber tray.

Support IME properties in uber tray.


BUG=118862, 119672
TEST=manually

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=128487

[Yusuke Sato, 2012-03-23 08:54:22]

Thanks for working on this. Please add BUG=119672 as well.

http://codereview.chromium.org/9835045/diff/1/ash/system/ime/tray_ime.cc
File ash/system/ime/tray_ime.cc (right):

http://codereview.chromium.org/9835045/diff/1/ash/system/ime/tray_ime.cc#newc...
ash/system/ime/tray_ime.cc:94: if (status_ != user::LOGGED_IN_NONE)
The setting menu should be shown neither on the login nor lock screen. Can this
CL handle the lock screen correctly?

[Jun Mukai, 2012-03-23 09:05:42]

http://codereview.chromium.org/9835045/diff/1/ash/system/ime/tray_ime.cc
File ash/system/ime/tray_ime.cc (right):

http://codereview.chromium.org/9835045/diff/1/ash/system/ime/tray_ime.cc#newc...
ash/system/ime/tray_ime.cc:94: if (status_ != user::LOGGED_IN_NONE)
On 2012/03/23 08:54:22, Yusuke Sato wrote:
> The setting menu should be shown neither on the login nor lock screen. Can
this
> CL handle the lock screen correctly?
> 

Ugh, no.  It appears.  However, I found that Uber tray does not have special
state for lock screens, and tray_network also has the 'settings' item there.
Also, this is safe if the item is shown in lock screen.
Clicking the IME settings in the lock screen does not make any effects -- it
opens the settings page after you unlock the screen in the normal way.

Thus I want to go ahead with this code, and file another bug for lock screen
status.

[Yusuke Sato, 2012-03-23 09:51:58]

lgtm assuming that you'll file another bug for the lock screen.

On 2012/03/23 09:05:42, Jun Mukai wrote:
> http://codereview.chromium.org/9835045/diff/1/ash/system/ime/tray_ime.cc
> File ash/system/ime/tray_ime.cc (right):
> 
>
http://codereview.chromium.org/9835045/diff/1/ash/system/ime/tray_ime.cc#newc...
> ash/system/ime/tray_ime.cc:94: if (status_ != user::LOGGED_IN_NONE)
> On 2012/03/23 08:54:22, Yusuke Sato wrote:
> > The setting menu should be shown neither on the login nor lock screen. Can
> this
> > CL handle the lock screen correctly?
> > 
> 
> Ugh, no.  It appears.  However, I found that Uber tray does not have special
> state for lock screens, and tray_network also has the 'settings' item there.
> Also, this is safe if the item is shown in lock screen.
> Clicking the IME settings in the lock screen does not make any effects -- it
> opens the settings page after you unlock the screen in the normal way.
> 
> Thus I want to go ahead with this code, and file another bug for lock screen
> status.

[sadrul, 2012-03-23 12:22:48]

Thanks for fixing! LGTM

(I can take care of the lock-screen issue with 119503)

http://codereview.chromium.org/9835045/diff/4001/ash/system/ime/tray_ime.cc
File ash/system/ime/tray_ime.cc (right):

http://codereview.chromium.org/9835045/diff/4001/ash/system/ime/tray_ime.cc#n...
ash/system/ime/tray_ime.cc:68: explicit IMEDetailedView(SystemTrayItem* owner,
user::LoginStatus status)
Remove explicit

[Ben Goodger (Google), 2012-03-23 14:57:34]

lgtm

[Jun Mukai, 2012-03-23 14:57:47]

Yusuke, there is already a bug (crbug.com/119503) and pkotwicz is working on it
(thanks for teaching this sadrul)

http://codereview.chromium.org/9835045/diff/4001/ash/system/ime/tray_ime.cc
File ash/system/ime/tray_ime.cc (right):

http://codereview.chromium.org/9835045/diff/4001/ash/system/ime/tray_ime.cc#n...
ash/system/ime/tray_ime.cc:68: explicit IMEDetailedView(SystemTrayItem* owner,
user::LoginStatus status)
On 2012/03/23 12:22:49, sadrul wrote:
> Remove explicit

Done.