Bug 234933 - [AppleWin] Some fast/shadow-dom/fullscreen-in-* tests are crashing after r287698
Summary: [AppleWin] Some fast/shadow-dom/fullscreen-in-* tests are crashing after r287698
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
Keywords: InRadar
Depends on:
Blocks: 233963
  Show dependency treegraph
Reported: 2022-01-06 13:23 PST by Fujii Hironori
Modified: 2022-03-01 10:24 PST (History)
11 users (show)

See Also:

Patch (8.04 KB, patch)
2022-01-21 16:44 PST, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2022-01-06 13:23:41 PST
Since r287698 (Bug 233963), AppleWin is crashing.

Regressions: Unexpected crashes (5)
  accessibility/insert-children-assert.html [ Crash ]
  fast/shadow-dom/fullscreen-in-shadow-event-should-propagate.html [ Crash ]
  fast/shadow-dom/fullscreen-in-shadow-full-screen-ancestor.html [ Crash ]
  fast/shadow-dom/fullscreen-in-shadow-fullscreenElement.html [ Crash ]
  fast/shadow-dom/fullscreen-in-slot-fullscreenElement.html [ Crash ]
Comment 1 Fujii Hironori 2022-01-06 13:29:10 PST
Since WinCairo skip fast/shadow-dom tests, I mannually skipped those tests and ran tests locally. I don't observe such crash with WinCairo.
I thinik WinCairo bots and I are using newer version of MSVC than AppleWin EWS bots. It's worth to try updating the compiler before debugging.
Comment 2 Fujii Hironori 2022-01-07 17:30:59 PST
r287770 reverted the change. The crashes have gone.
> 23:09:08.281 35661 worker/7 fast/shadow-dom/fullscreen-in-shadow-event-should-propagate.html passed
Comment 3 Alex Christensen 2022-01-08 08:48:24 PST
There is a significant amount of different code between AppleWin and WinCairo in the fullscreen implementation.  Could someone check running AppleWin on those tests with bug 233963 applied to see whether it is just needing newer MSVC or whether there is something going on in that code?
Comment 4 Fujii Hironori 2022-01-08 19:21:08 PST
I'm using Visual Studio 2019 version 16.11.8.
I have to work around some compilation errors to compile out AppleWin. https://gist.github.com/fujii/6dd1d7da709ca7c9015b1d0b82b289d9
I don't observe such crashes for AppleWin on my env.
Comment 6 Robert Jenner 2022-01-12 10:39:41 PST
This was also slowing down EWS on Windows. I have added a test expectation for these tests as [ Crash ] for Windows, to take care of the lag on EWS while this is being investigated. Test expectations were added here:

Comment 7 Robert Jenner 2022-01-12 10:45:27 PST
I missed marking the accessibility test that goes along with this bug as well. Marked here:

Comment 8 Radar WebKit Bug Importer 2022-01-13 13:24:16 PST
Comment 9 Alex Christensen 2022-01-20 18:40:24 PST
Fujii, could you verify which version of MSVC you are using that does not exhibit these crashes?  https://build.webkit.org/#/builders/56/builds/8455/steps/9/logs/stdio line 87 says this from our open source builders:
"The CXX compiler identification is MSVC 19.23.28106.4"

Our internal builders are using 19.24.28316.0 and I'm wondering if that's new enough.
Comment 10 Fujii Hironori 2022-01-20 23:09:47 PST
They are Visual Studio 2019 Version 16.3 and 16.4.

I'm using version 16.11 that is the best for C++20.
Comment 11 Alex Christensen 2022-01-20 23:34:52 PST
If it wouldn't be too much trouble, could you do a clean build and look for "The CXX compiler identification is MSVC xx.xx.xxxxx.x" and let me know what those exact numbers are?  That's all I have to compare from the logs.
Comment 12 Alex Christensen 2022-01-21 00:14:31 PST
I verified all your changes were necessary and I left the m_download = nullptr; at the end and am committing it in https://bugs.webkit.org/show_bug.cgi?id=235431
Comment 13 Alex Christensen 2022-01-21 16:44:02 PST
Created attachment 449711 [details]
Comment 14 Maciej Stachowiak 2022-01-22 13:43:59 PST
Comment on attachment 449711 [details]

Comment 15 Alex Christensen 2022-01-24 14:03:26 PST
Thanks for the review, but all the blocking radars need to be resolved before this can land.
Comment 16 EWS 2022-03-01 10:23:55 PST
Committed r290656 (247929@main): <https://commits.webkit.org/247929@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449711 [details].