1460101 - Make ESM::ContentRemoved a bit faster.

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

We can just check that the element belongs to the hover / active chain instead of going through ContentIsDescendantOf.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1460101: Check whether the node is an element in the chain instead of ContentIsDescendantOf.

We rely on :hover and :active being hierarchical, and on the fact that there are
only elements and documents in the flattened tree ancestor chain if the element
is in the composed doc.

Review commit: https://reviewboard.mozilla.org/r/242516/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242516/

Comment 2

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

Comment on attachment 8974226 [details]
Bug 1460101: Check whether the node is an element in the chain instead of ContentIsDescendantOf.

https://reviewboard.mozilla.org/r/242516/#review248442

Yeah,this should be good

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Relevant info of the assertion failure on try.

Hmm, so there's a single devtools test in tree that triggers the assertion on try:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=7177f9ef93c351b409328b4dee1d93a0d620841d

This is because mHoverContent belongs to the anonymous subtree in XBL, and we unbind it without notifying (see the test-case).

In any case, this looks like the behavior now would be more correct (we would correctly clear the state of the non-anonymous node, and keep mHoverContent in a bogus state less often).

Olli, would you be ok with me landing this patch without the assertion, or with it turned into something like an NS_ASSERTION / NS_WARNING_ASSERTION or with a chrome exception?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(In reply to Emilio Cobos Álvarez [:emilio] (Away-ish from 27/04 to 09/05) from comment #3)
> This is because mHoverContent belongs to the anonymous subtree in XBL, and
> we unbind it without notifying (see the test-case).

Err, I meant "see the attached file".

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I talked with Olli over IRC, will add an XBL-specific condition to the assertion.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/64c4562813fa
Check whether the node is an element in the chain instead of ContentIsDescendantOf. r=smaug

Comment 7

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 64c4562813fa (bug 1460101) for debug crashtest failures on crashtests/626014.xhtml

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/d76102e32327391dc69b83a3a4da0c2ea55255f3

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=699e9816b7ff862cf30db83e8f365cf50d02e90f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running&selectedJob=178431306

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=178431306&repo=mozilla-inbound&lineNumber=28817

22:06:41     INFO -  REFTEST TEST-PASS | file:///Z:/task_1526333972/build/tests/reftest/tests/layout/forms/crashtests/626014.xhtml | (LOAD ONLY)
22:06:41     INFO -  REFTEST TEST-END | file:///Z:/task_1526333972/build/tests/reftest/tests/layout/forms/crashtests/626014.xhtml
22:06:41     INFO -  ++DOMWINDOW == 200 (11D6F800) [pid = 572] [serial = 4756] [outer = 05BC1540]
22:06:41    ERROR -  REFTEST TEST-UNEXPECTED-FAIL | file:///Z:/task_1526333972/build/tests/reftest/tests/layout/forms/crashtests/626014.xhtml | assertion count 1 is more than expected 0 assertions

Comment 8

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dde66eb4736
Backed out changeset 64c4562813fa for debug crashtest failures on crashtests/626014.xhtml

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/783cf8b58ba4
Check whether the node is an element in the chain instead of ContentIsDescendantOf. r=smaug

Comment 10

[Cristian Brindusan [:cbrindusan]]

Backed out 3 changesets (bug 1461299, bug 1460101) for reftest failures on events/EventStateManager.cpp

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/9d8086740c860e8f7beeb890401b7d850d68f539

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=fe2b8fcd7a475b65fd5cf85921a58ff22868ac53&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running&selectedJob=178456463

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=178456463&repo=mozilla-inbound&lineNumber=3205

[task 2018-05-14T23:59:27.560Z] 23:59:27     INFO - REFTEST TEST-START | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-extents.html == file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-extents-ref.html
[task 2018-05-14T23:59:27.562Z] 23:59:27     INFO - REFTEST TEST-LOAD | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-extents.html | 22 / 23 (95%)
[task 2018-05-14T23:59:27.580Z] 23:59:27     INFO - ++DOMWINDOW == 36 (0x7ff052d35400) [pid = 1338] [serial = 70] [outer = 0x7ff073099400]
[task 2018-05-14T23:59:27.900Z] 23:59:27     INFO - --DOMWINDOW == 35 (0x7ff052d3a800) [pid = 1338] [serial = 39] [outer = (nil)] [url = data:text/html;charset=UTF-8,%3C%21%2D%2DCLEAR%2D%2D%3E]
[task 2018-05-14T23:59:27.902Z] 23:59:27     INFO - --DOMWINDOW == 34 (0x7ff052d39400) [pid = 1338] [serial = 40] [outer = (nil)] [url = file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-objectpattern.svg]
[task 2018-05-14T23:59:27.904Z] 23:59:27     INFO - --DOMWINDOW == 33 (0x7ff052d31400) [pid = 1338] [serial = 41] [outer = (nil)] [url = file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-objectpattern-ref.svg]
[task 2018-05-14T23:59:27.904Z] 23:59:27     INFO - --DOMWINDOW == 32 (0x7ff05300f800) [pid = 1338] [serial = 42] [outer = (nil)] [url = data:text/html;charset=UTF-8,%3C%21%2D%2DCLEAR%2D%2D%3E]
[task 2018-05-14T23:59:27.907Z] 23:59:27     INFO - --DOMWINDOW == 31 (0x7ff052d3ec00) [pid = 1338] [serial = 36] [outer = (nil)] [url = data:text/html;charset=UTF-8,%3C%21%2D%2DCLEAR%2D%2D%3E]
[task 2018-05-14T23:59:27.908Z] 23:59:27     INFO - --DOMWINDOW == 30 (0x7ff052d36c00) [pid = 1338] [serial = 38] [outer = (nil)] [url = file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-objectgradient-zoom-ref.svg]
[task 2018-05-14T23:59:27.908Z] 23:59:27     INFO - --DOMWINDOW == 29 (0x7ff052d3cc00) [pid = 1338] [serial = 37] [outer = (nil)] [url = file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-objectgradient-zoom.svg]
[task 2018-05-14T23:59:27.915Z] 23:59:27     INFO - --DOMWINDOW == 28 (0x7ff052d39800) [pid = 1338] [serial = 44] [outer = (nil)] [url = file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/clip-ref.html]
[task 2018-05-14T23:59:27.917Z] 23:59:27     INFO - --DOMWINDOW == 27 (0x7ff05300f000) [pid = 1338] [serial = 43] [outer = (nil)] [url = file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/clip.html]
[task 2018-05-14T23:59:30.864Z] 23:59:30     INFO - REFTEST TEST-LOAD | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-extents-ref.html | 22 / 23 (95%)
[task 2018-05-14T23:59:30.882Z] 23:59:30     INFO - ++DOMWINDOW == 28 (0x7ff051685800) [pid = 1338] [serial = 71] [outer = 0x7ff073099400]
[task 2018-05-14T23:59:31.077Z] 23:59:31     INFO - REFTEST TEST-PASS | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-extents.html == file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-extents-ref.html | image comparison, max difference: 0, number of differing pixels: 0
[task 2018-05-14T23:59:31.079Z] 23:59:31     INFO - REFTEST TEST-END | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-extents.html == file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-extents-ref.html
[task 2018-05-14T23:59:31.082Z] 23:59:31     INFO - ++DOMWINDOW == 29 (0x7ff051686800) [pid = 1338] [serial = 72] [outer = 0x7ff073099400]
[task 2018-05-14T23:59:31.128Z] 23:59:31     INFO - REFTEST INFO | Slowest test took 3287ms (file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-extents.html)
[task 2018-05-14T23:59:31.130Z] 23:59:31     INFO - REFTEST INFO | Total canvas count = 0
[task 2018-05-14T23:59:31.515Z] 23:59:31     INFO - JavaScript error: chrome://reftest/content/reftest.jsm, line 1554: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIPropertyBag2.getPropertyAsAString]
[task 2018-05-14T23:59:31.517Z] 23:59:31     INFO - Assertion failure: aContent->IsRootOfNativeAnonymousSubtree(), at /builds/worker/workspace/build/src/dom/events/EventStateManager.cpp:5411

Comment 11

[Emilio Cobos Álvarez (:emilio)]

*sigh* :((

Asserting IsNativeAnonymous in DestroyNativeAnonymousContent shouldn't be that hard... Thanks, looking.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd55c2c6b6fa
Check whether the node is an element in the chain instead of ContentIsDescendantOf. r=smaug

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/dd55c2c6b6fa