249903 - nsGNOMERegistry::HandlerExists uses gconf client after g_object_unref()ing it

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Christian Persch (GNOME) (away; not receiving bug mail)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0
Build Identifier: 

nsGNOMERegistry::HandlerExists uses the default GConf client after unreffing it.

Reproducible: Always
Steps to Reproduce:

Comment 1

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: proposed fix

Defer unreffing the GConf client until after we're done with it.

Comment 2

[Brian Ryner (not reading)]

Comment on attachment 152359 [details] [diff] [review]
proposed fix

I'd prefer if you just move isEnabled outside of the |if (app)|, initializing
it to false, then have a single unref and return at the end of the function.
r=bryner with that change.

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

confirming bug.

Comment 4

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: updated patch as requested by the reviewer

Comment 5

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 152362 [details] [diff] [review]
updated patch as requested by the reviewer

Carrying forward bryner's r+, and asking for sr

Comment 6

[Darin Fisher]

Comment on attachment 152362 [details] [diff] [review]
updated patch as requested by the reviewer

>Index: nsGNOMERegistry.cpp

>+  return isEnabled ? PR_TRUE : PR_FALSE;

    return isEnabled != FALSE;


sr=darin with that change.

Comment 7

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: updated patch as per superreviewer's request

Comment 8

[Christian Persch (GNOME) (away; not receiving bug mail)]

This is fixed in trunk by the checkin from bug 258802.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

marking so