Bug 81056 – Lion Intel Debug WebKit2 Tests crashing under [WKFullScreenWindowController _startEnterFullScreenAnimationWithDuration:]

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Jessie Berlin]

I am not sure when this started happening, but I can reproduce it locally by running all the fullscreen tests under WK2 (or just run-webkit-tests -2 LayoutTests/fullscreen/full-screen-request.html).

Process:         WebKitTestRunner [35065]
Path:            /Volumes/VOLUME/*/WebKitTestRunner
Identifier:      WebKitTestRunner
Version:         ??? (???)
Code Type:       X86-64 (Native)
Parent Process:  Python [35058]

Date/Time:       2012-03-13 14:37:13.789 -0700
OS Version:      Mac OS X 10.7.2 (11C74)
Report Version:  9

Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000000

VM Regions Near 0:
--> 
    __TEXT                 00000001090a6000-00000001090bc000 [   88K] r-x/rwx SM=COW  /Volumes/VOLUME/*

Application Specific Information:
objc[35065]: garbage collection is OFF

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   ???                           	000000000000000000 0 + 0
1   com.apple.WebKit2             	0x0000000109418380 -[WKFullScreenWindowController _startEnterFullScreenAnimationWithDuration:] + 976 (WKFullScreenWindowController.mm:473)
2   com.apple.WebKit2             	0x0000000109416e3d -[WKFullScreenWindowController beganEnterFullScreenWithInitialFrame:finalFrame:] + 317 (WKFullScreenWindowController.mm:235)
3   com.apple.WebKit2             	0x00000001092d0708 WebKit::WebFullScreenManagerProxy::beganEnterFullScreen(WebCore::IntRect const&, WebCore::IntRect const&) + 88 (WebFullScreenManagerProxyMac.mm:67)
4   com.apple.WebKit2             	0x00000001092d0e05 void CoreIPC::callMemberFunction<WebKit::WebFullScreenManagerProxy, void (WebKit::WebFullScreenManagerProxy::*)(WebCore::IntRect const&, WebCore::IntRect const&), WebCore::IntRect, WebCore::IntRect>(CoreIPC::Arguments2<WebCore::IntRect, WebCore::IntRect> const&, WebKit::WebFullScreenManagerProxy*, void (WebKit::WebFullScreenManagerProxy::*)(WebCore::IntRect const&, WebCore::IntRect const&)) + 149 (HandleMessage.h:26)
5   com.apple.WebKit2             	0x00000001092d0b93 void CoreIPC::handleMessage<Messages::WebFullScreenManagerProxy::BeganEnterFullScreen, WebKit::WebFullScreenManagerProxy, void (WebKit::WebFullScreenManagerProxy::*)(WebCore::IntRect const&, WebCore::IntRect const&)>(CoreIPC::ArgumentDecoder*, WebKit::WebFullScreenManagerProxy*, void (WebKit::WebFullScreenManagerProxy::*)(WebCore::IntRect const&, WebCore::IntRect const&)) + 115 (HandleMessage.h:297)
6   com.apple.WebKit2             	0x00000001092d0844 WebKit::WebFullScreenManagerProxy::didReceiveWebFullScreenManagerProxyMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 212 (WebFullScreenManagerProxyMessageReceiver.cpp:50)
7   com.apple.WebKit2             	0x00000001092cf971 WebKit::WebFullScreenManagerProxy::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 49 (WebFullScreenManagerProxy.cpp:60)
8   com.apple.WebKit2             	0x0000000109352080 WebKit::WebPageProxy::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 240 (WebPageProxy.cpp:1597)
9   com.apple.WebKit2             	0x00000001093ecfe6 WebKit::WebProcessProxy::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 438 (WebProcessProxy.cpp:332)
10  com.apple.WebKit2             	0x0000000109267a75 WebKit::WebConnectionToWebProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 405 (WebConnectionToWebProcess.cpp:93)
11  com.apple.WebKit2             	0x0000000109267abd non-virtual thunk to WebKit::WebConnectionToWebProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 61
12  com.apple.WebKit2             	0x000000010910419c CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::ArgumentDecoder>&) + 428 (Connection.cpp:692)
13  com.apple.WebKit2             	0x0000000109106cf3 CoreIPC::Connection::dispatchMessages() + 211 (Connection.cpp:720)
14  com.apple.WebKit2             	0x000000010910de00 WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>::operator()(CoreIPC::Connection*) + 112 (Functional.h:173)
15  com.apple.WebKit2             	0x000000010910dd85 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>, void ()(CoreIPC::Connection*)>::operator()() + 53 (Functional.h:373)
16  com.apple.WebCore             	0x000000010c75675d WTF::Function<void ()()>::operator()() const + 141 (Functional.h:581)
17  com.apple.WebCore             	0x000000010c7564e3 WebCore::RunLoop::performWork() + 147 (RunLoop.cpp:66)
18  com.apple.WebCore             	0x000000010c757900 WebCore::RunLoop::performWork(void*) + 96 (RunLoopMac.mm:65)
19  com.apple.CoreFoundation      	0x00007fff875e2b51 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
20  com.apple.CoreFoundation      	0x00007fff875e23bd __CFRunLoopDoSources0 + 253
21  com.apple.CoreFoundation      	0x00007fff876091a9 __CFRunLoopRun + 905
22  com.apple.CoreFoundation      	0x00007fff87608ae6 CFRunLoopRunSpecific + 230
23  com.apple.Foundation          	0x00007fff80f4e04f -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 267
24  WebKitTestRunner              	0x00000001090b0ddc WTR::TestController::platformRunUntil(bool&, double) + 204 (TestControllerMac.mm:60)
25  WebKitTestRunner              	0x00000001090ac6f5 WTR::TestController::runUntil(bool&, WTR::TestController::TimeoutDuration) + 149 (TestController.cpp:563)
26  WebKitTestRunner              	0x00000001090b1b64 WTR::TestInvocation::invoke() + 1396 (TestInvocation.cpp:171)
27  WebKitTestRunner              	0x00000001090acd78 WTR::TestController::runTest(char const*) + 1656 (TestController.cpp:513)
28  WebKitTestRunner              	0x00000001090ace82 WTR::TestController::runTestingServerLoop() + 178 (TestController.cpp:529)
29  WebKitTestRunner              	0x00000001090ab500 WTR::TestController::run() + 48 (TestController.cpp:537)
30  WebKitTestRunner              	0x00000001090aa1c6 WTR::TestController::TestController(int, char const**) + 614 (TestController.cpp:88)
31  WebKitTestRunner              	0x00000001090a9f53 WTR::TestController::TestController(int, char const**) + 35 (TestController.cpp:89)
32  WebKitTestRunner              	0x00000001090a7eaf main + 143 (main.mm:36)
33  WebKitTestRunner              	0x00000001090a7e14 start + 52

[Radar WebKit Bug Importer]

<rdar://problem/11042811>

[Jer Noble]

InjectedBundlePageFullScreenClient needs to be taught about beganEnterFullScreen and beganExitFullScreen, so that WebKitTestRunner can intercept these calls and not attempt to move around windows, etc.

[Jer Noble]

Created attachment 131756 [details]
Patch

Comment on attachment 131756 [details]
Patch

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

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:322
> +
> +    // Version 1:
> +    WKBundlePageBeganEnterFullScreen                                    beganEnterFullScreen;
> +    WKBundlePageBeganExitFullScreen                                     beganExitFullScreen;
>  };
>  typedef struct WKBundlePageFullScreenClient WKBundlePageFullScreenClient;
>  
> -enum { kWKBundlePageFullScreenClientCurrentVersion = 0 };
> +enum { kWKBundlePageFullScreenClientCurrentVersion = 1 };

You need to add an APIClientTraits specialization for WKBundlePageFullScreenClient now, so that the struct is properly zeroed out for version 0 clients.

[Jer Noble]

Created attachment 131767 [details]
Patch

[Build Bot]

Comment on attachment 131767 [details]
Patch

Attachment 131767 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/11948832

[Jer Noble]

Created attachment 131860 [details]
Patch

Fixed windows build failure.

[Jessie Berlin]

Comment on attachment 131860 [details]
Patch

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

r=me!

> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFullScreenClient.cpp:77
> +        m_client.beganExitFullScreen(toAPI(page), toAPI(initialFrame), toAPI(finalFrame));

I think you meant

m_client.beganEnterFullScreen( ....

here, not beganExitFullScreen.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:1118
> +void InjectedBundlePage::beganEnterFullScreen(WKBundlePageRef pageRef, WKRect initialFrame, WKRect finalFrame)

Odd that you aren't seeing any compiler warnings / errors about unused parameters. You should remove the names for the parameters here.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:1124
> +void InjectedBundlePage::beganExitFullScreen(WKBundlePageRef pageRef, WKRect initialFrame, WKRect finalFrame)

Ditto.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:1127
> +        InjectedBundle::shared().os() << "beganExitFullScreen()\n";

Do you know if there are any tests that depend on beganEnterFullScreen/beganExitFullScreen being in the output? If so, are there expected failing results checked in for them for WK2 that would need to be updated now that we are logging it?

[Jer Noble]

Comment on attachment 131860 [details]
Patch

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

>> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFullScreenClient.cpp:77
>> +        m_client.beganExitFullScreen(toAPI(page), toAPI(initialFrame), toAPI(finalFrame));
> 
> I think you meant
> 
> m_client.beganEnterFullScreen( ....
> 
> here, not beganExitFullScreen.

Whoops!  Changed.

>> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:1118
>> +void InjectedBundlePage::beganEnterFullScreen(WKBundlePageRef pageRef, WKRect initialFrame, WKRect finalFrame)
> 
> Odd that you aren't seeing any compiler warnings / errors about unused parameters. You should remove the names for the parameters here.

Odd indeed.  Odder still that I thought I removed them.  Though the other functions in the file have the same problem.  I'll remove them anyways.

>> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:1127
>> +        InjectedBundle::shared().os() << "beganExitFullScreen()\n";
> 
> Do you know if there are any tests that depend on beganEnterFullScreen/beganExitFullScreen being in the output? If so, are there expected failing results checked in for them for WK2 that would need to be updated now that we are logging it?

No, none of the tests seem to enable dumping.  And, while there are a couple of failing tests when running "run-webkit-tests -2", none are due to logging.

[Jer Noble]

Committed r110708: <http://trac.webkit.org/changeset/110708>

[Alexey Proskuryakov]

> Odd indeed.  Odder still that I thought I removed them.  Though the other functions in the file have the same problem.  I'll remove them anyways.

Sam once told me that some of WebKit2 code has argument names left intentionally. Not sure if that's one of those cases.

[Jessie Berlin]

(In reply to comment #11)
> > Odd indeed.  Odder still that I thought I removed them.  Though the other functions in the file have the same problem.  I'll remove them anyways.
> 
> Sam once told me that some of WebKit2 code has argument names left intentionally. Not sure if that's one of those cases.

Maybe he was referring to leaving the argument names in the WK API header files, even if they are obvious? He has told me that a couple of times.