As of http://trac.webkit.org/changeset/98380, CachedResourceRequest is the only SubresourceLoaderClient. SubresourceLoader does little more than call its client. Ergo, merge them. Call the merged class SubresourceLoader, because CachedResourceRequest is the worst naming decision I've made on this project.
Created attachment 112923 [details] Attempt #1
Comment on attachment 112923 [details] Attempt #1 Attachment 112923 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10244058
Created attachment 113070 [details] Address compile failure
Comment on attachment 113070 [details] Address compile failure Attachment 113070 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10241878 New failing tests: http/tests/inspector/resource-tree/resource-tree-reload.html
Created attachment 114360 [details] Fix chromium test failure + merge to trunk
Comment on attachment 114360 [details] Fix chromium test failure + merge to trunk This looks great. A couple ideas for a follow-up patch: 1) Add some ASSERTS to the destructor that show we properly advance the state machine to the end (and that documenting becomes null). 2) Use a member variable to track whether we've incremented/decremented the request count to ensure we balance them properly (e.g., by ASSERTing in its destructor that we've balanced). That will also remove a bunch of multipart related banches because we can tell this object to decrement the request count if it hasn't already done so.
Comment on attachment 114360 [details] Fix chromium test failure + merge to trunk Rejecting attachment 114360 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ubresourceLoaderClient.h rm 'Source/WebCore/loader/SubresourceLoaderClient.h' patching file Source/WebCore/loader/cf/SubresourceLoaderCF.cpp patching file Source/WebCore/loader/ResourceLoader.cpp patching file Source/WebCore/loader/ResourceLoadScheduler.h patching file Source/WebCore/loader/SubresourceLoader.cpp patching file Source/WebCore/loader/SubresourceLoader.h Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Adam Barth', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/10372598
Created attachment 115194 [details] Patch for landing
Comment on attachment 115194 [details] Patch for landing Clearing flags on attachment: 115194 Committed r100311: <http://trac.webkit.org/changeset/100311>
All reviewed patches have been landed. Closing bug.
This patch seems to have caused http://code.google.com/p/chromium/issues/detail?id=104582 downstream.
This patch caused http://code.google.com/p/chromium/issues/detail?id=104920 upstream. didFailLoading is now called on InspectorInstrumentation when XHR is loaded correctly, because resource loading is cancelled after successful load. View in context: https://bugs.webkit.org/attachment.cgi?id=115194&action=review > Source/WebCore/loader/cache/CachedRawResource.cpp:82 > + m_loader->cancel(); This triggers cancellation. > Source/WebCore/loader/cache/CachedResourceRequest.cpp:-159 > - if (m_finishing) This check is missing now. We can override ResourceLoader::cancel() in SubresourceLoader::cancel() and add this check there, does that seem correct to you?
Another regression from this change: bug 83918.