Launch the second ibus engine too to reduce the launching time.

Launch the second ibus engine too to reduce the launching time.

Currently the first ibus engine only is running but the second engine
is also frequently used. It is good to run the second engine before
user types Ctrl+space.

BUG=RH#838734
TEST=src/tests/ibus-bus

[Peng, 2013-03-04 15:35:17]

https://codereview.appspot.com/7433049/diff/1/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/7433049/diff/1/bus/ibusimpl.c#newcode179
bus/ibusimpl.c:179: "    <method name='StartComponentByEngineName'>\n"
How about rename it to PreloadEngines(string[]names)?
In case, maybe we want to preload more engines in future.

https://codereview.appspot.com/7433049/diff/1/bus/ibusimpl.c#newcode1219
bus/ibusimpl.c:1219: * Implement the "StartComponentByEngineName" method call of
the org.freedesktop.IBus interface.
> 80 chars

https://codereview.appspot.com/7433049/diff/1/bus/ibusimpl.c#newcode1254
bus/ibusimpl.c:1254: g_dbus_method_invocation_get_connection (invocation));
Don't understand why get connection from invocation. I think this connection
from invocation is between ibus-daemon and ibus-panel.

https://codereview.appspot.com/7433049/diff/1/bus/ibusimpl.c#newcode1255
bus/ibusimpl.c:1255: factory = bus_factory_proxy_new (connection);
I don't think the factory will be ready just after bus_component_start().

https://codereview.appspot.com/7433049/diff/1/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/7433049/diff/1/src/ibusbus.c#newcode2024
src/ibusbus.c:2024: NULL);
I think it is better to catch the error and print it in log.
But seems other sync functions do not do this. Maybe it is better to improve all
of them.

https://codereview.appspot.com/7433049/diff/1/ui/gtk3/panel.vala
File ui/gtk3/panel.vala (right):

https://codereview.appspot.com/7433049/diff/1/ui/gtk3/panel.vala#newcode486
ui/gtk3/panel.vala:486:
m_bus.start_component_by_engine_name(engines[0].get_name());
> 80 chars?

I think it is better to use async version there.

[fujiwara, 2013-03-05 04:37:18]

https://codereview.appspot.com/7433049/diff/1/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/7433049/diff/1/bus/ibusimpl.c#newcode179
bus/ibusimpl.c:179: "    <method name='StartComponentByEngineName'>\n"
On 2013/03/04 15:35:17, Peng wrote:
> How about rename it to PreloadEngines(string[]names)?
> In case, maybe we want to preload more engines in future.

Renamed.

https://codereview.appspot.com/7433049/diff/1/bus/ibusimpl.c#newcode1254
bus/ibusimpl.c:1254: g_dbus_method_invocation_get_connection (invocation));
On 2013/03/04 15:35:17, Peng wrote:
> Don't understand why get connection from invocation. I think this connection
> from invocation is between ibus-daemon and ibus-panel.

Fixed my mistake.

https://codereview.appspot.com/7433049/diff/1/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/7433049/diff/1/src/ibusbus.c#newcode2024
src/ibusbus.c:2024: NULL);
On 2013/03/04 15:35:17, Peng wrote:
> I think it is better to catch the error and print it in log.
> But seems other sync functions do not do this. Maybe it is better to improve
all
> of them.

Probably I don't understand the suggestion. I think ibus_bus_call_sync() print
errors.

https://codereview.appspot.com/7433049/diff/1/ui/gtk3/panel.vala
File ui/gtk3/panel.vala (right):

https://codereview.appspot.com/7433049/diff/1/ui/gtk3/panel.vala#newcode486
ui/gtk3/panel.vala:486:
m_bus.start_component_by_engine_name(engines[0].get_name());
On 2013/03/04 15:35:17, Peng wrote:
> > 80 chars?
> 
> I think it is better to use async version there.

OK, async version is implemented.

[Peng, 2013-03-05 15:13:02]

https://codereview.appspot.com/7433049/diff/6001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/7433049/diff/6001/bus/ibusimpl.c#newcode1254
bus/ibusimpl.c:1254: bus_component_start (component, g_verbose);
I think it is very likely several engines are in same component. It is better to
go through all engines first to get components and remove duplicated components,
and then start those components one by one.

[fujiwara, 2013-03-06 09:12:31]

https://codereview.appspot.com/7433049/diff/6001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/7433049/diff/6001/bus/ibusimpl.c#newcode1254
bus/ibusimpl.c:1254: bus_component_start (component, g_verbose);
On 2013/03/05 15:13:03, Peng wrote:
> I think it is very likely several engines are in same component. It is better
to
> go through all engines first to get components and remove duplicated
components,
> and then start those components one by one.
> 

OK, checked component names.

[Peng, 2013-03-06 15:14:29]

https://codereview.appspot.com/7433049/diff/11001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/7433049/diff/11001/bus/ibusimpl.c#newcode1232
bus/ibusimpl.c:1232: GSList *list = NULL, *l;
May be use GArray is better. It is easy to access.

https://codereview.appspot.com/7433049/diff/11001/bus/ibusimpl.c#newcode1262
bus/ibusimpl.c:1262: if (g_strcmp0 (bus_component_get_name ((BusComponent *)
l->data),
I think bus_component_from_engine_desc() always returns same pointer of
component. So maybe compare the pointer is enough.
Please double check the implementation of bus_component_from_engine_desc()

https://codereview.appspot.com/7433049/diff/11001/bus/ibusimpl.c#newcode1264
bus/ibusimpl.c:1264: has_component = TRUE;
Why not just append the component and break? So has_component is not necessary.

[fujiwara, 2013-03-07 04:06:53]

https://codereview.appspot.com/7433049/diff/11001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/7433049/diff/11001/bus/ibusimpl.c#newcode1232
bus/ibusimpl.c:1232: GSList *list = NULL, *l;
On 2013/03/06 15:14:29, Peng wrote:
> May be use GArray is better. It is easy to access.

OK, changed it to GPtrArray.

https://codereview.appspot.com/7433049/diff/11001/bus/ibusimpl.c#newcode1264
bus/ibusimpl.c:1264: has_component = TRUE;
On 2013/03/06 15:14:29, Peng wrote:
> Why not just append the component and break? So has_component is not
necessary.

Because has_component == FALSE is needed but not TRUE.

[Peng, 2013-03-07 15:28:25]

lgtm thanks.