Bug 288378 – REGRESSION (289309@main): [wk1] fast/harness/internals-object-property-access-on-window-without-frame-crash.html causing multiple tests to fail under imported/w3c/web-platform-tests/css/css-grid/grid-items/*.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jonathan Bedard]

Sometimes, many imported/w3c/web-platform-tests/css/css-grid/grid-items/* tests start failing on WebKitLegacy. The first failure we see in infrastructure is at 289398@main, although this is merely the upper bound of the regression point, not the regression point itself. (determined based on https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcss-grid%2Fgrid-items%2Fanonymous-grid-item-001.html)

https://ews-build.webkit.org/#/builders/135/builds/481 is an example of this failure mode in EWS.

What especially odd about this failure mode is that it seems that every-other 'css-grid/grid-items' test is failing, rather than every single test when we find ourselves in this mode. It means that we're likely not dealing with an off-by-1 error.

[Radar WebKit Bug Importer]

<rdar://problem/145488787>

[Ben Schwartz]

I suspect this could be an issue with these tests being run after a certain test(s) on the same worker. Will do some bisection here.

[Jonathan Bedard]

*** Bug 288243 has been marked as a duplicate of this bug. ***

[Ben Schwartz]

My suspicion was correct. It looks like the test leaving DumpRenderTree in a bad state is `fast/harness/internals-object-property-access-on-window-without-frame-crash.html`. If any of the affected tests are alone, they'll pass; however, if they're run with that one before, they'll fail every time (and pass when the retry on the unexpected failure occurs).

The problematic test was added at 289309@main, and the issue does not reproduce at 289308@main.

[EWS]

Test gardening commit 290986@main (2c7c77fce750): <https://commits.webkit.org/290986@main>

Reviewed commits have been landed. Closing PR #41237 and removing active labels.

[Ben Schwartz]

CCing Frédéric, as they committed the change at 289309@main.

[Frédéric Wang (:fredw)]

So this test verifies that calling internals API on a window without a frame and can reveal real bugs with APIs interacting with the DOM tree.

However, some APIs (e.g. terminateWebContentProcess) also do "weird" things while at the same time not interacting with the DOM tree, and so they are listed in the skippedInternalFunctions array to exclude them from the test. I believed I had already verified the problematic ones, but there are more that affect subsequent tests that I forgot. Also, maybe new APIs were introduced recently. I'll take a look.

[Ben Schwartz]

A manual blocklist of problematic functions seems to me like it would be vulnerable to changes over time. Is there a better way to determine which functions should be excluded? If not, maybe the test should be re-written?

[Frédéric Wang (:fredw)]

> A manual blocklist of problematic functions seems to me like it would be vulnerable to changes over time. Is there a better way to determine which functions should be excluded? If not, maybe the test should be re-written?

Right, I had thought about it when I wrote it. I did that because fuzzers generate JS code with random internal API calls and detect nullptr crashes in our testing code. I could just have written a test for the specific report in bug 286252 but then we would get similar bug reports for current APIs and future APIs, so I thought it would be better to try and catch a maximum of this kind of crashes so that people fix their code when they add new internal APIs.

[Frédéric Wang (:fredw)]

Pull request: https://github.com/WebKit/WebKit/pull/41387

[Frédéric Wang (:fredw)]

> Pull request: https://github.com/WebKit/WebKit/pull/41387

I uploaded https://github.com/WebKit/WebKit/pull/41387 to skip that function and reactivate the test.

I was able to reproduce with

Tools/Scripts/run-webkit-tests --release --dump-render-tree --child-processes=1 LayoutTests/fast/harness/internals-object-property-access-on-window-without-frame-crash.html LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/anonymous-grid-item-001.html

After running MozillaSecurity/lithium to get a minimal set of functions to skip, the problematic one is internals.setGridMaxTracksLimit().

As a follow-up, an alternative way of testing could be to have two explicit lists for the functions included and the functions excluded and make the test throw for unknown functions. That will keep the benefit of not missing coverage for future functions but letting WebKit developers decide whether they should be covered by the test.

[EWS]

Committed 291242@main (c6c694c35157): <https://commits.webkit.org/291242@main>

Reviewed commits have been landed. Closing PR #41387 and removing active labels.