Closed
Bug 730587
Opened 12 years ago
Closed 12 years ago
Stash pointer to the subtree parent on nodes in disconnected subtrees
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: khuey, Assigned: khuey)
References
Details
Attachments
(1 file, 1 obsolete file)
14.92 KB,
patch
|
smaug
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
Being able to always quickly get the subtree parent will make some CC optimizations more performant.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #601012 -
Flags: superreview?(jst)
Attachment #601012 -
Flags: review?(bugs)
Comment 2•12 years ago
|
||
Comment on attachment 601012 [details] [diff] [review] Patch >- nsIFrame* GetPrimaryFrame() const { return mPrimaryFrame; } >+ nsIFrame* GetPrimaryFrame() const { { should be in the next line. >+ void SetSubtreeRootPointer(nsINode* aSubtreeRoot) { ditto >+ void ClearSubtreeRootPointer() { And here. > nsINode::~nsINode() > { > NS_ASSERTION(!HasSlots(), "nsNodeUtils::LastRelease was not called?"); >+ NS_ASSERTION(mSubtreeRoot == this, "Didn't restore state properly?"); > } Use stronger MOZ_ASSERT here?
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #601012 -
Attachment is obsolete: true
Attachment #601033 -
Flags: superreview?(jst)
Attachment #601033 -
Flags: review?(bugs)
Attachment #601012 -
Flags: superreview?(jst)
Attachment #601012 -
Flags: review?(bugs)
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #2) > Comment on attachment 601012 [details] [diff] [review] > Patch > > > >- nsIFrame* GetPrimaryFrame() const { return mPrimaryFrame; } > >+ nsIFrame* GetPrimaryFrame() const { > { should be in the next line. Done. > >+ void SetSubtreeRootPointer(nsINode* aSubtreeRoot) { > ditto Done. > >+ void ClearSubtreeRootPointer() { > And here. Done. > > nsINode::~nsINode() > > { > > NS_ASSERTION(!HasSlots(), "nsNodeUtils::LastRelease was not called?"); > >+ NS_ASSERTION(mSubtreeRoot == this, "Didn't restore state properly?"); > > } > Use stronger MOZ_ASSERT here? It seems odd to have one fatal assert when we use NS_ASSERTION all over the DOM code.
Comment 5•12 years ago
|
||
Well, we haven't had MOZ_ASSERT for a long time. In this case I really want debug builds to crash asap if something goes wrong.
Comment 6•12 years ago
|
||
Comment on attachment 601033 [details] [diff] [review] Patch >+ nsIFrame* GetPrimaryFrame() const >+ { >+ return IsInDoc() ? mPrimaryFrame : nsnull; I wonder if this shows up in the profiles. Probably not. >+ union { I think { should be in the next line.
Attachment #601033 -
Flags: review?(bugs) → review+
Comment 7•12 years ago
|
||
Comment on attachment 601033 [details] [diff] [review] Patch Looks good.
Attachment #601033 -
Flags: superreview?(jst) → superreview+
Assignee | ||
Comment 8•12 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/5280e98d2d77
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Comment 9•12 years ago
|
||
Pushed a followup to make OwnerDocAsNode non-inline, with rs=khuey: https://hg.mozilla.org/integration/mozilla-inbound/rev/f2bacfc9013c
Version: unspecified → Trunk
Comment 11•12 years ago
|
||
We should consider backing this out of Aurora 14 due to bug 736695.
tracking-firefox14:
--- → ?
Comment 12•12 years ago
|
||
Although backing this out will probably end up being the solution, we'll instead continue to track bug 736695 since it's the actual regression.
Assignee | ||
Comment 13•12 years ago
|
||
Backed out of 14 to fix bug 736695. https://hg.mozilla.org/releases/mozilla-aurora/rev/abe9bb783f88
Target Milestone: mozilla14 → mozilla15
Comment 14•12 years ago
|
||
We need to back this out of Aurora 15 as well....
tracking-firefox15:
--- → ?
Assignee | ||
Comment 16•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/44103bdf65b1
Target Milestone: mozilla15 → mozilla16
Assignee | ||
Comment 17•12 years ago
|
||
And once more, with feeling: https://hg.mozilla.org/releases/mozilla-aurora/rev/0f805ac53831
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•