Bug 190098

Summary:

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

Product:

WebKit

Reporter:

Blaze Burg <bburg>

Component:

Media

Assignee:

Jer Noble <jer.noble>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, commit-queue, jeremyj-wk, jer.noble, joepeck, simon.fraser, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch for landing	none

Blaze Burg

Reported 2018-09-28 16:23:48 PDT

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.

Attachments
Patch (10.76 KB, patch) 2018-10-09 10:24 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Patch (10.76 KB, patch) 2018-10-09 10:39 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Patch for landing (10.77 KB, patch) 2018-10-09 14:07 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Blaze Burg

Comment 1 2018-09-28 16:24:34 PDT

<rdar://problem/42822671>

Jer Noble

Comment 2 2018-10-09 10:24:44 PDT

Created attachment 351887 [details] Patch

Simon Fraser (smfr)

Comment 3 2018-10-09 10:33:44 PDT

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

Comment 4 2018-10-09 10:39:55 PDT

Created attachment 351889 [details] Patch

Jer Noble

Comment 5 2018-10-09 10:43:11 PDT

(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 6 2018-10-09 11:59:28 PDT

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 7 2018-10-09 13:31:01 PDT

Comment on attachment 351889 [details] Patch r=me

Jer Noble

Comment 8 2018-10-09 14:02:05 PDT

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

Comment 9 2018-10-09 14:07:23 PDT

Created attachment 351911 [details] Patch for landing

WebKit Commit Bot

Comment 10 2018-10-09 14:45:23 PDT

Comment on attachment 351911 [details] Patch for landing Clearing flags on attachment: 351911 Committed r236984: <https://trac.webkit.org/changeset/236984>

WebKit Commit Bot

Comment 11 2018-10-09 14:45:25 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.