[Webkit-unassigned] [Bug 144262] [GTK] Crash in WebProcess when loading large content with custom URI schemes

[bugzilla-daemon at webkit.org]

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

--- Comment #10 from Mario Sanchez Prada <mario at webkit.org> ---
(In reply to comment #9)
> [...]
> > I think we shouldn't even call data->releaseTask() when we have a stream. I would use an early return instead, something like:
> > 
> > if (!data->stream) {
> >     // Failed while reading the stream, the task was already completed by didLoadData().
> >     return;
> > }
> > 
> > GRefPtr<GTask> task = data->releaseTask();
> > ASSERT(task.get());
> > g_task_return_new_error....
> 
> That was my initial version, but I was unsure about doing it and in the end
> preferred to do it always and assert task or !task depending on the case.
> 
> Anyway, I will change it in the next patch

Just a quick heads-up after a conversation on IRC with Carlos: we can't really early return because we still need to remove the data from the m_customProtocolMap hash table, and also, perhaps ASSERT() task and !task depending on the case would not be a too bad thing either... so we kind of settled on the following code for didFailError():

    void CustomProtocolManagerImpl::didFailWithError(uint64_t customProtocolID, const WebCore::ResourceError& error)
    {
        WebSoupRequestAsyncData* data = m_customProtocolMap.get(customProtocolID);
        ASSERT(data);

        // Either we haven't started reading the stream yet, in which case we need to complete the
        // task first, or we failed reading it and the task was already completed by didLoadData().
        ASSERT(!data->stream || !data->task);

        if (!data->stream) {
            GRefPtr<GTask> task = data->releaseTask();
            ASSERT(task.get());
            g_task_return_new_error(task.get(), g_quark_from_string(error.domain().utf8().data()),
                error.errorCode(), "%s", error.localizedDescription().utf8().data());
        }

        m_customProtocolMap.remove(customProtocolID);
    }

Still, I need to write the unit test in a proper way, but I thought I would share this before, because previous comments could be confusing.

-- 
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/20150429/1a3c9d9e/attachment-0001.html>