WebKitGtk crashes when there is no focused Frame

[Ori Bernstein]

Occasionally (eg, as a new tab is being opened in the background) the focus widget is NULL, but an attempt is made to access it.

Attached patch adds a check to prevent this.

[Ori Bernstein]

Created attachment 18343 [details]
Avoid crash when focused widget is NULL

Add check to avoid a crash when the focused widget is NULL.

[Alp Toker]

The Win port systematically null checks focusedFrame(). We might want to do this too, perhaps using g_return_if_fail() in some cases.

This bug also exposes an issue in focus control which should be filed separately.

[Luca Bruno]

I can't see anywhere windows doing so many checks, I've seen just two or three of those for focusing stuff.
Can you guess any other case for which the focused frame might be NULL?

[David Jaša]

Created attachment 18689 [details]
Output of gdb in such case

Webkit rev. 29753 in Midori 0.0.17 segfaults if I try to open this link in background tab:
http://www.linuxdevices.com/news/NS5877802443.html

[Alp Toker]

A similar issue was noticed in this code:


static void webkit_web_view_size_allocate(GtkWidget* widget, GtkAllocation* allocation)
{
    GTK_WIDGET_CLASS(webkit_web_view_parent_class)->size_allocate(widget,allocation);

    Frame* frame = core(webkit_web_view_get_main_frame(WEBKIT_WEB_VIEW(widget)));
    frame->view()->resize(allocation->width, allocation->height);
    frame->forceLayout();
    frame->view()->adjustViewSize();
}

This causes the crash:
    Frame* frame = core(webkit_web_view_get_main_frame(WEBKIT_WEB_VIEW(widget)));

So either get_main_frame is failing or core() is failing on the frame..

(Problem noticed when user was using Midori and WebKit r29753)

[David Jaša]

I've attached gdb output of my crash, which seems similar. I noticed that I can reproduce crash only if I open background link which leads to different site or server.

[Alp Toker]

The first step in fixing this issue consistently is understanding and validating what core() does in webkitprivate.cpp:

WebCore::Page* core(WebKitWebView* webView)
{
    if (!webView)
        return 0;

    WebKitWebViewPrivate* webViewData = WEBKIT_WEB_VIEW_GET_PRIVATE(webView);
    return webViewData ? webViewData->corePage : 0;
}

So, if webView is NULL, the return is NULL. No warning is given.
If webView is not NULL, the return can either be the corePage or NULL.

This is pretty much equivalent to what Win does:

Page* core(IWebView* iWebView)
{
    Page* page = 0;

    COMPtr<WebView> webView;
    if (SUCCEEDED(iWebView->QueryInterface(&webView)) && webView)
        page = webView->page();

    return page;
}

We need to examine the WebKit API layer code now keeping these rules in mind..

[Alp Toker]

Fix for the original issue landed in r29793. Keeping the bug open since there may be a wider issue.

[David Jaša]

Midori stopped crashing on loading pages in new tabs with svn 29807.

[Alp Toker]

Looks like this was resolved around r29793. Re-open if the issue persists.