1454835 - Intermittent /css/css-shapes/spec-examples/shape-outside-012.html | CSS Test: Shape from image - shape-image-threshold - 0.9 - uncaught exception: Error: assert_unreached: Giving up waiting for shape layout to happen! Reached unreachable code

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: ncsoregi [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=174183338&repo=autoland

https://queue.taskcluster.net/v1/task/fDicUb-DQ-iQWDO9S9nBcQ/runs/0/artifacts/public/logs/live_backing.log

[task 2018-04-17T20:13:13.892Z] 20:13:13     INFO - TEST-START | /css/css-shapes/spec-examples/shape-outside-012.html
[task 2018-04-17T20:13:14.219Z] 20:13:14     INFO - PID 6894 | ERROR 2018-04-17T20:13:14Z: webrender::display_list_flattener: Unknown pipeline used for iframe IframeDisplayItem { clip_id: Clip(4, PipelineId(1, 1)), pipeline_id: PipelineId(1, 5) }
[task 2018-04-17T20:13:14.479Z] 20:13:14     INFO - PID 6894 | ++DOCSHELL 0x7fd2dca35800 == 5 [pid = 6959] [id = {b5a3031d-c716-4cb6-b94e-bd239240c2f4}]
[task 2018-04-17T20:13:14.481Z] 20:13:14     INFO - PID 6894 | ++DOMWINDOW == 13 (0x7fd2dd4b2c00) [pid = 6959] [serial = 14] [outer = (nil)]
[task 2018-04-17T20:13:14.580Z] 20:13:14     INFO - PID 6894 | ++DOMWINDOW == 14 (0x7fd2dd4b7400) [pid = 6959] [serial = 15] [outer = 0x7fd2dd4b2c00]
[task 2018-04-17T20:13:14.702Z] 20:13:14     INFO - PID 6894 | --DOCSHELL 0x7f1dbdf74000 == 3 [pid = 6894] [id = {c84de5ad-bef4-416f-916a-c9931a9003b7}]
[task 2018-04-17T20:13:14.867Z] 20:13:14     INFO - PID 6894 | ++DOMWINDOW == 15 (0x7fd2dd4b7000) [pid = 6959] [serial = 16] [outer = 0x7fd2dd4b2c00]
[task 2018-04-17T20:13:14.985Z] 20:13:14     INFO - PID 6894 | ERROR 2018-04-17T20:13:14Z: webrender::display_list_flattener: Unknown pipeline used for iframe IframeDisplayItem { clip_id: Clip(4, PipelineId(1, 1)), pipeline_id: PipelineId(1, 6) }
[task 2018-04-17T20:13:15.074Z] 20:13:15     INFO - PID 6894 | ERROR 2018-04-17T20:13:15Z: webrender::display_list_flattener: Unknown pipeline used for iframe IframeDisplayItem { clip_id: Clip(4, PipelineId(1, 1)), pipeline_id: PipelineId(1, 6) }
[task 2018-04-17T20:13:15.143Z] 20:13:15     INFO - PID 6894 | --DOCSHELL 0x7fd2e8091000 == 4 [pid = 6959] [id = {779ae39b-5947-40b9-b918-819143ccba2e}]
[task 2018-04-17T20:13:16.773Z] 20:13:16     INFO - PID 6894 | JavaScript error: , line 0: uncaught exception: Error: assert_unreached: Giving up waiting for shape layout to happen! Reached unreachable code
[task 2018-04-17T20:13:16.953Z] 20:13:16     INFO - PID 6894 | [Parent 6894, Main Thread] WARNING: NS_ENSURE_TRUE(aSecondURI) failed: file /builds/worker/workspace/build/src/dom/base/ThirdPartyUtil.cpp, line 98
[task 2018-04-17T20:13:17.040Z] 20:13:17     INFO - 
[task 2018-04-17T20:13:17.041Z] 20:13:17     INFO - TEST-UNEXPECTED-FAIL | /css/css-shapes/spec-examples/shape-outside-012.html | CSS Test: Shape from image - shape-image-threshold - 0.9 - uncaught exception: Error: assert_unreached: Giving up waiting for shape layout to happen! Reached unreachable code

Comment 1

[Brad Werth [:bradwerth]]

This test, and several others in the directory seem to use a race condition to check float positions, instead of waiting for the document onload event to fire: https://searchfox.org/mozilla-central/source/testing/web-platform/tests/css/css-shapes/spec-examples/support/spec-example-utils.js. I'll change these tests to wait for the onload event, and then they'll work at least as consistently as the other shape-outside tests.

Comment 2

[Brad Werth [:bradwerth]]

Attachment: Bug 1454835: Remove racy-ness from some shape-outside web-platform tests.

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

Comment 3

[Brad Werth [:bradwerth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f046fb8f56b9968b8144b4aacb11e951eb14738

Comment 4

[Brad Werth [:bradwerth]]

Comment on attachment 8969120 [details]
Bug 1454835: Remove racy-ness from some shape-outside web-platform tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237828/diff/1-2/

Comment 5

[Brad Werth [:bradwerth]]

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

Comment 6

[Intermittent Failures Robot]

1 failures in 782 pushes (0.001 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* autoland: 1

Platform breakdown:
* linux64-qr: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1454835&startday=2018-04-16&endday=2018-04-22&tree=trunk

Comment 7

[Brad Werth [:bradwerth]]

The proposed patches are correct and necessary, but there is a root cause issue in the platform that also must be resolved. Bug 1454694 is tracking that root cause, and will block this bug.

Comment 8

[Brad Werth [:bradwerth]]

Comment on attachment 8969120 [details]
Bug 1454835: Remove racy-ness from some shape-outside web-platform tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237828/diff/2-3/

Comment 9

[Brad Werth [:bradwerth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=130e6ddcfa1b2966cfaa2adf2839d68cd09f6dbf

Comment 10

[Daniel Holbert [:dholbert]]

Comment on attachment 8969120 [details]
Bug 1454835: Remove racy-ness from some shape-outside web-platform tests.

https://reviewboard.mozilla.org/r/237828/#review250806

r=me, though since this is a shared test not authored by us, I'm going to bikeshed the commit-message language a bit, in the interests of making sure the reason for the change is extra-clear to third parties if/when they sync it back upstream & into their repos.

::: commit-message-4af4a:3
(Diff revision 3)
> +These tests rely on a race condition to wait for shape-outside layout to
> +complete. This change makes those tests instead trigger on the onload event. It
> +also removes the race code and replaces it with an assert.

Nit #1: It doesn't quite make sense to say that tests "rely on a race condition".  To my eye, that literally means the tests rely on nondeterminism (i.e. rely on *the fact that something is nondeterministic*) in order to pass.  And that's not what you mean to say.

Nit #2: "removes the race code" is a bit vague RE whether it's talking about the code-with-a-racy-dependency vs. the hackaround for the race (you mean the latter but it sounds like the former).

Here's some suggested replacement text that's a bit more direct/clear, I think (feel free to edit/reword as you like - this is just what I came up with):
==========
"Without this change, these tests have no guarantee about whether the rendering will be complete by the time the test's JS fires. Up until now, the tests worked around this issue with a hacky setTimeout loop. This patch addresses the race condition (and removes the workaround for it) by making the test JS fire from the body's load event, at which point the layout should be complete & reliably testable."

Comment 11

[Brad Werth [:bradwerth]]

Comment on attachment 8969120 [details]
Bug 1454835: Remove racy-ness from some shape-outside web-platform tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237828/diff/3-4/

Comment 12

[Brad Werth [:bradwerth]]

Comment on attachment 8969120 [details]
Bug 1454835: Remove racy-ness from some shape-outside web-platform tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237828/diff/4-5/

Comment 13

[Brad Werth [:bradwerth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=63866d0f21291de9b2d5ca6832517fed95e017da

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/927fe2d0a4b9
Remove racy-ness from some shape-outside web-platform tests. r=dholbert

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/11061 for changes under testing/web-platform/tests

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 17

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/927fe2d0a4b9

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged