Do not block UI in IBusIMContext anymore.

Do not block UI in IBusIMContext anymore.

Replace some block IPC calls with async IPC calls, and then IBusIMContext will
not block UI anymore.

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

[Yusuke Sato, 2011-03-17 12:29:05]

Thanks for working on this!!

http://codereview.appspot.com/4287054/diff/6001/client/gtk2/ibusimcontext.c
File client/gtk2/ibusimcontext.c (right):

http://codereview.appspot.com/4287054/diff/6001/client/gtk2/ibusimcontext.c#n...
client/gtk2/ibusimcontext.c:1360: /* Cancel previouse create input context
request */
typo: previous

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.c
File src/ibusbus.c (right):

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.c#newcode599
src/ibusbus.c:599: g_dbus_connection_call (bus->priv->connection,
Please add a short comment that explains why we can't use ibus_bus_call_async
here.

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.h
File src/ibusbus.h (right):

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.h#newcode461
src/ibusbus.h:461: * ibus_bus_reate_input_context_async:
typo: create

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.h#newcode472
src/ibusbus.h:472: void        ibus_bus_create_input_context_async
could you add the following tests to src/tests/ibus-bus.c?

1. call this function, then cancel the async operation.
2. call this function twice (or more) quickly.
3. do both 1 and 2.
..

Cancellation is hard.

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.c
File src/ibusinputcontext.c (right):

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.c#newcod...
src/ibusinputcontext.c:626: initable = g_initable_new (IBUS_TYPE_INPUT_CONTEXT,
I didn't know that g_initable_new is a synchronous call.
Maybe we should add async version of ibus_config_new to ibusconfig.c as well?

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h
File src/ibusinputcontext.h (right):

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h#newcode98
src/ibusinputcontext.h:98: * New an IBusInputContext.
might be better to explicitly mention that this function is synchronous.

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h#newcod...
src/ibusinputcontext.h:111: *      or %NULL if you don't care about the result
of the method invocation.
Is it okay to pass a NULL callback to the function?

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h#newcod...
src/ibusinputcontext.h:154: *      or %NULL if you don't care about the result
of the method invocation.
ditto. passing NULL to the function does not make sense?

[Peng, 2011-03-17 14:10:10]

Updated the CL. Please check it again.

http://codereview.appspot.com/4287054/diff/6001/client/gtk2/ibusimcontext.c
File client/gtk2/ibusimcontext.c (right):

http://codereview.appspot.com/4287054/diff/6001/client/gtk2/ibusimcontext.c#n...
client/gtk2/ibusimcontext.c:1360: /* Cancel previouse create input context
request */
On 2011/03/17 12:29:05, Yusuke Sato wrote:
> typo: previous

Done.

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.c
File src/ibusbus.c (right):

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.c#newcode599
src/ibusbus.c:599: g_dbus_connection_call (bus->priv->connection,
On 2011/03/17 12:29:05, Yusuke Sato wrote:
> Please add a short comment that explains why we can't use ibus_bus_call_async
> here.

Done.

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.h
File src/ibusbus.h (right):

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.h#newcode461
src/ibusbus.h:461: * ibus_bus_reate_input_context_async:
On 2011/03/17 12:29:05, Yusuke Sato wrote:
> typo: create

Done.

http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.h#newcode472
src/ibusbus.h:472: void        ibus_bus_create_input_context_async
On 2011/03/17 12:29:05, Yusuke Sato wrote:
> could you add the following tests to src/tests/ibus-bus.c?
> 
> 1. call this function, then cancel the async operation.
> 2. call this function twice (or more) quickly.
> 3. do both 1 and 2.
> ..
> 
> Cancellation is hard.

Done.

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.c
File src/ibusinputcontext.c (right):

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.c#newcod...
src/ibusinputcontext.c:626: initable = g_initable_new (IBUS_TYPE_INPUT_CONTEXT,
On 2011/03/17 12:29:05, Yusuke Sato wrote:
> I didn't know that g_initable_new is a synchronous call.
> Maybe we should add async version of ibus_config_new to ibusconfig.c as well?

I noticed this. I will try to fix it in another CL

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h
File src/ibusinputcontext.h (right):

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h#newcod...
src/ibusinputcontext.h:111: *      or %NULL if you don't care about the result
of the method invocation.
On 2011/03/17 12:29:05, Yusuke Sato wrote:
> Is it okay to pass a NULL callback to the function?

Done.

http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h#newcod...
src/ibusinputcontext.h:154: *      or %NULL if you don't care about the result
of the method invocation.
On 2011/03/17 12:29:05, Yusuke Sato wrote:
> ditto. passing NULL to the function does not make sense?

Done.

[Yusuke Sato, 2011-03-17 14:48:18]

LGTM, thanks!

On 2011/03/17 14:10:10, Peng wrote:
> Updated the CL. Please check it again.
> 
> http://codereview.appspot.com/4287054/diff/6001/client/gtk2/ibusimcontext.c
> File client/gtk2/ibusimcontext.c (right):
> 
>
http://codereview.appspot.com/4287054/diff/6001/client/gtk2/ibusimcontext.c#n...
> client/gtk2/ibusimcontext.c:1360: /* Cancel previouse create input context
> request */
> On 2011/03/17 12:29:05, Yusuke Sato wrote:
> > typo: previous
> 
> Done.
> 
> http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.c
> File src/ibusbus.c (right):
> 
> http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.c#newcode599
> src/ibusbus.c:599: g_dbus_connection_call (bus->priv->connection,
> On 2011/03/17 12:29:05, Yusuke Sato wrote:
> > Please add a short comment that explains why we can't use
ibus_bus_call_async
> > here.
> 
> Done.
> 
> http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.h
> File src/ibusbus.h (right):
> 
> http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.h#newcode461
> src/ibusbus.h:461: * ibus_bus_reate_input_context_async:
> On 2011/03/17 12:29:05, Yusuke Sato wrote:
> > typo: create
> 
> Done.
> 
> http://codereview.appspot.com/4287054/diff/6001/src/ibusbus.h#newcode472
> src/ibusbus.h:472: void        ibus_bus_create_input_context_async
> On 2011/03/17 12:29:05, Yusuke Sato wrote:
> > could you add the following tests to src/tests/ibus-bus.c?
> > 
> > 1. call this function, then cancel the async operation.
> > 2. call this function twice (or more) quickly.
> > 3. do both 1 and 2.
> > ..
> > 
> > Cancellation is hard.
> 
> Done.
> 
> http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.c
> File src/ibusinputcontext.c (right):
> 
>
http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.c#newcod...
> src/ibusinputcontext.c:626: initable = g_initable_new
(IBUS_TYPE_INPUT_CONTEXT,
> On 2011/03/17 12:29:05, Yusuke Sato wrote:
> > I didn't know that g_initable_new is a synchronous call.
> > Maybe we should add async version of ibus_config_new to ibusconfig.c as
well?
> 
> I noticed this. I will try to fix it in another CL
> 
> http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h
> File src/ibusinputcontext.h (right):
> 
>
http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h#newcod...
> src/ibusinputcontext.h:111: *      or %NULL if you don't care about the result
> of the method invocation.
> On 2011/03/17 12:29:05, Yusuke Sato wrote:
> > Is it okay to pass a NULL callback to the function?
> 
> Done.
> 
>
http://codereview.appspot.com/4287054/diff/6001/src/ibusinputcontext.h#newcod...
> src/ibusinputcontext.h:154: *      or %NULL if you don't care about the result
> of the method invocation.
> On 2011/03/17 12:29:05, Yusuke Sato wrote:
> > ditto. passing NULL to the function does not make sense?
> 
> Done.