[Webkit-unassigned] [Bug 42716] Implement WebArchive for Android

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42716


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |levin at chromium.org




--- Comment #5 from David Levin <levin at chromium.org>  2010-07-27 17:19:35 PST ---
(In reply to comment #3)
> Created an attachment (id=62227)
 --> (https://bugs.webkit.org/attachment.cgi?id=62227) [details]
> Android WebArchive patch v2.

General comments:

1. Don't use PassRefPtr as a local variable. It should only be a parameter or a return value.

2. Don't include parameter names in declarations when they add no information (like these places "create(Frame* frame);" and "create(SharedBuffer* buffer);" and "saveWebArchive(xmlTextWriterPtr writer);").

3. WebArchiveAndroid.h looks like it is missing includes for several things that it uses like PassRefPtr and vector<>

4. Use c++ style casting "(const char*)xmlNodeGetContent(fieldNode->xmlChildrenNode)"

5. Why does "saveArchive(xmlTextWriterPtr writer, PassRefPtr<Archive> archive)" take a PassRefPtr when it doesn't appear to take ownership of the pointer?

6. In general, wrapped lines should align with the open (. (I think this is just like Google's C++ standard.), so this

     static PassRefPtr<WebArchiveAndroid> create(PassRefPtr<ArchiveResource> mainResource,
         Vector<PassRefPtr<ArchiveResource> >& subresources,
         Vector<PassRefPtr<Archive> >& subframeArchives);

should be

static PassRefPtr<WebArchiveAndroid> create(PassRefPtr<ArchiveResource> mainResource,
                                            Vector<PassRefPtr<ArchiveResource> >& subresources,
                                            Vector<PassRefPtr<Archive> >& subframeArchives);
(which is aligned when using a fixed width font).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.