Verify global engine after changing preload_engines

Verify global engine after changing preload_engines


BUG=http://crosbug.com/13406
TEST=Linux desktop

[Yusuke Sato, 2011-03-25 14:43:15]

Thanks for fixing this quickly. LGTM

http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c#newcode164
bus/ibusimpl.c:164: static void     bus_ibus_impl_global_engine_check
I think bus_ibus_impl_check_global_engine() might be more consistent naming, but
it's up to you.

http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c#newcode841
bus/ibusimpl.c:841: /* focus the fake_context, if use_global_engine is enabled.
*/
Can you briefly let me know the reason why you changed this part?
It's not so clear to me.

[Peng Huang, 2011-03-25 15:07:01]

http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c
File bus/ibusimpl.c (left):

http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c#oldcode838
bus/ibusimpl.c:838: bus_ibus_impl_reload_config (ibus);
ibus->config is NULL here. So call bus_ibus_impl_reload_config() will do noting

http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c#newcode164
bus/ibusimpl.c:164: static void     bus_ibus_impl_global_engine_check
On 2011/03/25 14:43:15, Yusuke Sato wrote:
> I think bus_ibus_impl_check_global_engine() might be more consistent naming,
but
> it's up to you.

Done.

http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c#newcode841
bus/ibusimpl.c:841: /* focus the fake_context, if use_global_engine is enabled.
*/
On 2011/03/25 14:43:15, Yusuke Sato wrote:
> Can you briefly let me know the reason why you changed this part?
> It's not so clear to me.
>
It fixes a bug that found in testing this CL.

If the use_global_engine is TRUE, and no real context has focus, the
ibus->focused_context should be ibus->fake_context. You could check code in
bus_ibus_impl_set_focused_context (). But at the begin of ibus-daemon starting
up, ibus->focused_context is NULL, when a real context get focus at some time,
ibus will in some wrong state. So I need set it to fake_context, if
use_gloable_engine is TRUE (it is true in our cros branch).

[Yusuke Sato, 2011-03-25 15:12:03]

thanks, lgtm.

On 2011/03/25 15:07:01, Peng Huang wrote:
> http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c
> File bus/ibusimpl.c (left):
> 
> http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c#oldcode838
> bus/ibusimpl.c:838: bus_ibus_impl_reload_config (ibus);
> ibus->config is NULL here. So call bus_ibus_impl_reload_config() will do
noting
> 
> http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c
> File bus/ibusimpl.c (right):
> 
> http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c#newcode164
> bus/ibusimpl.c:164: static void     bus_ibus_impl_global_engine_check
> On 2011/03/25 14:43:15, Yusuke Sato wrote:
> > I think bus_ibus_impl_check_global_engine() might be more consistent naming,
> but
> > it's up to you.
> 
> Done.
> 
> http://codereview.appspot.com/4273111/diff/2001/bus/ibusimpl.c#newcode841
> bus/ibusimpl.c:841: /* focus the fake_context, if use_global_engine is
enabled.
> */
> On 2011/03/25 14:43:15, Yusuke Sato wrote:
> > Can you briefly let me know the reason why you changed this part?
> > It's not so clear to me.
> >
> It fixes a bug that found in testing this CL.
> 
> If the use_global_engine is TRUE, and no real context has focus, the
> ibus->focused_context should be ibus->fake_context. You could check code in
> bus_ibus_impl_set_focused_context (). But at the begin of ibus-daemon starting
> up, ibus->focused_context is NULL, when a real context get focus at some time,
> ibus will in some wrong state. So I need set it to fake_context, if
> use_gloable_engine is TRUE (it is true in our cros branch).