49148 - MLK: nsWindow::SetTitle() leaks something out of tmpProp

Status: VERIFIED WORKSFORME

(Core :: XUL, defect, P3)

Description

[Bruce Mitchener]

Under Purify, I'm seeing leaks arise from what appears to be every call to 
nsWindow::SetTitle() from the tmpProp.  I only marked it as Solaris because I am 
not sure that it isn't a Solaris bug.

Comment 1

[Stuart Parmenter]

i think dbaron had a fix for this and was going to check it in

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Marking All/Linux to mean All Unix/All.

Yup, I have a fix and I would've checked it in this morning (along with other XP
stuff) if mac hadn't been red...

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Fix checked in.

Comment 4

[Bruce Mitchener]

Happening again!!  I'm seeing this on a build from today.

Comment 5

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Reassigning to [e-mail], who reversed the condition on the if without
moving the freeing back into the else where it was before I fixed this.

Comment 6

[Hidetoshi Tajima]

The codes to free tmpProp should have been located out of the if.
I'll check in the code as soon as the tree gets open this afternoon
with the fix for bugzilla 41786.

Comment 7

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Doesn't moving it out of the if mean you're trying to free an uninitialized
variable if the call fails?

Comment 8

[Hidetoshi Tajima]

Thanks. tmpProp.value should be initlialized to 0 before calling
XGetWMName.

Comment 9

[Hidetoshi Tajima]

Attachment: remove TWM sucks stuff - no longer needed for XStdICCTextStyle was used

Comment 10

[Hidetoshi Tajima]

pavlov, dbaron, waterson, please review the attached patch, which is conbinded
fix
for this bug, 43108, and 41786. As it uses XStdICCTextStyle in the 1st
XmbTextListToTextProperty call, the fallback to try STRING encoding for TWM is
no
longer necessary, as the 1st call takes care of the case.

Comment 11

[Stuart Parmenter]

this patch looks fine.  I have tested it in both sawfish and twm

Comment 12

[Decklin Foster]

the patch also works here; CCing myself.

Comment 13

[Jaime Rodriguez, Jr.]

Marking nsbeta3+ per I18N Bug Triage.

Comment 14

[Hidetoshi Tajima]

Checked in the fix today.

Comment 15

[Hidetoshi Tajima]

Oops, forgot about my previous update. I updated wrong bug, sorry. Reopen it.

Comment 16

[Hidetoshi Tajima]

Worksforme, bruce, please verify.

Comment 17

[Bruce Mitchener]

Did you check something in?

Comment 18

[Hidetoshi Tajima]

Yes, I did checked in mozilla/widget/src/gtk/nsWindow.cpp, version 1.297.
The tmpProp is no longer used there.

Comment 19

[John Morrison]

verified wfm (or, the current code frees the temporary prop in either branch).