Destroy existing connection before creating a new connection, and only create the fake context once.

Destroy existing connection before creating a new connection, and only create the fake context once.


BUG=chromium-os:7998
TEST=manual

[Yusuke Sato, 2010-10-21 06:15:47]

basically looks good to me, but i have 2 questions:

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

http://codereview.appspot.com/2640041/diff/1/client/gtk2/ibusimcontext.c#newc...
client/gtk2/ibusimcontext.c:357: /* init the global fake context */
You mean ibus_im_context_class_init() could be called twice or more, right? 
(Suppose it's true) I'm still wondering when it happens...

http://codereview.appspot.com/2640041/diff/1/src/ibusbus.c
File src/ibusbus.c (right):

http://codereview.appspot.com/2640041/diff/1/src/ibusbus.c#newcode195
src/ibusbus.c:195: /* destry old connection at first */
I think you fixed a memory leak which could occur when ibus_bus_connect() is
called for the same |bus| object, right? And this fix is not directly related to
the Issue 7998.

Is my understanding correct?

[Peng Huang, 2010-10-21 07:42:10]

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

http://codereview.appspot.com/2640041/diff/1/client/gtk2/ibusimcontext.c#newc...
client/gtk2/ibusimcontext.c:357: /* init the global fake context */
On 2010/10/21 06:15:48, Yusuke Sato wrote:
> You mean ibus_im_context_class_init() could be called twice or more, right? 
> (Suppose it's true) I'm still wondering when it happens...

I found chromeos will call class_init twice. But I don't know why. I will
investigate it later.

http://codereview.appspot.com/2640041/diff/1/client/gtk2/ibusimcontext.c#newc...
client/gtk2/ibusimcontext.c:357: /* init the global fake context */
On 2010/10/21 06:15:48, Yusuke Sato wrote:
> You mean ibus_im_context_class_init() could be called twice or more, right? 
> (Suppose it's true) I'm still wondering when it happens...

Done.

http://codereview.appspot.com/2640041/diff/1/src/ibusbus.c
File src/ibusbus.c (right):

http://codereview.appspot.com/2640041/diff/1/src/ibusbus.c#newcode195
src/ibusbus.c:195: /* destry old connection at first */
On 2010/10/21 06:15:48, Yusuke Sato wrote:
> I think you fixed a memory leak which could occur when ibus_bus_connect() is
> called for the same |bus| object, right? And this fix is not directly related
to
> the Issue 7998.
> 
> Is my understanding correct?

It can make sure the old connection will be released before creating a new
connection. But may be it is not necessary. Anyway it is no harm.

[Yusuke Sato, 2010-10-21 08:12:22]

LGTM, thanks for working on it.

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

http://codereview.appspot.com/2640041/diff/1/client/gtk2/ibusimcontext.c#newc...
client/gtk2/ibusimcontext.c:357: /* init the global fake context */
On 2010/10/21 07:42:10, Peng Huang wrote:
> On 2010/10/21 06:15:48, Yusuke Sato wrote:
> > You mean ibus_im_context_class_init() could be called twice or more, right? 
> > (Suppose it's true) I'm still wondering when it happens...
> 
> I found chromeos will call class_init twice. But I don't know why. I will
> investigate it later.
> 

Please file an issue on http://crosbug.com/ just in case.