1. DocumentLoader::m_gotFirstByte is weird. It appears to be primarily be used to gate committing a provisional load, except that it's always true when commitIfReady() is called. Remove that usage and instead use it to ensure we don't start document parsing more than once. 2. FrameLoader::m_hasReceivedFirstData effectively exists for this purpose currently, even though that's not what the name implies and it's per-DocumentLoader state living on FrameLoader. Remove it. 3. After these changes, FrameLoader::willSetEncoding() just calls receivedFirstData(), so call receivedFirstData directly. Do it from DocumentLoader instead of DocumentWriter. 4. Move some DocumentLoader calls from FrameLoader::receivedFirstData() into DocumentLoader::commitData(). This patch will remove the need for every FrameLoaderClient to call DocumentWriter::setEncoding() in finishedLoading() to create an empty document. I will remove that dead code in a separate patch.
Created attachment 137103 [details] patch
Comment on attachment 137103 [details] patch Attachment 137103 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12393955 New failing tests: mhtml/multi_frames_binary.mht mhtml/page_with_css_and_js_unmht.mht
Created attachment 137128 [details] Archive of layout-test-results from ec2-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 137103 [details] patch Yeah, those test failures are legitimate, removing review?
Created attachment 137176 [details] Fix test failures
Comment on attachment 137176 [details] Fix test failures This looks great.
Comment on attachment 137176 [details] Fix test failures Clearing flags on attachment: 137176 Committed r115654: <http://trac.webkit.org/changeset/115654>
All reviewed patches have been landed. Closing bug.
Looks like this change broke loading web archives in Safari; encodings specified in the archive are apparently not being respected. More details forthcoming as we diagnose further; probably a separate bug report too.
Regressions caused by this so far: bug 85275, bug 88428, bug 88436.
Do you think it's worth continuing to work though the regressions or should we consider backing out this patch?
(In reply to comment #11) > Do you think it's worth continuing to work though the regressions or should we consider backing out this patch? I'm disinclined to back it out now, especially since I'm close to a layout test to go with my patch for the last open regression, and reverting would entail backing out multiple patches that depend on it. That said, I fully recognize that I'm biased :)
(In reply to comment #12) > (In reply to comment #11) > > Do you think it's worth continuing to work though the regressions or should we consider backing out this patch? > > I'm disinclined to back it out now, especially since I'm close to a layout test to go with my patch for the last open regression, and reverting would entail backing out multiple patches that depend on it. > > That said, I fully recognize that I'm biased :) Especially since Nate is close to wrapping up the last known regression, I think we can stick with it.