2008-10-12 Sam Weinig <sam@webkit.org>
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 12 Oct 2008 20:43:15 +0000 (20:43 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 12 Oct 2008 20:43:15 +0000 (20:43 +0000)
        Reviewed by Darin Adler.

        Fix for https://bugs.webkit.org/show_bug.cgi?id=21560
        Layering violation: String should not be responsible for creating Lengths

        It was a layering violation for String to know haw to parse
        into Lengths, LengthArrays, and CoordsArrays.

        * GNUmakefile.am:
        * WebCore.pro:
        * WebCore.vcproj/WebCore.vcproj:
        * WebCore.xcodeproj/project.pbxproj:
        * WebCoreSources.bkl:
        * html/HTMLAreaElement.cpp:
        (WebCore::HTMLAreaElement::parseMappedAttribute):
        * html/HTMLFrameSetElement.cpp:
        (WebCore::HTMLFrameSetElement::parseMappedAttribute):
        * platform/text/AtomicString.h:
        (WebCore::AtomicString::percentage):
        * platform/text/PlatformString.h:
        * platform/text/String.cpp:
        * platform/text/StringImpl.cpp:
        * platform/text/StringImpl.h:
        * rendering/Length.cpp: Added.
        (WebCore::parseLength):
        (WebCore::countCharacter):
        (WebCore::newCoordsArray):
        (WebCore::newLengthArray):
        * rendering/Length.h:
        (WebCore::LengthSize::LengthSize):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@37527 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
WebCore/ChangeLog
WebCore/GNUmakefile.am
WebCore/WebCore.pro
WebCore/WebCore.vcproj/WebCore.vcproj
WebCore/WebCore.xcodeproj/project.pbxproj
WebCore/WebCoreSources.bkl
WebCore/html/HTMLAreaElement.cpp
WebCore/html/HTMLFrameSetElement.cpp
WebCore/platform/text/AtomicString.h
WebCore/platform/text/PlatformString.h
WebCore/platform/text/String.cpp
WebCore/platform/text/StringImpl.cpp
WebCore/platform/text/StringImpl.h
WebCore/rendering/Length.cpp [new file with mode: 0644]
WebCore/rendering/Length.h

index 2d6155f..04b73a9 100644 (file)
@@ -1,3 +1,36 @@
+2008-10-12  Sam Weinig  <sam@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        Fix for https://bugs.webkit.org/show_bug.cgi?id=21560
+        Layering violation: String should not be responsible for creating Lengths
+
+        It was a layering violation for String to know haw to parse
+        into Lengths, LengthArrays, and CoordsArrays.
+
+        * GNUmakefile.am:
+        * WebCore.pro:
+        * WebCore.vcproj/WebCore.vcproj:
+        * WebCore.xcodeproj/project.pbxproj:
+        * WebCoreSources.bkl:
+        * html/HTMLAreaElement.cpp:
+        (WebCore::HTMLAreaElement::parseMappedAttribute):
+        * html/HTMLFrameSetElement.cpp:
+        (WebCore::HTMLFrameSetElement::parseMappedAttribute):
+        * platform/text/AtomicString.h:
+        (WebCore::AtomicString::percentage):
+        * platform/text/PlatformString.h:
+        * platform/text/String.cpp:
+        * platform/text/StringImpl.cpp:
+        * platform/text/StringImpl.h:
+        * rendering/Length.cpp: Added.
+        (WebCore::parseLength):
+        (WebCore::countCharacter):
+        (WebCore::newCoordsArray):
+        (WebCore::newLengthArray):
+        * rendering/Length.h:
+        (WebCore::LengthSize::LengthSize):
+
 2008-10-12  Brad Garcia  <bgarcia@google.com>
 
         Reviewed by Darin Adler.
index 8b7eed4..ac0dc75 100644 (file)
@@ -1478,6 +1478,7 @@ webcore_sources += \
        WebCore/rendering/InlineTextBox.h \
        WebCore/rendering/LayoutState.cpp \
        WebCore/rendering/LayoutState.h \
+       WebCore/rendering/Length.cpp \
        WebCore/rendering/Length.h \
        WebCore/rendering/ListMarkerBox.cpp \
        WebCore/rendering/ListMarkerBox.h \
index b5a1074..6239026 100644 (file)
@@ -874,6 +874,7 @@ SOURCES += \
     rendering/InlineFlowBox.cpp \
     rendering/InlineTextBox.cpp \
     rendering/LayoutState.cpp \
+    rendering/Length.cpp \
     rendering/ListMarkerBox.cpp \
     rendering/RenderApplet.cpp \
     rendering/RenderArena.cpp \
index b81e0c5..23650b9 100644 (file)
                                >\r
                        </File>\r
                        <File\r
+                               RelativePath="..\rendering\Length.cpp"\r
+                               >\r
+                               <FileConfiguration\r
+                                       Name="Release_PGO|Win32"\r
+                                       >\r
+                                       <Tool\r
+                                               Name="VCCLCompilerTool"\r
+                                               WholeProgramOptimization="true"\r
+                                       />\r
+                               </FileConfiguration>\r
+                       </File>\r
+                       <File\r
                                RelativePath="..\rendering\Length.h"\r
                                >\r
                        </File>\r
index 3686cf6..1750d99 100644 (file)
                BC7FA6820D1F167900DB22A9 /* SelectorNodeList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BC7FA6800D1F167900DB22A9 /* SelectorNodeList.cpp */; };
                BC80C9870CD294EE00A0B7B3 /* CSSTimingFunctionValue.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BC80C9850CD294EE00A0B7B3 /* CSSTimingFunctionValue.cpp */; };
                BC80C9880CD294EE00A0B7B3 /* CSSTimingFunctionValue.h in Headers */ = {isa = PBXBuildFile; fileRef = BC80C9860CD294EE00A0B7B3 /* CSSTimingFunctionValue.h */; };
