624722 - nsBidiPresUtils should participate in CC

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Peter Van der Beken [:peterv]]

It holds strong refs to nodes, and is owned by the nsPresContext which already participates.

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 2

[Peter Van der Beken [:peterv]]

Leak fix that blocks a blocker should probably block.

Comment 3

[(no longer active)]

Comment on attachment 502796 [details] [diff] [review]
v1

Looks great!

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is OK. However, keeping state in nsBidiPresUtils is actually a bug. That state is local to each API call and should go away completely between calls, in which case there is no need for CC here.

Also, mContentToFrameIndex doesn't need to hold strong refs to content, as far as I can tell.

Comment 5

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/eab687bc329f

Comment 6

[(no longer active)]

(In reply to comment #4)
> This is OK. However, keeping state in nsBidiPresUtils is actually a bug. That
> state is local to each API call and should go away completely between calls, in
> which case there is no need for CC here.
> 
> Also, mContentToFrameIndex doesn't need to hold strong refs to content, as far
> as I can tell.

I filed bug 624798 on this.

Comment 7

[Michael Yoshitaka Erlewine [:mitcho]]

Thank you than you thank you!!

This patch indeed looks to be solving our leaks for both bug 567029 and bug 587503!