Closed Bug 1460101 Opened 4 years ago Closed 4 years ago

Make ESM::ContentRemoved a bit faster.

Categories

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

Product:
Core
Component:
DOM: Events
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla62
Tracking Flags:
Tracking Status
firefox62 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(2 files)

Bug 1460101: Check whether the node is an element in the chain instead of ContentIsDescendantOf.
59 bytes, text/x-review-board-request
smaug
: review+
Details
Relevant info of the assertion failure on try.
3.24 KB, text/plain
Details 
We can just check that the element belongs to the hover / active chain instead of going through ContentIsDescendantOf.
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
Attachment #8974226 - Flags: review?(bugs) → review+
Priority: -- → P3
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?
Flags: needinfo?(bugs)
(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".
I talked with Olli over IRC, will add an XBL-specific condition to the assertion.
Flags: needinfo?(bugs)
Depends on: 1461299
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/64c4562813fa
Check whether the node is an element in the chain instead of ContentIsDescendantOf. r=smaug
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
Flags: needinfo?(emilio)
Backout by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dde66eb4736
Backed out changeset 64c4562813fa for debug crashtest failures on crashtests/626014.xhtml
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/783cf8b58ba4
Check whether the node is an element in the chain instead of ContentIsDescendantOf. r=smaug
Flags: needinfo?(emilio)
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
Flags: needinfo?(emilio)
*sigh* :((

Asserting IsNativeAnonymous in DestroyNativeAnonymousContent shouldn't be that hard... Thanks, looking.
Flags: needinfo?(emilio)
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd55c2c6b6fa
Check whether the node is an element in the chain instead of ContentIsDescendantOf. r=smaug
https://hg.mozilla.org/mozilla-central/rev/dd55c2c6b6fa
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Assignee: nobody → emilio
You need to log in before you can comment on or make changes to this bug.