Closed Bug 1845215 Opened 11 months ago Closed 10 months ago

Make `ContentEventHandler` stop using strong pointers as far as possible

Categories

(Core :: DOM: UI Events & Focus Handling, defect, P2)

defect

Tracking

()

RESOLVED FIXED
118 Branch
Tracking Status
firefox118 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [sp3])

Attachments

(5 files)

According to the profile reported in bug 1844470, ContentEventHandler spends on the content iterator which updates and references its nsCOMPtr<nsINode> members a lot. Additionally, ContentEventHandler::NodePosition inherits RangeBoundary and used for temporary use a lot. However, while they are being used, there is no call of MOZ_CAN_RUN_SCRIPT. Therefore, we can make them work without strong pointers.

ContentEventHandler works with PostContentIterator and PreContentIterator
when it scans DOM nodes in a range. While iterating the DOM nodes, script
never runs. Therefore, we can make ContentEventHandler work with new
content iterators which do not store nodes with strong pointers.

This patch makes ContentIteratorBase a template class and create new
UnsafePostContentIterator and UnsafePreContentIterator. They will
check whether DOM mutation occurs before destruction to detect dangerous
regressions.

It's used for tying a set of node and offset in it in short time and
ContentEventHandler does not flush pending things in most paths.
Therefore, it can avoid using RangeBoundary for solving its performance
issue.

Depends on D184441

Ah, in this profile, ContentEventHandler::RawRange appears too. I'll add a patch for it today.

Priority: P3 → P2

Testing again with additional patches...

It's used as both a temporary object in the stack and storing first range
selection range while ContentEventHandler handles everything. In the
first case, ContentEventHandler won't call any methods marked as
MOZ_CAN_RUN_SCRIPT. Therefore, we can make it store only raw nsINode
pointer and RawRangeBoundary.

Depends on D184442

Whiteboard: [sp3]
Attachment #9345483 - Attachment description: Bug 1845215 - Make `ContentIteratorBase` a template class to work without strong pointers r=#dom-core → Bug 1845215 - part 1: Make `ContentIteratorBase` a template class to work without strong pointers r=#dom-core
Attachment #9345484 - Attachment description: Bug 1845215 - Make `ContentEventHandler::NodePosition` inherits `RawRangeBoundary` instead of `RangeBoundary` r=#dom-core → Bug 1845215 - part 2: Make `ContentEventHandler::NodePosition` inherits `RawRangeBoundary` instead of `RangeBoundary` r=#dom-core
Attachment #9345483 - Attachment description: Bug 1845215 - part 1: Make `ContentIteratorBase` a template class to work without strong pointers r=#dom-core → Bug 1845215 - part 1: Make `ContentIteratorBase` a template class to work without strong pointers r=smaug!,#dom-core
Attachment #9345484 - Attachment description: Bug 1845215 - part 2: Make `ContentEventHandler::NodePosition` inherits `RawRangeBoundary` instead of `RangeBoundary` r=#dom-core → Bug 1845215 - part 2: Make `ContentEventHandler::NodePosition` inherits `RawRangeBoundary` instead of `RangeBoundary` r=smaug!,#dom-core
Attachment #9345539 - Attachment description: Bug 1845215 - part 3: Make `ContentEventHandler::RawRange` a template class r=#dom-core → Bug 1845215 - part 3: Make `ContentEventHandler::RawRange` a template class r=smaug!,#dom-core
Attachment #9345540 - Attachment description: Bug 1845215 - part 4: Make `ContentEventHandler` stop using strong pointers if safe r=#dom-core → Bug 1845215 - part 4: Make `ContentEventHandler` stop using strong pointers if safe r=smaug,#dom-core

RawRange sounds like related to RawRangeBoundary, but it now has
RangeBoundary members and UnsafeRawRange has RawRangeBoundarys.

They are helpers for ContentEventHandler to work faster than nsRange
and dom::StaticRange with avoiding to do unnecessary things for
ContentEventHandler. Therefore, this patch renames them to *SimplRange*
since they just store two range boundaries and their root node simply.

Depends on D184467

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3601308f553b
part 1: Make `ContentIteratorBase` a template class to work without strong pointers r=smaug
https://hg.mozilla.org/integration/autoland/rev/681e574c9ccb
part 2: Make `ContentEventHandler::NodePosition` inherits `RawRangeBoundary` instead of `RangeBoundary` r=smaug
https://hg.mozilla.org/integration/autoland/rev/d0a2d3fc1837
part 3: Make `ContentEventHandler::RawRange` a template class r=smaug
https://hg.mozilla.org/integration/autoland/rev/716372eb9576
part 4: Make `ContentEventHandler` stop using strong pointers if safe r=smaug
https://hg.mozilla.org/integration/autoland/rev/b2b54f91567d
part 5: Rename `ContentEventHandler::*RawRange*` to `*SimpleRange*` r=smaug
Regressions: 1849275
Regressions: 1850938
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: