Bug 75887

Summary: Followup SubresourceLoader cleanup after r100311
Product: WebKit Reporter: Nate Chapin <japhet>
Component: WebCore Misc.Assignee: Nate Chapin <japhet>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, dglazkov, koivisto, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
Diffing from the right directory this time
abarth: review+, webkit.review.bot: commit-queue-
Merged to trunk.
none
Patch for landing none

Description Nate Chapin 2012-01-09 13:35:57 PST
Per https://bugs.webkit.org/show_bug.cgi?id=71149#c6 , there were some details that were deferred in http://trac.webkit.org/changeset/100311 :

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.

In addition to these cleanups, I'd like to add: 
3) Remove the CachedImage multipart special case by adding an overridden setResponse() to CachedImage to do that work.
4) Remove the custom didReceiveData() call in the multipart logic of didReceiveResponse().  Have didReceiveData() get called normally, then do the multipart cleanup.

Patch shortly.
Comment 1 Nate Chapin 2012-01-09 13:51:55 PST
Created attachment 121723 [details]
patch
Comment 2 Adam Barth 2012-01-09 14:23:52 PST
Comment on attachment 121723 [details]
patch

ChangeLog?
Comment 3 Nate Chapin 2012-01-09 14:27:20 PST
Created attachment 121726 [details]
Diffing from the right directory this time
Comment 4 Adam Barth 2012-01-09 14:31:22 PST
Comment on attachment 121726 [details]
Diffing from the right directory this time

Very nice.
Comment 5 WebKit Review Bot 2012-01-09 14:33:18 PST
Comment on attachment 121726 [details]
Diffing from the right directory this time

Rejecting attachment 121726 [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:
re/loader/SubresourceLoader.cpp
Hunk #8 FAILED at 222.
Hunk #9 succeeded at 307 (offset 7 lines).
1 out of 9 hunks FAILED -- saving rejects to file Source/WebCore/loader/SubresourceLoader.cpp.rej
patching file Source/WebCore/loader/SubresourceLoader.h
Hunk #2 FAILED at 72.
1 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/loader/SubresourceLoader.h.rej

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/11113585
Comment 6 Nate Chapin 2012-01-09 15:47:59 PST
Created attachment 121738 [details]
Merged to trunk.

Requesting a new approval since the merge was non-trivial.
Comment 7 WebKit Review Bot 2012-01-09 17:29:53 PST
Comment on attachment 121738 [details]
Merged to trunk.

Rejecting attachment 121738 [details] from commit-queue.

New failing tests:
tables/mozilla/bugs/bug50695-2.html
http/tests/multipart/invalid-image-data-standalone.html
editing/pasteboard/copy-standalone-image.html
editing/pasteboard/data-transfer-items-image-png.html
fast/events/standalone-image-drag-to-editable.html
Full output: http://queues.webkit.org/results/11188671
Comment 8 WebKit Review Bot 2012-01-09 18:11:47 PST
Comment on attachment 121738 [details]
Merged to trunk.

Attachment 121738 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11183717

New failing tests:
tables/mozilla/bugs/bug50695-2.html
http/tests/multipart/invalid-image-data-standalone.html
editing/pasteboard/copy-standalone-image.html
editing/pasteboard/data-transfer-items-image-png.html
fast/events/standalone-image-drag-to-editable.html
Comment 9 Nate Chapin 2012-01-11 14:13:10 PST
Created attachment 122093 [details]
Patch for landing
Comment 10 WebKit Review Bot 2012-01-11 15:54:43 PST
Comment on attachment 122093 [details]
Patch for landing

Clearing flags on attachment: 122093

Committed r104756: <http://trac.webkit.org/changeset/104756>
Comment 11 WebKit Review Bot 2012-01-11 15:54:49 PST
All reviewed patches have been landed.  Closing bug.