+               BC80FA130EA287B000B0821D /* Length.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BC80FA120EA287B000B0821D /* Length.cpp */; };
                BC8243290D0CE8A200460C8F /* JSSQLError.h in Headers */ = {isa = PBXBuildFile; fileRef = BC8243250D0CE8A200460C8F /* JSSQLError.h */; };
                BC82432A0D0CE8A200460C8F /* JSSQLTransaction.h in Headers */ = {isa = PBXBuildFile; fileRef = BC8243260D0CE8A200460C8F /* JSSQLTransaction.h */; };
                BC82432B0D0CE8A200460C8F /* JSVoidCallback.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BC8243270D0CE8A200460C8F /* JSVoidCallback.cpp */; };
                BC7FA6800D1F167900DB22A9 /* SelectorNodeList.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SelectorNodeList.cpp; sourceTree = "<group>"; };
                BC80C9850CD294EE00A0B7B3 /* CSSTimingFunctionValue.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = CSSTimingFunctionValue.cpp; sourceTree = "<group>"; };
                BC80C9860CD294EE00A0B7B3 /* CSSTimingFunctionValue.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = CSSTimingFunctionValue.h; sourceTree = "<group>"; };
+               BC80FA120EA287B000B0821D /* Length.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Length.cpp; sourceTree = "<group>"; };
                BC8243250D0CE8A200460C8F /* JSSQLError.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSSQLError.h; sourceTree = "<group>"; };
                BC8243260D0CE8A200460C8F /* JSSQLTransaction.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSSQLTransaction.h; sourceTree = "<group>"; };
                BC8243270D0CE8A200460C8F /* JSVoidCallback.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSVoidCallback.cpp; sourceTree = "<group>"; };
                                BCEA481B097D93020094C9E4 /* InlineTextBox.h */,
                                2D9066040BE141D400956998 /* LayoutState.cpp */,
                                2D9066050BE141D400956998 /* LayoutState.h */,
+                               BC80FA120EA287B000B0821D /* Length.cpp */,
                                935C476009AC4CD100A6AAB4 /* Length.h */,
                                A8EA7A4D0A191A5200A8EF5F /* ListMarkerBox.cpp */,
                                A8EA7A490A191A5200A8EF5F /* ListMarkerBox.h */,
                                BC3BE9AB0E9C242000835588 /* RenderScrollbarPart.cpp in Sources */,
                                1CF6BDFB0E9BB26A0025E1CD /* ObjCEventListener.mm in Sources */,
                                1CF6BE140E9BB4670025E1CD /* ObjCNodeFilterCondition.mm in Sources */,
+                               BC80FA130EA287B000B0821D /* Length.cpp in Sources */,
                        );
                        runOnlyForDeploymentPostprocessing = 0;
                };
index d1d1c66..e2dc31a 100644 (file)
@@ -826,6 +826,7 @@ This file contains the list of files needed to build WebCore.
         rendering/InlineFlowBox.cpp
         rendering/InlineTextBox.cpp
         rendering/LayoutState.cpp
+        rendering/Length.cpp
         rendering/ListMarkerBox.cpp
         rendering/MediaControlElements.cpp
         rendering/RenderApplet.cpp
