DOM: Fix gmail event handler registry crash #53510
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
EventHandlerRegistry
is used indirectly by the compositor to determineif a local frame root has a node with an event listener for certain
types of events that the compositor could otherwise fast-path around
to avoid the main thread, if there are no relevant event listeners.
This class is 1:1 with local frame roots, which means there must be
logic to register/unregister nodes that move across same-origin
local roots. This ensures a cross-document adopted node's old local
frame root no longer tracks nodes that move out of it, and the node's
new local root starts tracking adopted nodes that have the relevant
event listeners that EHR cares about.
This logic exists in
Node::WillMoveToNewDocument()
andNode::MoveEventListenersToNewDocument()
, however before this CL itwas broken. It would only trigger a transfer across
EventHandlerRegistries if the document was moving across blink::Page
objects. However, there can be multiple same-origin local frame roots
in the same blink::Page, so the transfer logic was kicking in under
fewer circumstances that it should have.
This made it possible to:
EventHandlerRegistry::CheckConsistency()
that ensures every tracked event target shares the local frame
root that's associated with the given EHR. The most notable site
triggering this DCHECK() was gmail, which is how this issue was
discovered.
the event listeners on that node that the compositor cares about
(see
EventHandlerRegistry::EventHandlerClass
) because thecompositor would never know about these listener after the node
transferred to its new local frame root's registry.
This CL fixes the update logic to transfer event targets across
registries whenever event targets move across local frame roots, and
adds two tests, one for each scenario above.
R=dbaron
Bug: 40277823
Change-Id: Icde11417e80850fe7c512d90895c90ddae9ac85b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6674259
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1480989}