Bug 234933 – [AppleWin] Some fast/shadow-dom/fullscreen-in-* tests are crashing after r287698

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

[Fujii Hironori]

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 ]

[Fujii Hironori]

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.

[Fujii Hironori]

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

[Alex Christensen]

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?

[Fujii Hironori]

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.

[Fujii Hironori]

r287848 landed the patch again. The tests have started crashing again.
https://build.webkit.org/#/builders/50/builds/3943
https://results.webkit.org/?suite=layout-tests&test=fast%2Fshadow-dom%2Ffullscreen-in-shadow-event-should-propagate.html&platform=win

[Robert Jenner]

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:

https://trac.webkit.org/changeset/287932/webkit

[Robert Jenner]

I missed marking the accessibility test that goes along with this bug as well. Marked here:

https://trac.webkit.org/changeset/287933/webkit

[Radar WebKit Bug Importer]

<rdar://problem/87567514>

[Alex Christensen]

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.

[Fujii Hironori]

They are Visual Studio 2019 Version 16.3 and 16.4.
https://en.wikipedia.org/wiki/Microsoft_Visual_C%2B%2B

I'm using version 16.11 that is the best for C++20.
https://devblogs.microsoft.com/cppblog/msvc-cpp20-and-the-std-cpp20-switch/

[Alex Christensen]

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.

[Alex Christensen]

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

[Alex Christensen]

Created attachment 449711 [details]
Patch

[Maciej Stachowiak]

Comment on attachment 449711 [details]
Patch

r=me

[Alex Christensen]

Thanks for the review, but all the blocking radars need to be resolved before this can land.

[EWS]

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].