With bug 49246 landed, MainResourceLoader's purpose is no longer clearly defined, and is poorly named. It isn't a ResourceLoader subclass (it implements the CachedRawResourceClient interface), and the organization of what DocumentLoader and MainResourceLoader each do is history rather than logical. It's probably worth merging them into a single class.
Created attachment 188173 [details] Monolithic version This monster patch is my current thought for merging these 2 classes. I'll split it up before asking for review, just posting to get EWS data.
Comment on attachment 188173 [details] Monolithic version Attachment 188173 [details] did not pass cr-linux-debug-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16514332
Comment on attachment 188173 [details] Monolithic version Attachment 188173 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16536321
Comment on attachment 188173 [details] Monolithic version Attachment 188173 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/16514438
Created attachment 193949 [details] Finish the merger
Comment on attachment 193949 [details] Finish the merger View in context: https://bugs.webkit.org/attachment.cgi?id=193949&action=review Amazing. > Source/WebCore/loader/DocumentLoader.cpp:1390 > + DEFINE_STATIC_LOCAL(ResourceLoaderOptions, mainResourceLoadOptions, > + (SendCallbacks, SniffContent, BufferData, AllowStoredCredentials, AskClientForCrossOriginCredentials, SkipSecurityCheck)); I'm surprised we use a static here. This is just a struct...
Comment on attachment 193949 [details] Finish the merger Clearing flags on attachment: 193949 Committed r146301: <http://trac.webkit.org/changeset/146301>
All reviewed patches have been landed. Closing bug.
It appears that this patch broke Windows (Debug) Tests: http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50526 http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50527
To make sure this is a real regression, I've ran the tests at the same revision on two distinct machines (apple-windows-3 and apple-windows-4 respectively): http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50528 http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50529
Re-opened since this is blocked by bug 112776
(In reply to comment #10) > To make sure this is a real regression, I've ran the tests at the same revision on two distinct machines (apple-windows-3 and apple-windows-4 respectively): > http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50528 > http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50529 I'll also note that I've rebooted those bots prior to running the tests.
It appears that this patch caused assertions to be hit on almost all tests: animations/additive-transform-animations.html animations/animation-add-events-in-handler.html animations/animation-border-overflow.html animations/animation-controller-drt-api.html animations/animation-css-rule-types.html animations/animation-delay-changed.html animations/animation-direction-alternate-reverse.html animations/animation-direction-reverse-fill-mode-hardware.html animations/animation-direction-reverse-fill-mode.html animations/animation-direction-reverse-hardware-opacity.html animations/animation-direction-reverse-hardware.html animations/animation-direction-reverse-non-hardware.html animations/animation-direction-reverse-timing-functions-hardware.html animations/animation-direction-reverse-timing-functions.html animations/animation-direction-reverse.html animations/animation-direction.html animations/animation-end-event-destroy-renderer.html animations/animation-end-event-short-iterations.html animations/animation-events-create.html animations/animation-hit-test-transform.html
I'm sorry but I'm rolling out the patch for now as we're losing the entire test coverage on Windows Debug.
Thanks.
It appears that this was a build corruption issue. http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50532 is the run that contains the rollout and this looks better but still a bunch of tests have failed. I've triggered a clean build after this as http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50536 and this one appears to be running just fine. Can we coordinate tomorrow to reland this patch so that we can trigger a clean build as needed tomorrow? Alternatively, you don't do anything, I'll try to reland it tomorrow morning and start a clean build as needed.
(In reply to comment #16) > It appears that this was a build corruption issue. > > http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50532 > is the run that contains the rollout and this looks better but still a bunch of tests have failed. I've triggered a clean build after this as http://build.webkit.org/builders/Apple%20Win%207%20Debug%20%28Tests%29/builds/50536 and this one appears to be running just fine. > > Can we coordinate tomorrow to reland this patch so that we can trigger a clean build as needed tomorrow? Alternatively, you don't do anything, I'll try to reland it tomorrow morning and start a clean build as needed. If you want me around, it should probably wait until Thursday, as I'm OOO today and won't have IRC access until this afternoon.
Committed r146449: <http://trac.webkit.org/changeset/146449>
Sorry about the unwanted noise. I've relanded your patch in http://trac.webkit.org/changeset/146449 after cleaning WebKitBuild directories in apple-windows-1 and apple-windows-2.
I'm very excited to see this land!
(In reply to comment #19) > Sorry about the unwanted noise. > > I've relanded your patch in http://trac.webkit.org/changeset/146449 after cleaning WebKitBuild directories in apple-windows-1 and apple-windows-2. Thanks for relanding this! I really appreciate it.