[webkit-reviews] review granted: [Bug 67983] CRASH under WebCore::ArchiveResourceCollection::addAllResources loading WebArchive : [Attachment 107138] [PATCH] Proposed Fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 13 08:40:11 PDT 2011

Darin Adler <darin at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 67983: CRASH under WebCore::ArchiveResourceCollection::addAllResources
loading WebArchive

Attachment 107138: [PATCH] Proposed Fix

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=107138&action=review

r=me but I think the test needs to be put in a different directory or somehow
otherwise skipped for non-webarchive platforms

> LayoutTests/ChangeLog:11
> +	   * loader/test-loading-archive-subresource-null-mimetype.html: Added.

This test needs to be skipped on the many platforms that do not support
CoreFoundation property list web archives. You should look for other similar
tests and see how that works. I’m guessing that the tests normally go inside
webarchive/loading rather than in loader to make sure they get skipped on
platforms that do not have web archives.

> Source/WebCore/loader/archive/cf/LegacyWebArchive.cpp:331
> +	   LOG(Archives, "LegacyWebArchive - Main Resource MIME Type is
required, but was null.");

The words Resource and Type should not be capitalized here.

> Source/WebCore/loader/archive/cf/LegacyWebArchive.cpp:351
> +	       RefPtr<ArchiveResource> subresource =
> +	       if (subresource)
> +		   addSubresource(subresource.release());

It would be nicer to define this variable inside the if.

More information about the webkit-reviews mailing list