REGRESSION (r141136): Wiki "Random article" function very broken When main resources are in the memory cache, they are keyed off http://en.wikipedia.org/wiki/Special:Random Even though http://en.wikipedia.org/wiki/Special:Random redirects to a random article each time. So we start to revalidate the resource with wikipedia, and it redirects the revalidation request. In our new redirect request, we still issue a conditional GET to try to revalidate the new request, and that gives us back a 304. So we update our URLs but use the main resource text from the previous article. If the redirect URL differs from the CachedResource we're trying to revalidate, we should stop trying to revalidate and issue an unconditional GET. In radar as <rdar://problem/13229985>
Created attachment 197910 [details] Patch v1 Still making sure I didn't break any other layout tests, but this is my plan.
Comment on attachment 197910 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=197910&action=review Does SubresourceLoader cover all the resources that may be affected? > Source/WebCore/loader/SubresourceLoader.cpp:137 > + // If this redirect takes us somewhere besides the final response URL of the resource we're revalidating, > + // then make this an unconditional GET request. I think that this "what" comment adds some value, but a "why" one would be even better.
(In reply to comment #2) > (From update of attachment 197910 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197910&action=review > > Does SubresourceLoader cover all the resources that may be affected? SubresourceLoader is the only thing that revalidates CachedResources, so it's an appropriate catch-all for accidentally reusing previously loaded resources. > > Source/WebCore/loader/SubresourceLoader.cpp:137 > > + // If this redirect takes us somewhere besides the final response URL of the resource we're revalidating, > > + // then make this an unconditional GET request. > > I think that this "what" comment adds some value, but a "why" one would be even better. I can definitely improve this.
EWS never churned all the way through this, and running tests locally is proving to be a hit-or-miss PITA. I need to move forward with this one and watch the actual testers afterwards.
http://trac.webkit.org/changeset/148929
(In reply to comment #3) > (In reply to comment #2) > > > Source/WebCore/loader/SubresourceLoader.cpp:137 > > > + // If this redirect takes us somewhere besides the final response URL of the resource we're revalidating, > > > + // then make this an unconditional GET request. > > > > I think that this "what" comment adds some value, but a "why" one would be even better. > > I can definitely improve this. Since I'd worked on other things for ~9 days, I forgot to address this comment. My apologies. Will fix in a followup
Followed up in http://trac.webkit.org/changeset/148932