[webkit-reviews] review denied: [Bug 39427] Increase limit on number of (i)frames from 200 to 1000. : [Attachment 56899] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 24 12:25:19 PDT 2010


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied
hans at chromium.org's request for review:
Bug 39427: Increase limit on number of (i)frames from 200 to 1000.
https://bugs.webkit.org/show_bug.cgi?id=39427

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebCore/page/Page.h:240
 +	    static const int m_maxNbrOfFrames = 1000;
style nit: spell out words and reserve m_ for private members:
m_maxNbrOfFrames -> maxNumberOfFrames

LayoutTests/compositing/iframes/too-many-objects.html:9
 +    var created_objects = 0;
nit: use camelCase for variables

LayoutTests/compositing/iframes/too-many-objects.html:26
 +    {
nit: be consistent with braces

LayoutTests/compositing/iframes/too-many-objects.html:25
 +    function createIFrames(nbr)
nit: avoid abbreviations like "nbr"

nit: it seems like you could combine lots-of-iframes and too-many-iframes.
lots-of-iframes is testing that you can create 1000 frames, and
too-many-iframes
is testing that once you have 1000 frames, you can't create one more.  they
seem
like they could just be part of a single test that asserts that 1000 is the
limit
for the number of tests you can create.  same goes for the object tag.


More information about the webkit-reviews mailing list