2009-03-25 Darin Adler <darin@apple.com>
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Mar 2009 14:36:31 +0000 (14:36 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Mar 2009 14:36:31 +0000 (14:36 +0000)
        Reviewed by John Sullivan.

        Tidy up LegacyWebArchive a bit. And don't include favicons in web archives when
        they are being used only for selections, not an entire document.

        * WebCore.base.exp: Remove exported function that's not used in WebKit.

        * loader/archive/cf/LegacyWebArchive.cpp:
        (WebCore::LegacyWebArchive::createPropertyListRepresentation): Made a static member
        function so it can share private things with other member functions. Added some FIXME
        comments to a branch of code I think is dead. Changed boolean argument into a named
        one. Renamed to match other similar functions.
        (WebCore::LegacyWebArchive::createResourceResponseFromPropertyListData): Ditto.
        (WebCore::LegacyWebArchive::createResource): Ditto.
        (WebCore::LegacyWebArchive::create): Merged the create and init functions.
        (WebCore::LegacyWebArchive::rawDataRepresentation): Added some assertions because
        we should never fail to write (we can fail to read).
        (WebCore::LegacyWebArchive::createResourceResponseFromMacArchivedData): Made a static
        member function as above.
        (WebCore::LegacyWebArchive::createFromSelection): Ditto.
        (WebCore::LegacyWebArchive::create): Iterate the vector with indices rather than
        iterators. Only include the favicon if the first node is the document since we don't
        want to include the favicon when copying and pasting.

        * loader/archive/cf/LegacyWebArchive.h: Made one of the create functions private.
        Made the Mac-specific functions be static member functions. Made other helpers be
        static member functions. Removed the nit function.

        * loader/archive/cf/LegacyWebArchiveMac.mm:
        (WebCore::LegacyWebArchive::createResourceResponseFromMacArchivedData): Updated
        comment and made this a static member function.
        (WebCore::LegacyWebArchive::createPropertyListRepresentation): Ditto. Also made
        it so we only type cast in one place.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@41973 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/WebCore.base.exp
WebCore/loader/archive/cf/LegacyWebArchive.cpp
WebCore/loader/archive/cf/LegacyWebArchive.h
WebCore/loader/archive/cf/LegacyWebArchiveMac.mm

index d8171e2..a80f7ee 100644 (file)
@@ -1,5 +1,41 @@
 2009-03-25  Darin Adler  <darin@apple.com>
 
+        Reviewed by John Sullivan.
+
+        Tidy up LegacyWebArchive a bit. And don't include favicons in web archives when
+        they are being used only for selections, not an entire document.
+
+        * WebCore.base.exp: Remove exported function that's not used in WebKit.
+
+        * loader/archive/cf/LegacyWebArchive.cpp:
+        (WebCore::LegacyWebArchive::createPropertyListRepresentation): Made a static member
+        function so it can share private things with other member functions. Added some FIXME
+        comments to a branch of code I think is dead. Changed boolean argument into a named
+        one. Renamed to match other similar functions.
+        (WebCore::LegacyWebArchive::createResourceResponseFromPropertyListData): Ditto.
+        (WebCore::LegacyWebArchive::createResource): Ditto.
+        (WebCore::LegacyWebArchive::create): Merged the create and init functions.
+        (WebCore::LegacyWebArchive::rawDataRepresentation): Added some assertions because
+        we should never fail to write (we can fail to read).
+        (WebCore::LegacyWebArchive::createResourceResponseFromMacArchivedData): Made a static
+        member function as above.
+        (WebCore::LegacyWebArchive::createFromSelection): Ditto.
+        (WebCore::LegacyWebArchive::create): Iterate the vector with indices rather than
+        iterators. Only include the favicon if the first node is the document since we don't
+        want to include the favicon when copying and pasting.
+
+        * loader/archive/cf/LegacyWebArchive.h: Made one of the create functions private.
+        Made the Mac-specific functions be static member functions. Made other helpers be
+        static member functions. Removed the nit function.
+
+        * loader/archive/cf/LegacyWebArchiveMac.mm:
+        (WebCore::LegacyWebArchive::createResourceResponseFromMacArchivedData): Updated
+        comment and made this a static member function.
+        (WebCore::LegacyWebArchive::createPropertyListRepresentation): Ditto. Also made
+        it so we only type cast in one place.
+
+2009-03-25  Darin Adler  <darin@apple.com>
+
         Reviewed by David Hyatt.
 
         Bug 24740: crash in RenderSlider::setPositionFromValue when calling pause() after setting <video> to display: none
index 8f8c709..ef9b05f 100644 (file)
@@ -361,7 +361,6 @@ __ZN7WebCore16LegacyWebArchive6createEPNS_12SharedBufferE
 __ZN7WebCore16LegacyWebArchive6createEPNS_4NodeE
 __ZN7WebCore16LegacyWebArchive6createEPNS_5FrameE
 __ZN7WebCore16LegacyWebArchive6createEPNS_5RangeE
-__ZN7WebCore16LegacyWebArchive6createERKNS_6StringEPNS_5FrameERN3WTF6VectorIPNS_4NodeELm0EEE
 __ZN7WebCore16LegacyWebArchive6createEv
 __ZN7WebCore16MIMETypeRegistry24isSupportedImageMIMETypeERKNS_6StringE
 __ZN7WebCore16MIMETypeRegistry26getSupportedImageMIMETypesEv
index 236c4e1..d1269cc 100644 (file)
@@ -63,15 +63,16 @@ static const CFStringRef LegacyWebArchiveResourceTextEncodingNameKey = CFSTR("We
 static const CFStringRef LegacyWebArchiveResourceResponseKey = CFSTR("WebResourceResponse");
 static const CFStringRef LegacyWebArchiveResourceResponseVersionKey = CFSTR("WebResourceResponseVersion");
 
-static RetainPtr<CFDictionaryRef> createPropertyListRepresentationFromResource(ArchiveResource* resource, bool mainResource)
+RetainPtr<CFDictionaryRef> LegacyWebArchive::createPropertyListRepresentation(ArchiveResource* resource, MainResourceStatus isMainResource)
 {
     if (!resource) {
-        // The property list representation of a null/empty WebResource has the following 3 objects stored as nil
+        // The property list representation of a null/empty WebResource has the following 3 objects stored as nil.
+        // FIXME: 0 is not serializable. Presumably we need to use kCFNull here instead for compatibility.
+        // FIXME: But why do we need to support a resource of 0? Who relies on that?
         RetainPtr<CFMutableDictionaryRef> propertyList(AdoptCF, CFDictionaryCreateMutable(0, 3, 0, 0));
         CFDictionarySetValue(propertyList.get(), LegacyWebArchiveResourceDataKey, 0);
         CFDictionarySetValue(propertyList.get(), LegacyWebArchiveResourceURLKey, 0);
         CFDictionarySetValue(propertyList.get(), LegacyWebArchiveResourceMIMETypeKey, 0);
-
         return propertyList;
     }
     
@@ -97,7 +98,7 @@ static RetainPtr<CFDictionaryRef> createPropertyListRepresentationFromResource(A
 
     // FrameName should be left out if empty for subresources, but always included for main resources
     const String& frameName(resource->frameName());
-    if (!frameName.isEmpty() || mainResource) {
+    if (!frameName.isEmpty() || isMainResource) {
         RetainPtr<CFStringRef> cfFrameName(AdoptCF, frameName.createCFString());
         CFDictionarySetValue(propertyList.get(), LegacyWebArchiveResourceFrameNameKey, cfFrameName.get());
     }
@@ -116,8 +117,8 @@ static RetainPtr<CFDictionaryRef> createPropertyListRepresentationFromResource(A
     }
 
     // Don't include the resource response for the main resource
-    if (!mainResource) {
-        RetainPtr<CFDataRef> resourceResponseData = propertyListDataFromResourceResponse(resource->response());
+    if (!isMainResource) {
+        RetainPtr<CFDataRef> resourceResponseData = createPropertyListRepresentation(resource->response());
         if (resourceResponseData)
             CFDictionarySetValue(propertyList.get(), LegacyWebArchiveResourceResponseKey, resourceResponseData.get());    
     }
@@ -125,19 +126,20 @@ static RetainPtr<CFDictionaryRef> createPropertyListRepresentationFromResource(A
     return propertyList;
 }
 
-static RetainPtr<CFDictionaryRef> createPropertyListRep(Archive* archive)
+RetainPtr<CFDictionaryRef> LegacyWebArchive::createPropertyListRepresentation(Archive* archive)
 {
     RetainPtr<CFMutableDictionaryRef> propertyList(AdoptCF, CFDictionaryCreateMutable(0, 3, 0, &kCFTypeDictionaryValueCallBacks));
     
-    RetainPtr<CFDictionaryRef> mainResourceDict = createPropertyListRepresentationFromResource(archive->mainResource(), true);
+    RetainPtr<CFDictionaryRef> mainResourceDict = createPropertyListRepresentation(archive->mainResource(), MainResource);
+    ASSERT(mainResourceDict);
     if (!mainResourceDict)
         return 0;
     CFDictionarySetValue(propertyList.get(), LegacyWebArchiveMainResourceKey, mainResourceDict.get());
-        
+
     RetainPtr<CFMutableArrayRef> subresourcesArray(AdoptCF, CFArrayCreateMutable(0, archive->subresources().size(), &kCFTypeArrayCallBacks));
     const Vector<RefPtr<ArchiveResource> >& subresources(archive->subresources());
     for (unsigned i = 0; i < subresources.size(); ++i) {
-        RetainPtr<CFDictionaryRef> subresource = createPropertyListRepresentationFromResource(subresources[i].get(), false);
+        RetainPtr<CFDictionaryRef> subresource = createPropertyListRepresentation(subresources[i].get(), Subresource);
         if (subresource)
             CFArrayAppendValue(subresourcesArray.get(), subresource.get());
         else
@@ -149,7 +151,7 @@ static RetainPtr<CFDictionaryRef> createPropertyListRep(Archive* archive)
     RetainPtr<CFMutableArrayRef> subframesArray(AdoptCF, CFArrayCreateMutable(0, archive->subframeArchives().size(), &kCFTypeArrayCallBacks));
     const Vector<RefPtr<Archive> >& subframeArchives(archive->subframeArchives());
     for (unsigned i = 0; i < subframeArchives.size(); ++i) {
-        RetainPtr<CFDictionaryRef> subframeArchive = createPropertyListRep(subframeArchives[i].get());
+        RetainPtr<CFDictionaryRef> subframeArchive = createPropertyListRepresentation(subframeArchives[i].get());
         if (subframeArchive)
             CFArrayAppendValue(subframesArray.get(), subframeArchive.get());
         else
@@ -161,22 +163,23 @@ static RetainPtr<CFDictionaryRef> createPropertyListRep(Archive* archive)
     return propertyList;
 }
 
-static ResourceResponse createResourceResponseFromPropertyListData(CFDataRef data, CFStringRef responseDataType)
+ResourceResponse LegacyWebArchive::createResourceResponseFromPropertyListData(CFDataRef data, CFStringRef responseDataType)
 {
     ASSERT(data);
     if (!data)
         return ResourceResponse();
     
-    // If the ResourceResponseVersion (passed in as responseDataType) exists at all, this is a "new" webarchive that we can parse well in a cross platform manner
-    // If it doesn't exist, we will assume this is an "old" Cocoa-based WebArchive, and parse the ResourceResponse as such
+    // If the ResourceResponseVersion (passed in as responseDataType) exists at all, this is a "new" web archive that we
+    // can parse well in a cross platform manner If it doesn't exist, we will assume this is an "old" web archive with,
+    // NSURLResponse objects in it and parse the ResourceResponse as such.
     if (!responseDataType)
         return createResourceResponseFromMacArchivedData(data);
         
-    // FIXME: Parse the "new" format that the above comment references here
+    // FIXME: Parse the "new" format that the above comment references here. This format doesn't exist yet.
     return ResourceResponse();
 }
 
-static PassRefPtr<ArchiveResource> createResource(CFDictionaryRef dictionary)
+PassRefPtr<ArchiveResource> LegacyWebArchive::createResource(CFDictionaryRef dictionary)
 {
     ASSERT(dictionary);
     if (!dictionary)
@@ -238,17 +241,6 @@ PassRefPtr<LegacyWebArchive> LegacyWebArchive::create()
     return adoptRef(new LegacyWebArchive);
 }
 
-PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(SharedBuffer* data)
-{
-    LOG(Archives, "LegacyWebArchive - Creating from raw data");
-    
-    RefPtr<LegacyWebArchive> archive = create();
-    if (!archive->init(data))
-        return 0;
-        
-    return archive.release();
-}
-
 PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(PassRefPtr<ArchiveResource> mainResource, Vector<PassRefPtr<ArchiveResource> >& subresources, Vector<PassRefPtr<LegacyWebArchive> >& subframeArchives)
 {
     ASSERT(mainResource);
@@ -267,19 +259,19 @@ PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(PassRefPtr<ArchiveResource
     return archive.release();
 }
 
-LegacyWebArchive::LegacyWebArchive()
-{
-}
-
-bool LegacyWebArchive::init(SharedBuffer* data)
+PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(SharedBuffer* data)
 {
+    LOG(Archives, "LegacyWebArchive - Creating from raw data");
+    
+    RefPtr<LegacyWebArchive> archive = create();
+        
     ASSERT(data);
     if (!data)
-        return false;
+        return 0;
         
     RetainPtr<CFDataRef> cfData(AdoptCF, data->createCFData());
     if (!cfData)
-        return false;
+        return 0;
         
     CFStringRef errorString = 0;
     
@@ -291,15 +283,18 @@ bool LegacyWebArchive::init(SharedBuffer* data)
 #endif
         if (errorString)
             CFRelease(errorString);
-        return false;
+        return 0;
     }
     
     if (CFGetTypeID(plist.get()) != CFDictionaryGetTypeID()) {
         LOG(Archives, "LegacyWebArchive - Archive property list is not the expected CFDictionary, aborting invalid WebArchive");
-        return false;
+        return 0;
     }
     
-    return extract(plist.get());
+    if (!archive->extract(plist.get()))
+        return 0;
+
+    return archive.release();
 }
 
 bool LegacyWebArchive::extract(CFDictionaryRef dictionary)
@@ -372,7 +367,8 @@ bool LegacyWebArchive::extract(CFDictionaryRef dictionary)
 
 RetainPtr<CFDataRef> LegacyWebArchive::rawDataRepresentation()
 {
-    RetainPtr<CFDictionaryRef> propertyList = createPropertyListRep(this);
+    RetainPtr<CFDictionaryRef> propertyList = createPropertyListRepresentation(this);
+    ASSERT(propertyList);
     if (!propertyList) {
         LOG(Archives, "LegacyWebArchive - Failed to create property list for archive, returning no data");
         return 0;
@@ -384,6 +380,7 @@ RetainPtr<CFDataRef> LegacyWebArchive::rawDataRepresentation()
     CFPropertyListWriteToStream(propertyList.get(), stream.get(), kCFPropertyListBinaryFormat_v1_0, 0);
 
     RetainPtr<CFDataRef> plistData(AdoptCF, static_cast<CFDataRef>(CFWriteStreamCopyProperty(stream.get(), kCFStreamPropertyDataWritten)));
+    ASSERT(plistData);
 
     CFWriteStreamClose(stream.get());
 
@@ -396,20 +393,21 @@ RetainPtr<CFDataRef> LegacyWebArchive::rawDataRepresentation()
 }
 
 #if !PLATFORM(MAC)
-// FIXME: Is it possible to parse in a Cocoa-style resource response manually, 
-// without NSKeyed(Un)Archiver, manipulating plists directly?
-// If so, the code that does it will go here.  
-// In the meantime, Mac will continue to NSKeyed(Un)Archive the response as it always has
-ResourceResponse createResourceResponseFromMacArchivedData(CFDataRef responseData)
+
+ResourceResponse LegacyWebArchive::createResourceResponseFromMacArchivedData(CFDataRef responseData)
 {
+    // FIXME: If is is possible to parse in a serialized NSURLResponse manually, without using
+    // NSKeyedUnarchiver, manipulating plists directly, then we want to do that here.
+    // Until then, this can be done on Mac only.
     return ResourceResponse();
 }
 
-RetainPtr<CFDataRef> propertyListDataFromResourceResponse(const ResourceResponse& response)
+RetainPtr<CFDataRef> LegacyWebArchive::createPropertyListRepresentation(const ResourceResponse& response)
 {
-    // FIXME: Write out the "new" format described in ::createResourceResponseFromPropertyListData() up above
+    // FIXME: Write out the "new" format described in createResourceResponseFromPropertyListData once we invent it.
     return 0;
 }
+
 #endif
 
 PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(Node* node)
@@ -453,7 +451,7 @@ PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(Frame* frame)
     Vector<PassRefPtr<ArchiveResource> > subresources;
     documentLoader->getSubresources(subresources);
 
-    return LegacyWebArchive::create(documentLoader->mainResource(), subresources, subframeArchives);
+    return create(documentLoader->mainResource(), subresources, subframeArchives);
 }
 
 PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(Range* range)
@@ -481,7 +479,7 @@ PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(Range* range)
     return create(markupString, frame, nodeList);
 }
 
-PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(const String& markupString, Frame* frame, Vector<Node*>& nodes)
+PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(const String& markupString, Frame* frame, const Vector<Node*>& nodes)
 {
     ASSERT(frame);
     
@@ -498,16 +496,14 @@ PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(const String& markupString
     Vector<PassRefPtr<LegacyWebArchive> > subframeArchives;
     Vector<PassRefPtr<ArchiveResource> > subresources;
     HashSet<KURL> uniqueSubresources;
-    
-    Vector<Node*>::iterator it = nodes.begin();
-    Vector<Node*>::iterator end = nodes.end();
-    
-    for (; it != end; ++it) {
+
+    size_t nodesSize = nodes.size();    
+    for (size_t i = 0; i < nodesSize; ++i) {
+        Node* node = nodes[i];
         Frame* childFrame;
-        if (((*it)->hasTagName(HTMLNames::frameTag) || (*it)->hasTagName(HTMLNames::iframeTag) || (*it)->hasTagName(HTMLNames::objectTag)) &&
-             (childFrame = static_cast<HTMLFrameOwnerElement*>(*it)->contentFrame())) {
-            RefPtr<LegacyWebArchive> subframeArchive;
-            subframeArchive = LegacyWebArchive::create(childFrame->document());
+        if ((node->hasTagName(HTMLNames::frameTag) || node->hasTagName(HTMLNames::iframeTag) || node->hasTagName(HTMLNames::objectTag)) &&
+             (childFrame = static_cast<HTMLFrameOwnerElement*>(node)->contentFrame())) {
+            RefPtr<LegacyWebArchive> subframeArchive = create(childFrame->document());
             
             if (subframeArchive)
                 subframeArchives.append(subframeArchive);
@@ -515,7 +511,7 @@ PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(const String& markupString
                 LOG_ERROR("Unabled to archive subframe %s", childFrame->tree()->name().string().utf8().data());
         } else {
             ListHashSet<KURL> subresourceURLs;
-            (*it)->getSubresourceURLs(subresourceURLs);
+            node->getSubresourceURLs(subresourceURLs);
             
             DocumentLoader* documentLoader = frame->loader()->documentLoader();
             ListHashSet<KURL>::iterator iterEnd = subresourceURLs.end();
@@ -547,8 +543,8 @@ PassRefPtr<LegacyWebArchive> LegacyWebArchive::create(const String& markupString
         }
     }
 
-    // Add favicon if one exists for this page
-    if (iconDatabase() && iconDatabase()->isEnabled()) {
+    // Add favicon if one exists for this page, if we are archiving the entire page.
+    if (nodesSize && nodes[0]->isDocumentNode() && iconDatabase() && iconDatabase()->isEnabled()) {
         const String& iconURL = iconDatabase()->iconURLForPageURL(responseURL);
         if (!iconURL.isEmpty() && iconDatabase()->iconDataKnownForIconURL(iconURL)) {
             if (Image* iconImage = iconDatabase()->iconForPageURL(responseURL, IntSize(16, 16))) {
@@ -586,7 +582,7 @@ PassRefPtr<LegacyWebArchive> LegacyWebArchive::createFromSelection(Frame* frame)
     Vector<PassRefPtr<LegacyWebArchive> > subframeArchives;
     subframeArchives.append(archive);
     
-    archive = LegacyWebArchive::create(iframeResource.release(), subresources, subframeArchives);
+    archive = create(iframeResource.release(), subresources, subframeArchives);
     
     return archive.release();
 }
index 70faba5..8c8f2e4 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -31,8 +31,6 @@
 
 #include "Archive.h"
 
-#include <wtf/PassRefPtr.h>
-
 namespace WebCore {
 
 class Frame;
@@ -40,28 +38,33 @@ class Node;
 class Range;
 
 class LegacyWebArchive : public Archive {
-public:    
+public:
     static PassRefPtr<LegacyWebArchive> create();
     static PassRefPtr<LegacyWebArchive> create(SharedBuffer*);
     static PassRefPtr<LegacyWebArchive> create(PassRefPtr<ArchiveResource> mainResource, Vector<PassRefPtr<ArchiveResource> >& subresources, Vector<PassRefPtr<LegacyWebArchive> >& subframeArchives);
     static PassRefPtr<LegacyWebArchive> create(Node*);
     static PassRefPtr<LegacyWebArchive> create(Frame*);
-    static PassRefPtr<LegacyWebArchive> createFromSelection(Frame* frame);
+    static PassRefPtr<LegacyWebArchive> createFromSelection(Frame*);
     static PassRefPtr<LegacyWebArchive> create(Range*);
-    static PassRefPtr<LegacyWebArchive> create(const String& markupString, Frame*, Vector<Node*>& nodes);
 
     RetainPtr<CFDataRef> rawDataRepresentation();
 
 private:
-    LegacyWebArchive();
-    bool init(SharedBuffer*);
+    LegacyWebArchive() { }
+
+    enum MainResourceStatus { Subresource, MainResource };
+
+    static PassRefPtr<LegacyWebArchive> create(const String& markupString, Frame*, const Vector<Node*>& nodes);
+    static PassRefPtr<ArchiveResource> createResource(CFDictionaryRef);
+    static ResourceResponse createResourceResponseFromMacArchivedData(CFDataRef);
+    static ResourceResponse createResourceResponseFromPropertyListData(CFDataRef, CFStringRef responseDataType);
+    static RetainPtr<CFDataRef> createPropertyListRepresentation(const ResourceResponse&);
+    static RetainPtr<CFDictionaryRef> createPropertyListRepresentation(Archive*);
+    static RetainPtr<CFDictionaryRef> createPropertyListRepresentation(ArchiveResource*, MainResourceStatus);
+
     bool extract(CFDictionaryRef);
-    
 };
 
-ResourceResponse createResourceResponseFromMacArchivedData(CFDataRef);
-RetainPtr<CFDataRef> propertyListDataFromResourceResponse(const ResourceResponse&);
-
 }
 
 #endif // Archive
index b853a15..c474bba 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -33,9 +33,9 @@ namespace WebCore {
 
 static const NSString *LegacyWebArchiveResourceResponseKey = @"WebResourceResponse";
 
-// FIXME: Is it possible to parse in a Cocoa-style resource response manually, 
-// without NSKeyed(Un)Archiver, manipulating plists directly?
-ResourceResponse createResourceResponseFromMacArchivedData(CFDataRef responseData)
+// FIXME: If is is possible to parse in a serialized NSURLResponse manually, without using
+// NSKeyedUnarchiver, manipulating plists directly, we would prefer to do that instead.
+ResourceResponse LegacyWebArchive::createResourceResponseFromMacArchivedData(CFDataRef responseData)
 {    
     ASSERT(responseData);
     if (!responseData)
@@ -56,19 +56,21 @@ ResourceResponse createResourceResponseFromMacArchivedData(CFDataRef responseDat
     return ResourceResponse(response);
 }
 
-RetainPtr<CFDataRef> propertyListDataFromResourceResponse(const ResourceResponse& response)
+RetainPtr<CFDataRef> LegacyWebArchive::createPropertyListRepresentation(const ResourceResponse& response)
 {    
     NSURLResponse *nsResponse = response.nsURLResponse();
+    ASSERT(nsResponse);
     if (!nsResponse)
         return 0;
-        
-    NSMutableData *responseData = (NSMutableData *)CFDataCreateMutable(0, 0);
-    NSKeyedArchiver *archiver = [[NSKeyedArchiver alloc] initForWritingWithMutableData:responseData];
+
+    CFMutableDataRef responseData = CFDataCreateMutable(0, 0);
+
+    NSKeyedArchiver *archiver = [[NSKeyedArchiver alloc] initForWritingWithMutableData:(NSMutableData *)responseData];
     [archiver encodeObject:nsResponse forKey:LegacyWebArchiveResourceResponseKey];
     [archiver finishEncoding];
     [archiver release];
     
-    return RetainPtr<CFDataRef>(AdoptCF, (CFDataRef)responseData);
+    return RetainPtr<CFDataRef>(AdoptCF, responseData);
 }
 
 }