[webkit-reviews] review granted: [Bug 52712] Add a compile-time option to disable WebArchive support : [Attachment 80036] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Jeremy Moskovich
<playmobil at google.com>'s request for review:
Bug 52712: Add a compile-time option to disable WebArchive support
https://bugs.webkit.org/show_bug.cgi?id=52712

Attachment 80036: Patch
https://bugs.webkit.org/attachment.cgi?id=80036&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=80036&action=review

Looks reasonable to me.  Certainly if WebArchive were added today, it would
have an ifdef.	I can't verify that all the placement details are correct, but
it's plausible.

> Source/WebCore/ChangeLog:3
> +

You've got an extra line here.

> Source/WebCore/WebCore.gypi:-2096
> -	       'loader/archive/cf/LegacyWebArchive.cpp',
> -	       'loader/archive/cf/LegacyWebArchive.h',
> -	       'loader/archive/cf/LegacyWebArchiveMac.mm',
> -	       'loader/archive/Archive.h',
> -	       'loader/archive/ArchiveFactory.cpp',
> -	       'loader/archive/ArchiveFactory.h',
> -	       'loader/archive/ArchiveResource.cpp',
> -	       'loader/archive/ArchiveResource.h',
> -	       'loader/archive/ArchiveResourceCollection.cpp',
> -	       'loader/archive/ArchiveResourceCollection.h',

The gypi should list all files in the project, regardless of whether they're
used by Chromium.

> Source/WebKit2/UIProcess/WebFrameProxy.h:113
>      void getWebArchive(PassRefPtr<DataCallback>);
>      void getMainResourceData(PassRefPtr<DataCallback>);
>      void getResourceData(WebURL*, PassRefPtr<DataCallback>);

This functions are mis-named (not the fault of this patch).