index e8a4e79..a865122 100644 (file)
@@ -26,6 +26,7 @@
 #include "FloatRect.h"
 #include "HTMLNames.h"
 #include "HitTestResult.h"
+#include "Length.h"
 #include "RenderObject.h"
 
 using namespace std;
@@ -61,7 +62,7 @@ void HTMLAreaElement::parseMappedAttribute(MappedAttribute *attr)
             m_shape = Rect;
     } else if (attr->name() == coordsAttr) {
         delete [] m_coords;
-        m_coords = attr->value().toCoordsArray(m_coordsLen);
+        m_coords = newCoordsArray(attr->value().string(), m_coordsLen);
     } else if (attr->name() == altAttr || attr->name() == accesskeyAttr) {
         // Do nothing.
     } else
index aacfb93..d693cb3 100644 (file)
@@ -32,6 +32,7 @@
 #include "EventNames.h"
 #include "HTMLNames.h"
 #include "Length.h"
+#include "Length.h"
 #include "MouseEvent.h"
 #include "RenderFrameSet.h"
 #include "Text.h"
@@ -88,13 +89,13 @@ void HTMLFrameSetElement::parseMappedAttribute(MappedAttribute *attr)
     if (attr->name() == rowsAttr) {
         if (!attr->isNull()) {
             if (m_rows) delete [] m_rows;
-            m_rows = attr->value().toLengthArray(m_totalRows);
+            m_rows = newLengthArray(attr->value().string(), m_totalRows);
             setChanged();
         }
     } else if (attr->name() == colsAttr) {
         if (!attr->isNull()) {
             delete [] m_cols;
-            m_cols = attr->value().toLengthArray(m_totalCols);
+            m_cols = newLengthArray(attr->value().string(), m_totalCols);
             setChanged();
         }
     } else if (attr->name() == frameborderAttr) {
index d8136a1..af72c46 100644 (file)
@@ -77,8 +77,6 @@ public:
     double toDouble(bool* ok = 0) const { return m_string.toDouble(ok); }
     float toFloat(bool* ok = 0) const { return m_string.toFloat(ok); }
     bool percentage(int& p) const { return m_string.percentage(p); }
-    Length* toLengthArray(int& len) const { return m_string.toLengthArray(len); }
-    Length* toCoordsArray(int& len) const { return m_string.toCoordsArray(len); }
 
     bool isNull() const { return m_string.isNull(); }
     bool isEmpty() const { return m_string.isEmpty(); }
index cf1df35..a2174ee 100644 (file)
@@ -164,8 +164,7 @@ public:
     uint64_t toUInt64(bool* ok = 0) const;
     double toDouble(bool* ok = 0) const;
     float toFloat(bool* ok = 0) const;
-    Length* toLengthArray(int& len) const;
-    Length* toCoordsArray(int& len) const;
+
     bool percentage(int& percentage) const;
 
     // Makes a deep copy. Helpful only if you need to use a String on another thread.
index 7afee61..662fd17 100644 (file)
@@ -519,16 +519,6 @@ bool String::isEmpty() const
     return !m_impl || !m_impl->length();
 }
 
-Length* String::toCoordsArray(int& len) const 
-{
-    return m_impl ? m_impl->toCoordsArray(len) : 0;
-}
-
-Length* String::toLengthArray(int& len) const 
-{
-    return m_impl ? m_impl->toLengthArray(len) : 0;
-}
-
 void String::split(const String& separator, bool allowEmptyEntries, Vector<String>& result) const
 {
     result.clear();
index 6ad3229..0fc8232 100644 (file)
@@ -29,7 +29,6 @@
 #include "CString.h"
 #include "CharacterNames.h"
 #include "FloatConversion.h"
-#include "Length.h"
 #include "StringBuffer.h"
 #include "StringHash.h"
 #include "TextBreakIterator.h"
@@ -194,124 +193,6 @@ UChar32 StringImpl::characterStartingAt(unsigned i)
     return 0;
 }
 
