Closed Bug 1563405 Opened 4 months ago Closed 3 months ago

Avoid some refcounting in XULContentSinkImpl::ContextStack::Push()

Categories

(Core :: XUL, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(2 files)

In a comment in my part 3 patch for bug 1563066, bz said:

So in OpenRoot, we push on the stack, then add the attributes. But in this code, we add the attributes, then children->AppendElement(), then push on the context stack.
(As a followup?) it might be worth making the order always be "add attributes, then push on the stack" and make the stack-push take an already_AddRefed so we can forget() into it.

AddAttributes has no side effects on |this|, so the reordering is okay. It can't be made static because it calls a method that reads the node info manager field off of |this| or something.

I used RefPtr<>&& instead of already_AddRefed<>, because the ref pointer has to get passed in to the Entry ctor, so you have to use some std::move anyways.

AddAttributes has no side effects on |this|, so that's okay.

Also remove an extra semi colon.

This avoids some refcounting.

Also remove a pointless local variable |entry|.

Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a4f406edcb1a
part 1 - Call AddAttributes earlier in XULContentSinkImpl::OpenRoot(). r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/dfe7d3650300
part 2 - Use move refs for Push in nsXULContentSink. r=bzbarsky
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.