Avoid doing some expensive work where possible at the end of Selection::NotifySelectionChanged() while calling the selection listeners

RESOLVED FIXED in Firefox 57

Status

()

enhancement
RESOLVED FIXED
2 years ago
a month ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks 1 bug)

unspecified
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(2 attachments)

Comment hidden (empty)
Attachment #8897574 - Flags: review?(bugs) → review+
Comment on attachment 8897575 [details] [diff] [review]
Part 2: Avoid using QueryInterface() to convert nsIDocument to nsIDOMDocument

You need to keep the document alive, so can't drop nsCOMPtr<nsIDOMDocument>.
So, unfortunately there is the extra addref/release here.
With that fixed, r+

If this is super hot, we could in theory cast nsIDocument to nsDocument and use
nsDocumentOnStack dos(document); to keep the object alive.
Attachment #8897575 - Flags: review?(bugs) → review+
(Assignee)

Comment 4

2 years ago
The motivating factor in the profile was the QI, so I'm happy to add the nsCOMPtr back.

Comment 5

2 years ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/230847d248d2
Part 1: Bail out early if there are no selection listeners to notify; r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/479d2103841e
Part 2: Avoid using QueryInterface() to convert nsIDocument to nsIDOMDocument; r=smaug

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/230847d248d2
https://hg.mozilla.org/mozilla-central/rev/479d2103841e
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee: nobody → ehsan
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.