[webkit-reviews] review denied: [Bug 20709] Implement HTML 5's HTMLElement.classList property : [Attachment 64365] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 13 17:05:21 PDT 2010


Sam Weinig <sam at webkit.org> has denied Erik Arvidsson <arv at chromium.org>'s
request for review:
Bug 20709: Implement HTML 5's HTMLElement.classList property
https://bugs.webkit.org/show_bug.cgi?id=20709

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

------- Additional Comments from Sam Weinig <sam at webkit.org>
> -	   typedef Vector<AtomicString, 8> StringVector;
>	   String m_string;
> -	   StringVector m_vector;
>	   bool m_shouldFoldCase;
> +	   bool m_folded;
> +	   StringVector m_vector;
> +	   StringVector m_vectorNoFold;

I am worried about the memory increase in adding a second vector here.	Have
you done any memory benchmarking to make sure this doesn't regress things (eg.
membuster)

> @@ -0,0 +1,151 @@
> +// Copyright (c) 2009 The Chromium Authors. All rights reserved.
> +// Use of this source code is governed by a BSD-style license that can be
> +// found in the LICENSE file.

This is an invalid license header.  Please see
http://webkit.org/coding/contributing.html for more on this.


> +++ b/WebCore/html/ClassList.h
> @@ -0,0 +1,44 @@
> +// Copyright (c) 2009 The Chromium Authors. All rights reserved.
> +// Use of this source code is governed by a BSD-style license that can be
> +// found in the LICENSE file.

Here too. (There are more as well).

r-.


More information about the webkit-reviews mailing list