Bug 190098 – WebDriver: thrown ObjC exception under -[WKFullScreenWindowController windowDidFailToEnterFullScreen:] when session is terminated

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Blaze Burg]

This seems to happen when running the full Selenium suite and it tries to tear down the session.

It finally falls apart when cleaning up after window_tests.py::test_should_minimize_the_current_window.

[Blaze Burg]

<rdar://problem/42822671>

[Jer Noble]

Created attachment 351887 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 351887 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351887&action=review

> Source/WebKit/ChangeLog:9
> +        Forcibly exit fullscreen in resetState(), while the VideoFullscreenManagerProxy is still valid.

Should we be catching Objective-C exceptions in more places?

[Jer Noble]

Created attachment 351889 [details]
Patch

[Jer Noble]

(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 351887 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=351887&action=review
> 
> > Source/WebKit/ChangeLog:9
> > +        Forcibly exit fullscreen in resetState(), while the VideoFullscreenManagerProxy is still valid.
> 
> Should we be catching Objective-C exceptions in more places?

This is a C++ problem; we're calling a null pointer (because the VideoFullscreenManager has been cleared). So in addition to this patch, we should probably add a bunch of (self._manager) null checks in functions which use it.

[Blaze Burg]

Comment on attachment 351887 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351887&action=review

> Tools/TestWebKitAPI/Tests/WebKitCocoa/CloseWebViewAfterEnterFullscreen.mm:79
> +

Nit: After

[Blaze Burg]

Comment on attachment 351889 [details]
Patch

r=me

[Jer Noble]

Comment on attachment 351889 [details]
Patch

Just realized that this test will likely not succeed on iOS, since the fullscreen delegate is different there. I'll put up a new patch which runs the test only on Mac.

[Jer Noble]

Created attachment 351911 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 351911 [details]
Patch for landing

Clearing flags on attachment: 351911

Committed r236984: <https://trac.webkit.org/changeset/236984>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.