[webkit-reviews] review granted: [Bug 133944] [iOS][wk2] Use ImageDocument to display subframe PDFs : [Attachment 233251] new patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 17 13:53:52 PDT 2014

Daniel Bates <dbates at webkit.org> has granted Tim Horton <thorton at apple.com>'s
request for review:
Bug 133944: [iOS][wk2] Use ImageDocument to display subframe PDFs

Attachment 233251: new patch

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=233251&action=review

This patch seems sane to me. Feel free to ask a domain expert to review this
patch if you're looking for a thorough review.

> Source/WebCore/dom/DOMImplementation.cpp:319
> +    if ((!frame || !frame->isMainFrame()) &&
MIMETypeRegistry::isPDFMIMEType(type) && (frame &&

This expression can be simplified to:

if (frame && !frame->isMainFrame() && MIMETypeRegistry::isPDFMIMEType(type) &&

Notice that "!frame && frame" always evaluates to false by the truth table of
the logical AND (&&) operator.

On another note, in practice is frame ever null in this function. I assume it
can be from looking at the code in this function, especially the explicit null
check around the logic for assigning a value to allowedPluginTypes. However, we
don't null check frame before dereferencing it when we call
ImageDocument::create() below (line 336).

> Source/WebCore/platform/MIMETypeRegistry.cpp:573
> +

This empty line seems extraneous. One empty line seems sufficient to provide
visual separation between the end of this function and the start of the next

More information about the webkit-reviews mailing list