Don't traverse certainly alive selections

RESOLVED FIXED in mozilla12

Status

()

Core
Selection
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

(Blocks: 1 bug)

Trunk
mozilla12
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
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.
Attachment #584818 - Flags: review?(matspal)
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.
(Assignee)

Updated

5 years ago
Attachment #584818 - Flags: review?(matspal) → review?(continuation)
(Assignee)

Comment 2

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=685465da3f10
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.
Attachment #584818 - Flags: review?(continuation) → review+
Blocks: 640853, 698919
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/0d684c34d1e4
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12

Updated

5 years ago
Version: 10 Branch → Trunk
(Assignee)

Comment 5

5 years ago
Created attachment 585860 [details] [diff] [review]
Safer version

I started to worry about this a bit, so better be safe.
Attachment #585860 - Flags: review?(continuation)
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).
Attachment #585860 - Flags: review?(continuation) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/57a33c9f08b1
Blocks: 716598
No longer blocks: 698919

Updated

5 years ago
Blocks: 728705
You need to log in before you can comment on or make changes to this bug.