[webkit-reviews] review granted: [Bug 82826] Add security checks for iframe seamless : [Attachment 134950] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Mar 31 01:32:43 PDT 2012


Adam Barth <abarth at webkit.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 82826: Add security checks for iframe seamless
https://bugs.webkit.org/show_bug.cgi?id=82826

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

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


This is fine, but I'd prefer to land this together with tests.

> Source/WebCore/dom/Document.cpp:4755
> +static bool computeEligabilityForSeamless(Document* parent, Document* child)


I would have called this isEligabilityForSeamless

> Source/WebCore/dom/SecurityContext.h:70
> +    bool mayDisplaySeamlessWithParent() const { return
m_mayDisplaySeamlessWithParent; }

Usually we use the word "can" rather than "may", although you're right that
"may" is more grammatically correct.  IMHO, we should use "can" for
consistency.

> Source/WebCore/dom/SecurityContext.h:85
> +    // Set in Document::initSecurityContext() at Document creation, per:

I'd remove this comment.


More information about the webkit-reviews mailing list