[Webkit-unassigned] [Bug 144508] [GTK] TestWebKitWebContext is crashing on debug builds

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=144508

--- Comment #1 from Mario Sanchez Prada <mario at webkit.org> ---
I debugged this extensively today, and found out that the problem seems to be that the hashtable holding the destruction observers for the Frame is being prematurely cleared due to a problem initiated when WebFrameLoaderClient::dispatchDidFinishDocumentLoad() is run, at the time it tries to notify the InjectedBundle:

    void WebFrameLoaderClient::dispatchDidFinishDocumentLoad()
    {
        [...]

        // Notify the bundle client.
        webPage->injectedBundleLoaderClient().didFinishDocumentLoadForFrame(webPage, m_frame, userData);

        [...]
    }

This bit will end up getting the following callback in Tools/TestWebKitAPI/Tests/WebKit2Gtk/WebExtensionTest.cpp executed:

    static void documentLoadedCallback(WebKitWebPage* webPage, WebKitWebExtension* extension)
    {
        // FIXME: Too much code just to send a message, we need convenient custom API for this.
        WebKitDOMDocument* document = webkit_web_page_get_dom_document(webPage);
        GRefPtr<WebKitDOMDOMWindow> window = adoptGRef(webkit_dom_document_get_default_view(document));
        if (WebKitDOMWebKitNamespace* webkit = webkit_dom_dom_window_get_webkit_namespace(window.get())) {
            WebKitDOMUserMessageHandlersNamespace* messageHandlers = webkit_dom_webkit_namespace_get_message_handlers(webkit);
            if (WebKitDOMUserMessageHandler* handler = webkit_dom_user_message_handlers_namespace_get_handler(messageHandlers, "dom"))
                webkit_dom_user_message_handler_post_message(handler, "DocumentLoaded");
        }

        webkit_dom_dom_window_webkit_message_handlers_post_message(window.get(), "dom-convenience", "DocumentLoaded");

        gpointer data = g_object_get_data(G_OBJECT(extension), "dbus-connection");
        if (data)
            emitDocumentLoaded(G_DBUS_CONNECTION(data));
        else
            delayedSignalsQueue.append(DelayedSignal(DocumentLoadedSignal));
    }


...and this seems to be the problem: before r180214 [1], 'window' was just a raw pointer obtained as this:

    GRefPtr<WebKitDOMDOMWindow> window = adoptGRef(webkit_dom_document_get_default_view(document));


However, since r180214 it's now put in a GRefPtr<WebKitDOMDOMWindow>, meaning that its refcount will get decreased as soon as we live the scope of this function. And because we are using adoptGRef() here, that means that 'window' won't increase the refcount at all when getting the value from webkit_dom_document_get_default_view(), effectively being 1 at the time this callback finishes, causing the destruction of this 'window' object.

And this would be probably ok if 'window' was not cached inside the DOMCache, but the truth is that it is cached (automatically on construction the first time time it's requested via webkit_dom_document_get_default_view()). So, we can't simply steal the reference from the cache and unref it freely afterwards or we will get the crash we are seeing.

So, I believe we either drop the adoptGRef() usage and prevent the refcount of 'window' from getting to zero or, even simpler, we stop using GRefPtr<> and use a raw pointer as it used to be the case before r180214, which seems also to be the case everywhere else but here (which looks like a mistake):

    $ git grep webkit_dom_document_get_default_view
    Tests/WebKit2Gtk/DOMDOMWindowTest.cpp:        WebKitDOMDOMWindow* domWindow = webkit_dom_document_get_default_view(document);
    Tests/WebKit2Gtk/DOMDOMWindowTest.cpp:        WebKitDOMDOMWindow* domWindow = webkit_dom_document_get_default_view(document);
    Tests/WebKit2Gtk/DOMDOMWindowTest.cpp:        WebKitDOMDOMWindow* domWindow = webkit_dom_document_get_default_view(document);
    Tests/WebKit2Gtk/WebExtensionTest.cpp:    GRefPtr<WebKitDOMDOMWindow> window = adoptGRef(webkit_dom_document_get_default_view(document));

Doing either way will get the crash gone.

[1] http://trac.webkit.org/changeset/180214

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150501/f1f6dce2/attachment-0001.html>