Fix adoptRef assertion failures caused by stack-allocated ResourceHandle objects
Created attachment 60818 [details] Patch
Comment on attachment 60818 [details] Patch Anders reviewed this.
Committed r62736: <http://trac.webkit.org/changeset/62736>
http://trac.webkit.org/changeset/62736 might have broken GTK Linux 32-bit Release
The failure reported by the bot seems to be indeed caused by this change. These two tests started timing out with the change, and reverting the change locally make them work again: http/tests/xmlhttprequest/simple-cross-origin-denied-events-post-sync.html http/tests/xmlhttprequest/simple-cross-origin-denied-events-sync.html
Uh oh. Any ideas for fixing that?
(In reply to comment #6) > Uh oh. Any ideas for fixing that? Aren't you just never calling .start() in the handle after the patch?
(In reply to comment #7) > Aren't you just never calling .start() in the handle after the patch? I believe the synchronous loading function now calls create(), which calls start(). So that should work.
(In reply to comment #8) > (In reply to comment #7) > > Aren't you just never calling .start() in the handle after the patch? > > I believe the synchronous loading function now calls create(), which calls start(). So that should work. I see, you are right. I guess the explanation is that it's failing in the previous checks in the create function now, otherwise the code seems identical.
(In reply to comment #9) > I see, you are right. I guess the explanation is that it's failing in the previous checks in the create function now, otherwise the code seems identical. I guess instead of calling create we should just explicitly do adoptRef(new) and start() to keep things working for now, and consider adding a FIXME saying we should use create. Xan, do you want to use a new bug to track that change? Do you want me to do it, or would you do it?
(In reply to comment #10) > (In reply to comment #9) > > I see, you are right. I guess the explanation is that it's failing in the previous checks in the create function now, otherwise the code seems identical. > > I guess instead of calling create we should just explicitly do adoptRef(new) and start() to keep things working for now, and consider adding a FIXME saying we should use create. Xan, do you want to use a new bug to track that change? Do you want me to do it, or would you do it? Let me do the patch and test it here, I'll attach it in the bug and you can review it.
Created attachment 60927 [details] resourcehandle.diff Sorry for the delay, a World recompile kicked in. This fixes things locally for me.
Comment on attachment 60927 [details] resourcehandle.diff WebCore/platform/network/soup/ResourceHandleSoup.cpp:665 + handle.get()->start(frame); No need for get() here.
Comment on attachment 60927 [details] resourcehandle.diff Landed in r62832 with the suggested fix.