[webkit-reviews] review denied: [Bug 32529] Rename skipCanLoadCheck to skipSecurityCheck : [Attachment 44891] patch2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 15 13:59:02 PST 2009


Darin Adler <darin at apple.com> has denied Nate Chapin <japhet at chromium.org>'s
request for review:
Bug 32529: Rename skipCanLoadCheck to skipSecurityCheck
https://bugs.webkit.org/show_bug.cgi?id=32529

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

------- Additional Comments from Darin Adler <darin at apple.com>
Sorry, I'm not trying to be difficult, but I see a problem now introduced by
the changes I suggested.

> +    enum SecurityCheck {
> +	   DoSecurityCheck,
> +	   SkipSecurityCheck
> +    };

> -	   bool shouldSkipCanLoadCheck() const { return
m_shouldSkipCanLoadCheck; }
> +	   SecurityCheck shouldDoSecurityCheck() const { return
m_shouldDoSecurityCheck; }

This is a risky combination. The value of DoSecurityCheck is 0 and
SkipSecurityCheck is 1, so this code:

    if (object->shouldDoSecurityCheck())

would look right, but be wrong. We should make the name of the function and
data member be shouldSkipSecurityCheck or something neutral like
securityCheckPolicy that does not imply a true/false mapping.


More information about the webkit-reviews mailing list