GNOME Bugzilla – Bug 662847
Make pause/unpause work in any state broke one WebKitGtk+ unit test
Last modified: 2011-10-28 14:34:16 UTC
The test is called test_web_resource_loading and can be seen here http://svn.webkit.org/repository/webkit/trunk/Source/WebKit/gtk/tests/testwebresource.c. The problem is that the message queue is not empty at the end of that test. The reason why it is not empty is that WebKit treats "about:blank" in a very special way. If it detects that we're trying to load "about:blank" at content sniffing time then WebKit just does not wait for the SoupMessage to be finished and reports that the load is completed. That leads to a code path that closes the stream before the message finalization and thus, the SoupMessage is left in paused state forever.
Created attachment 200101 [details] [review] Unpause before cancelling This patch basically unpauses the SoupMessage before cancelling it if we try to close the stream when the message is still not finished. This way, we give the SoupMessageQueue a chance to gracefully complete the SoupMessage processing.
Created attachment 200124 [details] [review] SoupHTTPInputStream: fix error on cancellation The message could get left behind still queued in the session if the stream was closed before it finished. Fix that. === does this fix the test as well? It's part of a patch I have on a branch where I'm playing around with changing how restartable requests (eg, redirection, authentication) are handled to try to simplify ResourceHandleSoup
(In reply to comment #2) > Created an attachment (id=200124) [details] [review] > SoupHTTPInputStream: fix error on cancellation > > The message could get left behind still queued in the session if the > stream was closed before it finished. Fix that. I don't get this. My patch precisely unpauses the message when we close the stream before it finishes. Maybe I am not understanding it correctly. > does this fix the test as well? It's part of a patch I have on a > branch where I'm playing around with changing how restartable requests > (eg, redirection, authentication) are handled to try to simplify > ResourceHandleSoup Will try.
(In reply to comment #3) > (In reply to comment #2) > > Created an attachment (id=200124) [details] [review] [details] [review] > > SoupHTTPInputStream: fix error on cancellation > > > > The message could get left behind still queued in the session if the > > stream was closed before it finished. Fix that. > > I don't get this. My patch precisely unpauses the message when we close the > stream before it finishes. Maybe I am not understanding it correctly. Oh sorry, I didn't realize that it was just your commit message. > > does this fix the test as well? It's part of a patch I have on a > > branch where I'm playing around with changing how restartable requests > > (eg, redirection, authentication) are handled to try to simplify > > ResourceHandleSoup > > Will try. Yours indeed fixes the test as well, although mine does not require the priv->queued thing and it is not that sensible to potential reference leaks I'd say.
Comment on attachment 200101 [details] [review] Unpause before cancelling Yeah, you're right, doing it from close() is better than doing it from finalize(). And close() already also cancels the message if needed, so we don't need that part of my patch either.
Committed 2ec9af5efab3410e40dc84ca37264b58878727e3