Last Comment Bug 737060 - don't traverse a nsXULTemplateBuilder with an observed document
: don't traverse a nsXULTemplateBuilder with an observed document
Status: RESOLVED FIXED
[snappy]
:
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Andrew McCreight [:mccr8]
:
:
Mentors:
Depends on:
Blocks: 716598 722715
  Show dependency treegraph
 
Reported: 2012-03-19 09:31 PDT by Andrew McCreight [:mccr8]
Modified: 2012-03-22 18:13 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
nsXULTemplateBuilder CC graph blob (24.60 KB, application/pdf)
2012-03-19 09:31 PDT, Andrew McCreight [:mccr8]
no flags Details
if we have an observed document, stop traversal (1.76 KB, patch)
2012-03-19 09:39 PDT, Andrew McCreight [:mccr8]
no flags Details | Diff | Splinter Review
if we have an observed document, stop traversal (1.44 KB, patch)
2012-03-20 17:46 PDT, Andrew McCreight [:mccr8]
bent.mozilla: review+
Details | Diff | Splinter Review

Description Andrew McCreight [:mccr8] 2012-03-19 09:31:47 PDT
Created attachment 607193 [details]
nsXULTemplateBuilder CC graph blob

A blob rooted with a number of nsXULTemplateBuilders shows up in the CC graph all the time.  In the attached example, there are 47 nodes.  It turns out that the builder attaches itself as a listener, and so is being held live by the listener service.  In that case, we don't need to traverse it.
Comment 1 Andrew McCreight [:mccr8] 2012-03-19 09:39:09 PDT
Created attachment 607197 [details] [diff] [review]
if we have an observed document, stop traversal

Simple patch.  I'm just not sure if I should add a debug-mode check to see if the template builder is actually a registered listener or not, or if that's just such a fundamental thing that nobody will ever break it.  That would require, in the case where we decide to interrupt traverse, to iterate over all listeners and compare them to ourselves.
Comment 2 Andrew McCreight [:mccr8] 2012-03-20 17:46:21 PDT
Created attachment 607794 [details] [diff] [review]
if we have an observed document, stop traversal
Comment 3 Andrew McCreight [:mccr8] 2012-03-21 20:32:54 PDT
Comment on attachment 607794 [details] [diff] [review]
if we have an observed document, stop traversal

Ben, you are familiar with the cycle collector, and wrote the code I'm relying on here in Bug 416939, back in 2008, so you seem like a good reviewer.  Basically, whenever the template builder has an observed document, it is registered with the global observer service, which holds it alive, so we don't need to traverse it in the cycle collector.

This passed a try run along with some other patches of mine.
Comment 4 Andrew McCreight [:mccr8] 2012-03-22 08:41:33 PDT
Thanks for the quick review!

https://hg.mozilla.org/integration/mozilla-inbound/rev/beb523be92e1
Comment 5 Marco Bonardo [::mak] 2012-03-22 18:13:46 PDT
https://hg.mozilla.org/mozilla-central/rev/beb523be92e1

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