Bug 16795 - WebKitGtk crashes when there is no focused Frame
Summary: WebKitGtk crashes when there is no focused Frame
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Major
Assignee: Nobody
Keywords: Gtk
Depends on:
Reported: 2008-01-08 20:43 PST by Ori Bernstein
Modified: 2008-02-18 17:24 PST (History)
4 users (show)

See Also:

Avoid crash when focused widget is NULL (693 bytes, patch)
2008-01-08 20:49 PST, Ori Bernstein
no flags Details | Formatted Diff | Diff
Output of gdb in such case (3.56 KB, text/plain)
2008-01-25 06:21 PST, David Jaša
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ori Bernstein 2008-01-08 20:43:49 PST
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.
Comment 1 Ori Bernstein 2008-01-08 20:49:18 PST
Created attachment 18343 [details]
Avoid crash when focused widget is NULL

Add check to avoid a crash when the focused widget is NULL.
Comment 2 Alp Toker 2008-01-08 20:54:04 PST
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.
Comment 3 Luca Bruno 2008-01-09 01:51:52 PST
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?
Comment 4 David Jaša 2008-01-25 06:21:21 PST
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:
Comment 5 Alp Toker 2008-01-25 06:25:23 PST
A similar issue was noticed in this code:

static void webkit_web_view_size_allocate(GtkWidget* widget, GtkAllocation* allocation)

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

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)
Comment 6 David Jaša 2008-01-25 06:33:39 PST
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.
Comment 7 Alp Toker 2008-01-25 09:34:49 PST
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..
Comment 8 Alp Toker 2008-01-25 10:33:59 PST
Fix for the original issue landed in r29793. Keeping the bug open since there may be a wider issue.
Comment 9 David Jaša 2008-01-27 09:54:36 PST
Midori stopped crashing on loading pages in new tabs with svn 29807.
Comment 10 Alp Toker 2008-02-18 17:24:50 PST
Looks like this was resolved around r29793. Re-open if the issue persists.