Bug 81056 - Lion Intel Debug WebKit2 Tests crashing under [WKFullScreenWindowController _startEnterFullScreenAnimationWithDuration:]
Summary: Lion Intel Debug WebKit2 Tests crashing under [WKFullScreenWindowController _...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL: http://build.webkit.org/results/Lion%...
Keywords: InRadar, LayoutTestFailure, MakingBotsRed, Regression
Depends on:
Blocks:
 
Reported: 2012-03-13 16:33 PDT by Jessie Berlin
Modified: 2012-03-14 09:38 PDT (History)
5 users (show)

See Also:


Attachments
Patch (9.15 KB, patch)
2012-03-13 17:16 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (11.49 KB, patch)
2012-03-13 18:31 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (12.42 KB, patch)
2012-03-14 08:43 PDT, Jer Noble
jberlin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jessie Berlin 2012-03-13 16:33:43 PDT
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
Comment 1 Radar WebKit Bug Importer 2012-03-13 16:34:02 PDT
<rdar://problem/11042811>
Comment 2 Jer Noble 2012-03-13 17:10:22 PDT
InjectedBundlePageFullScreenClient needs to be taught about beganEnterFullScreen and beganExitFullScreen, so that WebKitTestRunner can intercept these calls and not attempt to move around windows, etc.
Comment 3 Jer Noble 2012-03-13 17:16:33 PDT
Created attachment 131756 [details]
Patch
Comment 4 mitz 2012-03-13 17:25:17 PDT
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.
Comment 5 Jer Noble 2012-03-13 18:31:00 PDT
Created attachment 131767 [details]
Patch
Comment 6 Build Bot 2012-03-13 18:54:29 PDT
Comment on attachment 131767 [details]
Patch

Attachment 131767 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/11948832
Comment 7 Jer Noble 2012-03-14 08:43:35 PDT
Created attachment 131860 [details]
Patch

Fixed windows build failure.
Comment 8 Jessie Berlin 2012-03-14 09:03:24 PDT
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?
Comment 9 Jer Noble 2012-03-14 09:10:45 PDT
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.
Comment 10 Jer Noble 2012-03-14 09:21:36 PDT
Committed r110708: <http://trac.webkit.org/changeset/110708>
Comment 11 Alexey Proskuryakov 2012-03-14 09:25:06 PDT
> 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.
Comment 12 Jessie Berlin 2012-03-14 09:38:27 PDT
(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.