Summary: | Pending resource requests not always stopped when new request is loaded | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Jalkut <jalkut> | ||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | NEW --- | ||||||||
Severity: | Normal | CC: | beidson, emacemac7, koivisto | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Daniel Jalkut
2008-07-16 22:26:20 PDT
Created attachment 22331 [details]
Possible patch which needs careful review by somebody experienced with the loader.
I am not so much providing this patch as a proposed fix, but more of an example of one possibly crude way of addressing the problem. I'm hoping somebody with experience will see to deciding whether this or another solution would be most appropriate.
Thanks for your time!
Created attachment 22332 [details]
This example project reproduces the bug scenario on my machine
Note that the sample project reproduces the bug on my machine, but may require coercing to get it to exactly cause the bug on your machine. All the same, it demonstrates the type of scenario (albeit in a contrived way) that could lead to the type of bad timing that causes the bug to rear its head.
Note: I have searched for a workaround to this problem. I hoped that calling "stopLoading" on the affected frame would cause "the good stop" to get reached, but it doesn't (it uses the same faulty "stopAllLoaders" method). So far the best workaround I've come across is, when I need to fully reload a request in the web frame, to literally throw away the existing WebView and start from scratch, replacing the old WebView in the view hierarchy, and then loadHTMLString to start fresh. This seems to be the only way to guarantee that the stale broken-image CachedResource objects don't get used perpetually by my WebFrame. If anybody has a better workaround to suggest I'd be glad to hear it. Daniel Comment on attachment 22331 [details]
Possible patch which needs careful review by somebody experienced with the loader.
Bradee should review this
Just wanted to check in with Bradee about the patch... ? Comment on attachment 22331 [details]
Possible patch which needs careful review by somebody experienced with the loader.
I'd really like Anders to take a look at this one when he gets back... this one line change has so much potential to be... very complicated.
I looked at this patch, and I think I'll need to read a bunch of loader code to figure out if it's correct. Or get Brady to explain in person. Comment on attachment 22331 [details]
Possible patch which needs careful review by somebody experienced with the loader.
r- for the following:
1) The comment is too long.
2) No ChangeLog entry.
3) No test case.
4) After review of the code, I am not sure if true or false is right. The difference is that stopLoading(true) will send an unload event, while false will not. The best way to check which is right is to check if other browsers send an unload even in this situation.
Conceptually, it seems right to stop loading here, just these details need to be worked out.
Maciej - thanks for reviewing the code. I apologize for the long comment. It probably would have been better-suited to the bug report, but I wanted to make sure nobody assumed that the code should be integrated without careful consideration. Thank you for taking a careful look at it. I am not an experienced WebKit contributor, so I did not know enough to include a test case or a change log entry. I just submitted the bug because I thought it might be an important problem to address for the long term health of the project. It would be nice if there was a clear means of submitting bugs that did not require meeting the same test that "patch submissions" normally have to meet. Maybe this is asking for too much review work on the WebKit team's behalf. But it seems as though my contribution here is judged by WebKit patch standards, when it was really meant more as a glorified (hopefully useful) bug report. Daniel |