Bug 83055 – Move (most of) Archive handling from FrameLoader to DocumentLoader

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Nate Chapin]

Per the comment in FrameLoader::finishedLoadingDocument(), there's a lot of work FrameLoader is doing that it should be delegating.  For that matter, FrameLoader::m_archive should really be hanging off the DocumentLoader, too.

[Nate Chapin]

Created attachment 135389 [details]
patch

[Philippe Normand]

Comment on attachment 135389 [details]
patch

Attachment 135389 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12320338

[Nate Chapin]

Created attachment 135414 [details]
fix gtk compile

[WebKit Review Bot]

Comment on attachment 135414 [details]
fix gtk compile

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

New failing tests:
mhtml/check_domain.mht

[WebKit Review Bot]

Created attachment 135437 [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

[Nate Chapin]

Created attachment 135690 [details]
Fix mhtml test failure

[Adam Barth]

Comment on attachment 135690 [details]
Fix mhtml test failure

View in context: https://bugs.webkit.org/attachment.cgi?id=135690&action=review

This patch makes too much sense!

> Source/WebCore/loader/DocumentLoader.h:136
>          void addAllArchiveResources(Archive*);

Can this be private now?

> Source/WebCore/loader/DocumentLoader.cpp:453
> +    // Give archive machinery a crack at this document. If the MIME type is not an archive type, it will return 0.

Give archive machinery -> Give the archive machinery

[Nate Chapin]

(In reply to comment #7)
> (From update of attachment 135690 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135690&action=review
> 
> This patch makes too much sense!
> 
> > Source/WebCore/loader/DocumentLoader.h:136
> >          void addAllArchiveResources(Archive*);
> 
> Can this be private now?

Nope. I didn't deal with the usage in FrameLoader::loadArchive(), and I see usages in PasteboardMac and WebKit/mac's WebDataSource.

> 
> > Source/WebCore/loader/DocumentLoader.cpp:453
> > +    // Give archive machinery a crack at this document. If the MIME type is not an archive type, it will return 0.
> 
> Give archive machinery -> Give the archive machinery

Ok.

[Nate Chapin]

Created attachment 136063 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 136063 [details]
Patch for landing

Clearing flags on attachment: 136063

Committed r113526: <http://trac.webkit.org/changeset/113526>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[James Simonsen]

This broke one of the Chrome browser_tests: RenderViewHostTest.BaseURLParam. I've rolled it back. Can you take a look when you get a chance?

[Nate Chapin]

Reopening since this was rolled out.

[Nate Chapin]

Created attachment 136923 [details]
Move setBaseURLOverride call back to FrameLoader

It turns out that setBaseURLOverride() needs to be called as soon as possible after an MHTML Document is created, and there is currently no way to do that in DocumentLoader.

My next planned cleanup is in the relationship between DocumentLoader, DocumentWriter and FrameLoader, so I will deal with this soon.

[Nate Chapin]

Created attachment 136935 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 136935 [details]
Patch for landing

Clearing flags on attachment: 136935

Committed r114016: <http://trac.webkit.org/changeset/114016>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.