[webkit-reviews] review granted: [Bug 83721] Framesniffing defense is too aggressive. : [Attachment 137174] Patch

Mon Apr 16 11:26:29 PDT 2012

James Robinson <jamesr at chromium.org> has granted Thomas Sepez
<tsepez at chromium.org>'s request for review:
Bug 83721: Framesniffing defense is too aggressive.
https://bugs.webkit.org/show_bug.cgi?id=83721

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=137174&action=review

This looks pretty good to me. One style nitpick.

> Source/WebCore/loader/FrameLoader.cpp:2693
> +    if (boundaryFrame.get())

You can (and WebKit code typically does) null-check smart pointers by just
doing:

if (boundaryFrame)

since RefPtr/OwnPtr implement operator bool()

> LayoutTests/http/tests/navigation/anchor-frames-same-origin.html:1
> +<html>

For new tests, we typically add a <!DOCTYPE html> declaration to put the page
in standards (as opposed to quirks) mode unless we specifically want to check a
quirks behavior. most of the time it doesn't make a difference, but it's useful
to have the lack of a doctype mean that there is something quirks-mode-specific
going on