Bug 116253 – Replace WebFrameLoaderClient static_casts with a function that might return null

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

[Brady Eidson]

Replace WebFrameLoaderClient static_casts with a function that might return null

Spun off from https://bugs.webkit.org/show_bug.cgi?id=115917

This bug is about WebKit2.  We should handle WebKit1 separately.

[Brady Eidson]

Created attachment 202006 [details]
Patch v1

[Brady Eidson]

Somethings clearly off in the weeds on the EWS bot.  Landing soon.

[Brady Eidson]

http://trac.webkit.org/changeset/150282

[Andreas Kling]

Comment on attachment 202006 [details]
Patch v1 

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

> Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:182
>  void WebChromeClient::focusedFrameChanged(Frame* frame)
>  {
> -    WebFrame* webFrame = frame ? static_cast<WebFrameLoaderClient*>(frame->loader()->client())->webFrame() : 0;
> +    WebFrameLoaderClient* webFrameLoaderClient = toWebFrameLoaderClient(frame->loader()->client());
> +    WebFrame* webFrame = webFrameLoaderClient ? webFrameLoaderClient->webFrame() : 0;

This function no longer null-checks the 'frame' argument.

Just got this crash on ToT:
0   com.apple.WebKit2             	0x000000010e7e1e0e WebKit::WebChromeClient::focusedFrameChanged(WebCore::Frame*) + 18 (FrameLoader.h:197)
1   com.apple.WebCore             	0x000000010f5d4dce WebCore::FocusController::setFocusedFrame(WTF::PassRefPtr<WebCore::Frame>) + 590 (FocusController.cpp:201)
2   com.apple.WebCore             	0x000000010f5f6b8d WebCore::Frame::willDetachPage() + 221 (PassRefPtr.h:67)
3   com.apple.WebCore             	0x000000010f6032c7 WebCore::FrameLoader::detachFromParent() + 471 (FrameLoader.cpp:2402)
4   com.apple.WebKit2             	0x000000010e82ada3 WebKit::WebPage::close() + 393 (WebPage.cpp:855)

[Brady Eidson]

(In reply to comment #4)
> (From update of attachment 202006 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=202006&action=review
> 
> > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:182
> >  void WebChromeClient::focusedFrameChanged(Frame* frame)
> >  {
> > -    WebFrame* webFrame = frame ? static_cast<WebFrameLoaderClient*>(frame->loader()->client())->webFrame() : 0;
> > +    WebFrameLoaderClient* webFrameLoaderClient = toWebFrameLoaderClient(frame->loader()->client());
> > +    WebFrame* webFrame = webFrameLoaderClient ? webFrameLoaderClient->webFrame() : 0;
> 
> This function no longer null-checks the 'frame' argument.
> 
> Just got this crash on ToT:
> 0   com.apple.WebKit2                 0x000000010e7e1e0e WebKit::WebChromeClient::focusedFrameChanged(WebCore::Frame*) + 18 (FrameLoader.h:197)
> 1   com.apple.WebCore                 0x000000010f5d4dce WebCore::FocusController::setFocusedFrame(WTF::PassRefPtr<WebCore::Frame>) + 590 (FocusController.cpp:201)
> 2   com.apple.WebCore                 0x000000010f5f6b8d WebCore::Frame::willDetachPage() + 221 (PassRefPtr.h:67)
> 3   com.apple.WebCore                 0x000000010f6032c7 WebCore::FrameLoader::detachFromParent() + 471 (FrameLoader.cpp:2402)
> 4   com.apple.WebKit2                 0x000000010e82ada3 WebKit::WebPage::close() + 393 (WebPage.cpp:855)

Yikes.

Fixed in http://trac.webkit.org/changeset/150288