[Webkit-unassigned] [Bug 150927] Inconsistencies in main resource load delegates when loading from history

[bugzilla-daemon at webkit.org]

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

--- Comment #25 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #23)
> It's still my understanding that the delivery order is wrong, but red
> herring, and that the real issue is that some level of code doesn't
> understand what provisional load vs. committed load means. Sorry, I will not
> have the time to look into this deeper.

I'll try to start from scratch again, please forget about the TLS certificate issues, because that's a problem of our workaround in the UI process to solve this issue, but not the problem itself.

When a load starts the way load delegates are called is usually the following one:

1 - didStartProvisionalLoadForFrame
2 - willSendRequest (main resource)
3 - didReceiveResponse (main resource)
4 - didCommitLoadForFrame
didReceiveData (main resource)
other resource start loading as well
didFinishLoading (main resource)
didFinishLoadForFrame

I stopped using numbers after step 4 because the order can change from that step on, but not before, because the load is never committed until the main resource has started loading and a response has received from the server. At that point the load is no longer provisional and it's committed. Because of this fact, our public API claims that you can call webkit_web_view_get_main_resource() once the load has been committed and you will get an object, and not only that but also that you can call webkit_web_resource_get_response() to get the response. What triggered this investigation (see bugs #91482 and #91478) was that we were getting null pointers in same cases. Then we realized that it was only when loading from history, and I tried to fix it on our side instead of fixing the real issue as I'm doing now. My attempts to workaround the issue ended up being hack after hack, that never actually fixed the issue, but made the code a lot more complex and fragile. 

What happens when loading from the history cache *and the page cache is enabled*, is the following:

1 - didStartProvisionalLoadForFrame
2 - didCommitLoadForFrame
3 - willSendRequest (main resource)
4 - didReceiveResponse (main resource)
5 - didReceiveData (main resource)
6 - didFinishLoading (main resource)
other resource start loading as well
didFinishLoadForFrame

So, when the load is committed there's no main resource from the API point of view and of course there isn't a response either. When restoring from page cache, there's no actual load, but we still call all the load delegates, because this should be transparent from the client point of view, but it's not actually transparent, because of these inconsistencies. So, what this patch does is ensuring that willSendRequest and didReceiveResponse for the main resource are called before didCommitLoadForFrame. I don't see how this can break existing apps, unless there are applications having different ways to handle load delegates when the page is loaded from the page cache.

I don't know what else I can do to unblock this. The safest way as I said would be add platform ifdefs, but I wouldn't like it at all. We won't know if this actually breaks anything until we try, and we can just roll it out in such case.

-- 
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/20151121/faa277ca/attachment.html>