-static Length parseLength(const UChar* data, unsigned length)
-{
-    if (length == 0)
-        return Length(1, Relative);
-
-    unsigned i = 0;
-    while (i < length && isSpaceOrNewline(data[i]))
-        ++i;
-    if (i < length && (data[i] == '+' || data[i] == '-'))
-        ++i;
-    while (i < length && isASCIIDigit(data[i]))
-        ++i;
-    unsigned intLength = i;
-    while (i < length && (isASCIIDigit(data[i]) || data[i] == '.'))
-        ++i;
-    unsigned doubleLength = i;
-
-    // IE quirk: Skip whitespace between the number and the % character (20 % => 20%).
-    while (i < length && isSpaceOrNewline(data[i]))
-        ++i;
-
-    bool ok;
-    UChar next = (i < length) ? data[i] : ' ';
-    if (next == '%') {
-        // IE quirk: accept decimal fractions for percentages.
-        double r = charactersToDouble(data, doubleLength, &ok);
-        if (ok)
-            return Length(r, Percent);
-        return Length(1, Relative);
-    }
-    int r = charactersToIntStrict(data, intLength, &ok);
-    if (next == '*') {
-        if (ok)
-            return Length(r, Relative);
-        return Length(1, Relative);
-    }
-    if (ok)
-        return Length(r, Fixed);
-    return Length(0, Relative);
-}
-
-Length StringImpl::toLength()
-{
-    return parseLength(m_data, m_length);
-}
-
-static int countCharacter(StringImpl* string, UChar character)
-{
-    int count = 0;
-    int length = string->length();
-    for (int i = 0; i < length; ++i)
-            count += (*string)[i] == character;
-    return count;
-}
-
-Length* StringImpl::toCoordsArray(int& len)
-{
-    StringBuffer spacified(m_length);
-    for (unsigned i = 0; i < m_length; i++) {
-        UChar cc = m_data[i];
-        if (cc > '9' || (cc < '0' && cc != '-' && cc != '*' && cc != '.'))
-            spacified[i] = ' ';
-        else
-            spacified[i] = cc;
-    }
-    RefPtr<StringImpl> str = adopt(spacified);
-
-    str = str->simplifyWhiteSpace();
-
-    len = countCharacter(str.get(), ' ') + 1;
-    Length* r = new Length[len];
-
-    int i = 0;
-    int pos = 0;
-    int pos2;
-
-    while ((pos2 = str->find(' ', pos)) != -1) {
-        r[i++] = parseLength(str->characters() + pos, pos2 - pos);
-        pos = pos2+1;
-    }
-    r[i] = parseLength(str->characters() + pos, str->length() - pos);
-
-    ASSERT(i == len - 1);
-
-    return r;
-}
-
-Length* StringImpl::toLengthArray(int& len)
-{
-    RefPtr<StringImpl> str = simplifyWhiteSpace();
-    if (!str->length()) {
-        len = 1;
-        return 0;
-    }
-
-    len = countCharacter(str.get(), ',') + 1;
-    Length* r = new Length[len];
-
-    int i = 0;
-    int pos = 0;
-    int pos2;
-
-    while ((pos2 = str->find(',', pos)) != -1) {
-        r[i++] = parseLength(str->characters() + pos, pos2 - pos);
-        pos = pos2+1;
-    }
-
-    ASSERT(i == len - 1);
-
-    /* IE Quirk: If the last comma is the last char skip it and reduce len by one */
-    if (str->length()-pos > 0)
-        r[i] = parseLength(str->characters() + pos, str->length() - pos);
-    else
-        len--;
-
-    return r;
-}
-
 bool StringImpl::isLower()
 {
     // Do a faster loop for the case where all the characters are ASCII.
index 820b1ba..cc8f815 100644 (file)
@@ -44,7 +44,6 @@ class StringBuffer;
 
 struct CStringTranslator;
 struct HashAndCharactersTranslator;
-struct Length;
 struct StringHash;
 struct UCharBufferTranslator;
 
@@ -99,8 +98,6 @@ public:
     UChar operator[](unsigned i) { ASSERT(i < m_length); return m_data[i]; }
     UChar32 characterStartingAt(unsigned);
 
-    Length toLength();
-
     bool containsOnlyWhitespace();
 
     int toIntStrict(bool* ok = 0, int base = 10);
@@ -116,8 +113,6 @@ public:
     double toDouble(bool* ok = 0);
     float toFloat(bool* ok = 0);
 
-    Length* toCoordsArray(int& len);
-    Length* toLengthArray(int& len);
     bool isLower();
     PassRefPtr<StringImpl> lower();
     PassRefPtr<StringImpl> upper();
diff --git a/WebCore/rendering/Length.cpp b/WebCore/rendering/Length.cpp
new file mode 100644 (file)
index 0000000..3757d92
--- /dev/null
@@ -0,0 +1,151 @@
+/*
+ * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
+ *           (C) 1999 Antti Koivisto (koivisto@kde.org)
+ *           (C) 2001 Dirk Mueller ( mueller@kde.org )
+ * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2006 Andrew Wellington (proton@wiretapped.net)
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB.  If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ *
+ */
+
+#include "config.h"
+#include "Length.h"
+
+#include "PlatformString.h"
+#include "StringBuffer.h"
+#include <wtf/ASCIICType.h>
+#include <wtf/Assertions.h>
+
+using namespace WTF;
+
+namespace WebCore {
+
+static Length parseLength(const UChar* data, unsigned length)
+{
+    if (length == 0)
+        return Length(1, Relative);
+
+    unsigned i = 0;
+    while (i < length && isSpaceOrNewline(data[i]))
+        ++i;
+    if (i < length && (data[i] == '+' || data[i] == '-'))
+        ++i;
+    while (i < length && isASCIIDigit(data[i]))
+        ++i;
+    unsigned intLength = i;
+    while (i < length && (isASCIIDigit(data[i]) || data[i] == '.'))
+        ++i;
+    unsigned doubleLength = i;
+
+    // IE quirk: Skip whitespace between the number and the % character (20 % => 20%).
+    while (i < length && isSpaceOrNewline(data[i]))
+        ++i;
+
+    bool ok;
+    UChar next = (i < length) ? data[i] : ' ';
+    if (next == '%') {
+        // IE quirk: accept decimal fractions for percentages.
+        double r = charactersToDouble(data, doubleLength, &ok);
+        if (ok)
+            return Length(r, Percent);
+        return Length(1, Relative);
+    }
+    int r = charactersToIntStrict(data, intLength, &ok);
+    if (next == '*') {
+        if (ok)
+            return Length(r, Relative);
+        return Length(1, Relative);
+    }
+    if (ok)
+        return Length(r, Fixed);
+    return Length(0, Relative);
+}
+
+static int countCharacter(const UChar* data, unsigned length, UChar character)
+{
+    int count = 0;
+    for (int i = 0; i < static_cast<int>(length); ++i)
+        count += data[i] == character;
+    return count;
+}
+
+Length* newCoordsArray(const String& string, int& len)
+{
+    unsigned length = string.length();
+    const UChar* data = string.characters();
+    StringBuffer spacified(length);
+    for (unsigned i = 0; i < length; i++) {
+        UChar cc = data[i];
+        if (cc > '9' || (cc < '0' && cc != '-' && cc != '*' && cc != '.'))
+            spacified[i] = ' ';
+        else
+            spacified[i] = cc;
+    }
+    RefPtr<StringImpl> str = StringImpl::adopt(spacified);
+
+    str = str->simplifyWhiteSpace();
+
+    len = countCharacter(str->characters(), str->length(), ' ') + 1;
+    Length* r = new Length[len];
+
+    int i = 0;
+    int pos = 0;
+    int pos2;
+
+    while ((pos2 = str->find(' ', pos)) != -1) {
+        r[i++] = parseLength(str->characters() + pos, pos2 - pos);
+        pos = pos2+1;
+    }
+    r[i] = parseLength(str->characters() + pos, str->length() - pos);
+
+    ASSERT(i == len - 1);
+
+    return r;
+}
+
+Length* newLengthArray(const String& string, int& len)
+{
+    RefPtr<StringImpl> str = string.impl()->simplifyWhiteSpace();
+    if (!str->length()) {
+        len = 1;
+        return 0;
+    }
+
+    len = countCharacter(str->characters(), str->length(), ',') + 1;
+    Length* r = new Length[len];
+
+    int i = 0;
+    int pos = 0;
+    int pos2;
+
+    while ((pos2 = str->find(',', pos)) != -1) {
+        r[i++] = parseLength(str->characters() + pos, pos2 - pos);
+        pos = pos2+1;
+    }
+
+    ASSERT(i == len - 1);
+
+    // IE Quirk: If the last comma is the last char skip it and reduce len by one.
+    if (str->length()-pos > 0)
+        r[i] = parseLength(str->characters() + pos, str->length() - pos);
+    else
+        len--;
+
+    return r;
+}
+
+} // namespace WebCore
index f2e0dc6..569f1b9 100644 (file)
@@ -28,6 +28,8 @@
 
 namespace WebCore {
 
+class String;
+
 const int undefinedLength = -1;
 const int percentScaleFactor = 128;
 
@@ -224,16 +226,21 @@ struct LengthBox {
 struct LengthSize {
     Length width;
     Length height;
-    
+
     LengthSize()
-    {}
+    {
+    }
     
     LengthSize(const Length& w, const Length& h)
-    : width(w)
-    , height(h)
-    {}
+        : width(w)
+        , height(h)
+    {
+    }
 };
 
+Length* newCoordsArray(const String&, int& len);
+Length* newLengthArray(const String&, int& len);
+
 } // namespace WebCore
 
 #endif // Length_h