[webkit-reviews] review granted: [Bug 32368] Add defensive initialization of iframe sandbox flags : [Attachment 46242] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 32368: Add defensive initialization of iframe sandbox flags
https://bugs.webkit.org/show_bug.cgi?id=32368

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

------- Additional Comments from Darin Adler <darin at apple.com>
>      if (shouldTreatURLSchemeAsNoAccess(m_protocol))
>	   m_isUnique = true;
>  
> -    // If this ASSERT becomes false in the future, please consider the
impact
> -    // of m_sandoxFlags on m_isUnique.
> -    ASSERT(m_sandboxFlags == SandboxNone);
> +    if (isSandboxed(SandboxOrigin))
> +	   m_isUnique = true;

I suggest initializing m_unique to the correct value from the outset.

    m_isUnique(shouldTreatURLSchemeAsNoAccess(m_protocol) ||
isSandboxed(SandboxOrigin))

To some the if statements may be slightly clearer, so it seems OK to leave it
this way if you think that way.

> +    static PassRefPtr<SecurityOrigin> create(const KURL&, SandboxFlags
sandboxFlags = SandboxNone);

The argument name isn't needed here because the type speaks for itself.