REOPENED94644
Failure to dispatch delegate callbacks if resource load fails synchronously
https://bugs.webkit.org/show_bug.cgi?id=94644
Summary Failure to dispatch delegate callbacks if resource load fails synchronously
Pratik Solanki
Reported 2012-08-21 16:17:18 PDT
This is similar to bug 91018, but instead of special casing Font resources, we should generalize it so that this can't happen for other resource load fails.
Attachments
Patch (3.52 KB, patch)
2012-08-21 16:27 PDT, Pratik Solanki
koivisto: review+
Pratik Solanki
Comment 1 2012-08-21 16:17:54 PDT
Pratik Solanki
Comment 2 2012-08-21 16:27:35 PDT
Antti Koivisto
Comment 3 2012-08-21 22:47:07 PDT
Comment on attachment 159793 [details] Patch r=me. A test case would be cool.
Pratik Solanki
Comment 4 2012-08-22 11:44:25 PDT
Kenneth Russell
Comment 5 2012-08-22 17:34:27 PDT
I'm afraid this introduced a subtle regression that, bizarrely enough, only showed up in Chromium Mac debug builds with the layout test svg/custom/linking-uri-01-b.svg . The failure is 100% reproducible on my Mac Pro with a debug build of Chromium's DumpRenderTree, and reverting this patch locally fixes the regression. The bug is that with this test case, the onload handler isn't being called at the right time, so the ellipse isn't zoomed to fill the view. I wasn't able to reproduce the failure on Linux and it looks like the Apple bots aren't affected either. I'm concerned that this indicates a change in behavior that will affect real web sites and therefore intend to roll out this patch. Pratik, I'll try to reach you on IRC before doing so. First failing builds: http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.6%20%28dbg%29/builds/12059 http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.7%20%28dbg%29/builds/54 Flakiness dashboard: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=svg%2Fcustom%2Flinking-uri-01-b.svg
Kenneth Russell
Comment 6 2012-08-22 17:46:26 PDT
Adding a couple more people who know the loader code. Changes in behavior in this area are really scary, so unless I can reach Pratik soon, I'll roll this out. Haven't had any luck yet identifying new layout test failures on the Apple Mac debug bots caused by this change, but still looking.
Kenneth Russell
Comment 7 2012-08-22 18:04:27 PDT
Pratik and I talked on IRC and he agreed that it would be OK to roll out this patch. I spent some time going through the Apple Mac test bots on build.webkit.org around the revision where this landed. The only evidence I could find that something might be going wrong on other ports is that the following bot started getting an elevated crash rate at r126325 (2x the previous crash rate, from 12 to 24) -- and it has a build (#2757) where only that revision was incorporated: http://build.webkit.org/builders/Apple%20Lion%20Debug%20WK2%20%28Tests%29?numbuilds=25 Again, though, this change definitely affected the timing of the calling of the onload hander in the SVG layout test above in Chromium Mac Debug builds. I'm available to help test revisions to the patch.
Kenneth Russell
Comment 8 2012-08-22 18:06:59 PDT
Reverted r126325 for reason: Caused subtle but reproducible failure to call onload handler properly in an SVG layout test in Chromium Mac Debug builds, indicating potentially larger problem Committed r126373: <http://trac.webkit.org/changeset/126373>
Note You need to log in before you can comment on or make changes to this bug.