[Webkit-unassigned] [Bug 67983] CRASH under WebCore::ArchiveResourceCollection::addAllResources loading WebArchive

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #107138|review?                     |review+
               Flag|                            |




--- Comment #6 from Darin Adler <darin at apple.com>  2011-09-13 08:40:11 PST ---
(From update of attachment 107138)
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 = createResource(subresourceDict);
> +            if (subresource)
> +                addSubresource(subresource.release());

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

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