[webkit-reviews] review granted: [Bug 81056] Lion Intel Debug WebKit2 Tests crashing under [WKFullScreenWindowController _startEnterFullScreenAnimationWithDuration:] : [Attachment 131860] Patch

[bugzilla-daemon at webkit.org]

Jessie Berlin <jberlin at webkit.org> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 81056: Lion Intel Debug WebKit2 Tests crashing under
[WKFullScreenWindowController _startEnterFullScreenAnimationWithDuration:]
https://bugs.webkit.org/show_bug.cgi?id=81056

Attachment 131860: Patch
https://bugs.webkit.org/attachment.cgi?id=131860&action=review

------- Additional Comments from Jessie Berlin <jberlin at webkit.org>
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?