Last Comment Bug 714162 - Don't traverse certainly alive selections
: Don't traverse certainly alive selections
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Selection (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla12
Assigned To: Olli Pettay [:smaug]
:
Mentors:
Depends on:
Blocks: 716598 640853 728705
  Show dependency treegraph
 
Reported: 2011-12-29 12:51 PST by Olli Pettay [:smaug]
Modified: 2012-03-14 00:42 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.08 KB, patch)
2011-12-29 12:51 PST, Olli Pettay [:smaug]
continuation: review+
Details | Diff | Splinter Review
Safer version (1.68 KB, patch)
2012-01-04 12:58 PST, Olli Pettay [:smaug]
continuation: review+
Details | Diff | Splinter Review

Description Olli Pettay [:smaug] 2011-12-29 12:51:22 PST
Created attachment 584818 [details] [diff] [review]
patch

nsFrameSelection is kept alive by Presshell (until Presshell is deleted) and
for text form controls nsTextEditorState/nsTextControlFrame.
When presshell is going away mShell is set to null using DisconnectFromPresShell.

So, as long a mShell is there, the nsFrameSelection object should be certainly alive and 
the whole traversing is just useless.
Comment 1 Mats Palmgren (vacation) 2011-12-29 13:52:55 PST
Yes, the above comment is correct.  I'm not familiar with the cycle collector or
what NS_SUCCESS_INTERRUPTED_TRAVERSE means though, so you should get someone else
to review the actual code change.
Comment 2 Olli Pettay [:smaug] 2011-12-29 14:05:28 PST
https://tbpl.mozilla.org/?tree=Try&rev=685465da3f10
Comment 3 Andrew McCreight [:mccr8] 2011-12-29 16:23:47 PST
Comment on attachment 584818 [details] [diff] [review]
patch

Review of attachment 584818 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Death to death stars!

r=me given mats's comment.
Comment 4 Olli Pettay [:smaug] 2011-12-30 03:12:00 PST
https://hg.mozilla.org/mozilla-central/rev/0d684c34d1e4
Comment 5 Olli Pettay [:smaug] 2012-01-04 12:58:22 PST
Created attachment 585860 [details] [diff] [review]
Safer version

I started to worry about this a bit, so better be safe.
Comment 6 Andrew McCreight [:mccr8] 2012-01-05 08:28:43 PST
Comment on attachment 585860 [details] [diff] [review]
Safer version

Review of attachment 585860 [details] [diff] [review]:
-----------------------------------------------------------------

The diff on top of the previous patch wasn't really necessary for a tiny patch.

At some point, we should probably another CC macro for this pattern (if document is non-null and in the current generation, return interrupted traverse).
Comment 7 Olli Pettay [:smaug] 2012-01-06 11:18:41 PST
https://hg.mozilla.org/mozilla-central/rev/57a33c9f08b1

Note You need to log in before you can comment on or make changes to this bug.