Title: [174388] trunk/Source
Revision
174388
Author
da...@apple.com
Date
2014-10-06 23:58:41 -0700 (Mon, 06 Oct 2014)

Log Message

Make StringView check the lifetime of the StringImpl it's created from
https://bugs.webkit.org/show_bug.cgi?id=137202

Reviewed by Anders Carlsson.

Source/WebCore:

* platform/graphics/TextRun.cpp: Update since TextRun's definition now
uses a StringView.

Source/WTF:

* WTF.vcxproj/WTF.vcxproj: Added StringView.cpp.
* WTF.vcxproj/WTF.vcxproj.filters: Added StringView.cpp.
* WTF.xcodeproj/project.pbxproj: Added StringView.cpp.
* wtf/CMakeLists.txt: Added StringView.cpp.

* wtf/text/StringImpl.cpp:
(WTF::StringImpl::~StringImpl): Call StringView::invalidate.

* wtf/text/StringView.cpp: Added.
(WTF::underlyingStrings): Returns map from StringImpl to the underlying
string object used by StringView to track validity.
(WTF::StringView::invalidate): Mark the underlying string object invalid,
and remove it from the map, so any future StringImpl will get a new one,
even if it has the same pointer.
(WTF::StringView::underlyingStringIsValid): Return true only if the
underlying string is still valid.
(WTF::StringView::setUnderlyingString): Create and manage reference counts
of underlying string objects as needed.

* wtf/text/StringView.h: Moved function bodies out of the class definition,
so we can now read a clean class definition to see the class design and what
functions it offers.
(WTF::StringView::StringView): Added a comment to the default constructor.
Also added copy and move constructors so they can call setUnderlyingString
and assert the underlying string is valid as needed, replacing the
compiler-generated ones.
(WTF::StringView::~StringView): Added a call to setUnderlyingString.
(WTF::StringView::operator=): Added these assignment operators with the same
job as the constructors above.
(WTF::StringView::initialize): Added a comment.
(WTF::StringView::characters8): Added an assertion that the underlying
string is valid.
(WTF::StringView::characters16): Ditto.
(WTF::StringView::substring): Added code to propagate the underlying string
from the original string to the substring.
(WTF::StringView::invalidate): Inline empty version of this function for
non-debug builds.
(WTF::StringView::underlyingStringIsValid): Ditto.
(WTF::StringView::setUnderlyingString): Ditto.

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (174387 => 174388)


--- trunk/Source/WTF/ChangeLog	2014-10-07 06:54:48 UTC (rev 174387)
+++ trunk/Source/WTF/ChangeLog	2014-10-07 06:58:41 UTC (rev 174388)
@@ -1,3 +1,50 @@
+2014-10-06  Darin Adler  <da...@apple.com>
+
+        Make StringView check the lifetime of the StringImpl it's created from
+        https://bugs.webkit.org/show_bug.cgi?id=137202
+
+        Reviewed by Anders Carlsson.
+
+        * WTF.vcxproj/WTF.vcxproj: Added StringView.cpp.
+        * WTF.vcxproj/WTF.vcxproj.filters: Added StringView.cpp.
+        * WTF.xcodeproj/project.pbxproj: Added StringView.cpp.
+        * wtf/CMakeLists.txt: Added StringView.cpp.
+
+        * wtf/text/StringImpl.cpp:
+        (WTF::StringImpl::~StringImpl): Call StringView::invalidate.
+
+        * wtf/text/StringView.cpp: Added.
+        (WTF::underlyingStrings): Returns map from StringImpl to the underlying
+        string object used by StringView to track validity.
+        (WTF::StringView::invalidate): Mark the underlying string object invalid,
+        and remove it from the map, so any future StringImpl will get a new one,
+        even if it has the same pointer.
+        (WTF::StringView::underlyingStringIsValid): Return true only if the
+        underlying string is still valid.
+        (WTF::StringView::setUnderlyingString): Create and manage reference counts
+        of underlying string objects as needed.
+
+        * wtf/text/StringView.h: Moved function bodies out of the class definition,
+        so we can now read a clean class definition to see the class design and what
+        functions it offers.
+        (WTF::StringView::StringView): Added a comment to the default constructor.
+        Also added copy and move constructors so they can call setUnderlyingString
+        and assert the underlying string is valid as needed, replacing the
+        compiler-generated ones.
+        (WTF::StringView::~StringView): Added a call to setUnderlyingString.
+        (WTF::StringView::operator=): Added these assignment operators with the same
+        job as the constructors above.
+        (WTF::StringView::initialize): Added a comment.
+        (WTF::StringView::characters8): Added an assertion that the underlying
+        string is valid.
+        (WTF::StringView::characters16): Ditto.
+        (WTF::StringView::substring): Added code to propagate the underlying string
+        from the original string to the substring.
+        (WTF::StringView::invalidate): Inline empty version of this function for
+        non-debug builds.
+        (WTF::StringView::underlyingStringIsValid): Ditto.
+        (WTF::StringView::setUnderlyingString): Ditto.
+
 2014-10-06  Brent Fulgham  <bfulg...@apple.com>
 
         [Win] DateMath's calculateUTFOffset does not account for DST.

Modified: trunk/Source/WTF/WTF.vcxproj/WTF.vcxproj (174387 => 174388)


--- trunk/Source/WTF/WTF.vcxproj/WTF.vcxproj	2014-10-07 06:54:48 UTC (rev 174387)
+++ trunk/Source/WTF/WTF.vcxproj/WTF.vcxproj	2014-10-07 06:58:41 UTC (rev 174388)
@@ -134,6 +134,7 @@
     <ClCompile Include="..\wtf\text\StringBuilder.cpp" />
     <ClCompile Include="..\wtf\text\StringImpl.cpp" />
     <ClCompile Include="..\wtf\text\StringStatics.cpp" />
+    <ClCompile Include="..\wtf\text\StringView.cpp" />
     <ClCompile Include="..\wtf\text\WTFString.cpp" />
     <ClCompile Include="..\wtf\text\cf\AtomicStringCF.cpp" />
     <ClCompile Include="..\wtf\text\cf\StringCF.cpp" />

Modified: trunk/Source/WTF/WTF.vcxproj/WTF.vcxproj.filters (174387 => 174388)


--- trunk/Source/WTF/WTF.vcxproj/WTF.vcxproj.filters	2014-10-07 06:54:48 UTC (rev 174387)
+++ trunk/Source/WTF/WTF.vcxproj/WTF.vcxproj.filters	2014-10-07 06:58:41 UTC (rev 174388)
@@ -60,6 +60,9 @@
     <ClCompile Include="..\wtf\text\StringStatics.cpp">
       <Filter>text</Filter>
     </ClCompile>
+    <ClCompile Include="..\wtf\text\StringView.cpp">
+      <Filter>text</Filter>
+    </ClCompile>
     <ClCompile Include="..\wtf\text\AtomicString.cpp">
       <Filter>text</Filter>
     </ClCompile>
@@ -716,4 +719,4 @@
     <None Include="WTFPostBuild.cmd" />
     <None Include="WTFPreBuild.cmd" />
   </ItemGroup>
-</Project>
\ No newline at end of file
+</Project>

Modified: trunk/Source/WTF/WTF.xcodeproj/project.pbxproj (174387 => 174388)


--- trunk/Source/WTF/WTF.xcodeproj/project.pbxproj	2014-10-07 06:54:48 UTC (rev 174387)
+++ trunk/Source/WTF/WTF.xcodeproj/project.pbxproj	2014-10-07 06:58:41 UTC (rev 174388)
@@ -84,6 +84,7 @@
 		93934BD518A1F16900D0D6A1 /* StringViewCF.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 93934BD418A1F16900D0D6A1 /* StringViewCF.cpp */; };
 		93AC91A818942FC400244939 /* LChar.h in Headers */ = {isa = PBXBuildFile; fileRef = 93AC91A718942FC400244939 /* LChar.h */; };
 		93B1AA80180E5AF3004A2F05 /* PassRef.h in Headers */ = {isa = PBXBuildFile; fileRef = 93B1AA7F180E5AF3004A2F05 /* PassRef.h */; };
+		93F1993E19D7958D00C2390B /* StringView.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 93F1993D19D7958D00C2390B /* StringView.cpp */; };
 		974CFC8E16A4F327006D5404 /* WeakPtr.h in Headers */ = {isa = PBXBuildFile; fileRef = 974CFC8D16A4F327006D5404 /* WeakPtr.h */; };
 		9BC70F05176C379D00101DEC /* AtomicStringTable.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9BC70F04176C379D00101DEC /* AtomicStringTable.cpp */; };
 		9BD8F40B176C2B470002D865 /* AtomicStringTable.h in Headers */ = {isa = PBXBuildFile; fileRef = 9BD8F40A176C2AD80002D865 /* AtomicStringTable.h */; };
@@ -373,6 +374,7 @@
 		93934BD418A1F16900D0D6A1 /* StringViewCF.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = StringViewCF.cpp; path = cf/StringViewCF.cpp; sourceTree = "<group>"; };
 		93AC91A718942FC400244939 /* LChar.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LChar.h; sourceTree = "<group>"; };
 		93B1AA7F180E5AF3004A2F05 /* PassRef.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PassRef.h; sourceTree = "<group>"; };
+		93F1993D19D7958D00C2390B /* StringView.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = StringView.cpp; sourceTree = "<group>"; };
 		974CFC8D16A4F327006D5404 /* WeakPtr.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WeakPtr.h; sourceTree = "<group>"; };
 		9BC70F04176C379D00101DEC /* AtomicStringTable.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = AtomicStringTable.cpp; sourceTree = "<group>"; };
 		9BD8F40A176C2AD80002D865 /* AtomicStringTable.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = AtomicStringTable.h; sourceTree = "<group>"; };
@@ -954,12 +956,13 @@
 				A8A47327151A825B004123FF /* StringHash.h */,
 				A8A47328151A825B004123FF /* StringImpl.cpp */,
 				A8A47329151A825B004123FF /* StringImpl.h */,
+				1A6EB1DF187D0BD30030126F /* StringView.h */,
+				93F1993D19D7958D00C2390B /* StringView.cpp */,
 				A8A4732A151A825B004123FF /* StringOperators.h */,
 				A8A4732B151A825B004123FF /* StringStatics.cpp */,
 				A8A4732C151A825B004123FF /* TextPosition.h */,
 				A8A4732D151A825B004123FF /* WTFString.cpp */,
 				A8A4732E151A825B004123FF /* WTFString.h */,
-				1A6EB1DF187D0BD30030126F /* StringView.h */,
 			);
 			path = text;
 			sourceTree = "<group>";
@@ -1351,6 +1354,7 @@
 				A8A473EC151A825B004123FF /* MetaAllocator.cpp in Sources */,
 				A8A473F4151A825B004123FF /* NumberOfCores.cpp in Sources */,
 				A8A473F7151A825B004123FF /* OSAllocatorPosix.cpp in Sources */,
+				93F1993E19D7958D00C2390B /* StringView.cpp in Sources */,
 				A8A473F9151A825B004123FF /* OSRandomSource.cpp in Sources */,
 				A8A47400151A825B004123FF /* PageAllocationAligned.cpp in Sources */,
 				A8A47402151A825B004123FF /* PageBlock.cpp in Sources */,

Modified: trunk/Source/WTF/wtf/CMakeLists.txt (174387 => 174388)


--- trunk/Source/WTF/wtf/CMakeLists.txt	2014-10-07 06:54:48 UTC (rev 174387)
+++ trunk/Source/WTF/wtf/CMakeLists.txt	2014-10-07 06:58:41 UTC (rev 174388)
@@ -201,6 +201,7 @@
     text/StringBuilder.cpp
     text/StringImpl.cpp
     text/StringStatics.cpp
+    text/StringView.cpp
     text/WTFString.cpp
 
     threads/BinarySemaphore.cpp

Modified: trunk/Source/WTF/wtf/text/StringImpl.cpp (174387 => 174388)


--- trunk/Source/WTF/wtf/text/StringImpl.cpp	2014-10-07 06:54:48 UTC (rev 174387)
+++ trunk/Source/WTF/wtf/text/StringImpl.cpp	2014-10-07 06:58:41 UTC (rev 174388)
@@ -105,6 +105,8 @@
 {
     ASSERT(!isStatic());
 
+    StringView::invalidate(*this);
+
     STRING_STATS_REMOVE_STRING(*this);
 
     if (isAtomic() && m_length)

Added: trunk/Source/WTF/wtf/text/StringView.cpp (0 => 174388)


--- trunk/Source/WTF/wtf/text/StringView.cpp	                        (rev 0)
+++ trunk/Source/WTF/wtf/text/StringView.cpp	2014-10-07 06:58:41 UTC (rev 174388)
@@ -0,0 +1,123 @@
+/*
+
+Copyright (C) 2014 Apple Inc. All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+1.  Redistributions of source code must retain the above copyright
+    notice, this list of conditions and the following disclaimer.
+2.  Redistributions in binary form must reproduce the above copyright
+    notice, this list of conditions and the following disclaimer in the
+    documentation and/or other materials provided with the distribution.
+
+THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY
+EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+#include "config.h"
+#include "StringView.h"
+
+#include <mutex>
+#include <wtf/HashMap.h>
+#include <wtf/NeverDestroyed.h>
+
+#if CHECK_STRINGVIEW_LIFETIME
+
+namespace WTF {
+
+// Manage reference count manually so UnderlyingString does not need to be defined in the header.
+
+struct StringView::UnderlyingString {
+    std::atomic_uint refCount { 1 };
+    bool isValid { true };
+    const StringImpl& string;
+    explicit UnderlyingString(const StringImpl&);
+};
+
+StringView::UnderlyingString::UnderlyingString(const StringImpl& string)
+    : string(string)
+{
+}
+
+static std::mutex& underlyingStringsMutex()
+{
+    static NeverDestroyed<std::mutex> mutex;
+    return mutex;
+}
+
+static HashMap<const StringImpl*, StringView::UnderlyingString*>& underlyingStrings()
+{
+    static NeverDestroyed<HashMap<const StringImpl*, StringView::UnderlyingString*>> map;
+    return map;
+}
+
+void StringView::invalidate(const StringImpl& stringToBeDestroyed)
+{
+    UnderlyingString* underlyingString;
+    {
+        std::lock_guard<std::mutex> lock(underlyingStringsMutex());
+        underlyingString = underlyingStrings().take(&stringToBeDestroyed);
+        if (!underlyingString)
+            return;
+    }
+    ASSERT(underlyingString->isValid);
+    underlyingString->isValid = false;
+}
+
+bool StringView::underlyingStringIsValid() const
+{
+    return !m_underlyingString || m_underlyingString->isValid;
+}
+
+void StringView::adoptUnderlyingString(UnderlyingString* underlyingString)
+{
+    if (m_underlyingString) {
+        if (!--m_underlyingString->refCount) {
+            if (m_underlyingString->isValid) {
+                std::lock_guard<std::mutex> lock(underlyingStringsMutex());
+                underlyingStrings().remove(&m_underlyingString->string);
+            }
+            delete m_underlyingString;
+        }
+    }
+    m_underlyingString = underlyingString;
+}
+
+void StringView::setUnderlyingString(const StringImpl* string)
+{
+    UnderlyingString* underlyingString;
+    if (!string)
+        underlyingString = nullptr;
+    else {
+        std::lock_guard<std::mutex> lock(underlyingStringsMutex());
+        auto result = underlyingStrings().add(string, nullptr);
+        if (result.isNewEntry)
+            result.iterator->value = new UnderlyingString(*string);
+        else
+            ++result.iterator->value->refCount;
+        underlyingString = result.iterator->value;
+    }
+    adoptUnderlyingString(underlyingString);
+}
+
+void StringView::setUnderlyingString(const StringView& string)
+{
+    UnderlyingString* underlyingString = string.m_underlyingString;
+    if (underlyingString)
+        ++underlyingString->refCount;
+    adoptUnderlyingString(underlyingString);
+}
+
+}
+
+#endif
Property changes on: trunk/Source/WTF/wtf/text/StringView.cpp
___________________________________________________________________

Added: svn:eol-style

Modified: trunk/Source/WTF/wtf/text/StringView.h (174387 => 174388)


--- trunk/Source/WTF/wtf/text/StringView.h	2014-10-07 06:54:48 UTC (rev 174387)
+++ trunk/Source/WTF/wtf/text/StringView.h	2014-10-07 06:58:41 UTC (rev 174388)
@@ -32,160 +32,202 @@
 #include <wtf/Vector.h>
 #include <wtf/text/LChar.h>
 
+#ifdef NDEBUG
+#define CHECK_STRINGVIEW_LIFETIME 0
+#else
+#define CHECK_STRINGVIEW_LIFETIME 1
+#endif
+
 namespace WTF {
 
 // StringView is a non-owning reference to a string, similar to the proposed std::string_view.
 // Whether the string is 8-bit or 16-bit is encoded in the upper bit of the length member.
-// This means that strings longer than 2 Gigabytes can not be represented. If that turns out to be
-// a problem we can investigate alternative solutions.
+// This means that strings longer than 2 gigacharacters cannot be represented.
 
 class StringView {
 public:
-    StringView()
-        : m_characters(nullptr)
-        , m_length(0)
-    {
-    }
+    StringView();
+    ~StringView();
+    StringView(StringView&&);
+    StringView(const StringView&);
+    StringView& operator=(StringView&&);
+    StringView& operator=(const StringView&);
 
-    StringView(const LChar* characters, unsigned length)
-    {
-        initialize(characters, length);
-    }
+    StringView(const String&);
+    StringView(const StringImpl&);
+    StringView(const LChar*, unsigned length);
+    StringView(const UChar*, unsigned length);
 
-    StringView(const UChar* characters, unsigned length)
-    {
-        initialize(characters, length);
-    }
+    static StringView empty();
 
-    StringView(const StringImpl&);
-    StringView(const String&);
+    unsigned length() const;
+    bool isEmpty() const;
 
-    static StringView empty()
-    {
-        return StringView(reinterpret_cast<const LChar*>(""), 0);
-    }
+    explicit operator bool() const;
+    bool isNull() const;
 
-    const LChar* characters8() const
-    {
-        ASSERT(is8Bit());
+    UChar operator[](unsigned index) const;
 
-        return static_cast<const LChar*>(m_characters);
-    }
+    class CodeUnits;
+    CodeUnits codeUnits() const;
 
-    const UChar* characters16() const
-    {
-        ASSERT(!is8Bit());
+    class CodePoints;
+    CodePoints codePoints() const;
 
-        return static_cast<const UChar*>(m_characters);
-    }
+    bool is8Bit() const;
+    const LChar* characters8() const;
+    const UChar* characters16() const;
 
+    String toString() const;
+    String toStringWithoutCopying() const;
+
+#if USE(CF)
+    // This function converts null strings to empty strings.
+    WTF_EXPORT_STRING_API RetainPtr<CFStringRef> createCFStringWithoutCopying() const;
+#endif
+
+#ifdef __OBJC__
+    // These functions convert null strings to empty strings.
+    WTF_EXPORT_STRING_API RetainPtr<NSString> createNSString() const;
+    WTF_EXPORT_STRING_API RetainPtr<NSString> createNSStringWithoutCopying() const;
+#endif
+
+    class UpconvertedCharacters;
+    UpconvertedCharacters upconvertedCharacters() const;
+
     void getCharactersWithUpconvert(LChar*) const;
     void getCharactersWithUpconvert(UChar*) const;
 
-    class UpconvertedCharacters {
-    public:
-        explicit UpconvertedCharacters(const StringView&);
-        operator const UChar*() const { return m_characters; }
-        const UChar* get() const { return m_characters; }
-    private:
-        Vector<UChar, 32> m_upconvertedCharacters;
-        const UChar* m_characters;
-    };
-    UpconvertedCharacters upconvertedCharacters() const { return UpconvertedCharacters(*this); }
+    StringView substring(unsigned start, unsigned length = std::numeric_limits<unsigned>::max()) const;
 
-    bool isNull() const { return !m_characters; }
-    bool isEmpty() const { return !length(); }
-    unsigned length() const { return m_length & ~is16BitStringFlag; }
+    size_t find(UChar, unsigned start = 0) const;
+    bool contains(UChar) const;
 
-    explicit operator bool() const { return !isNull(); }
+    int toInt(bool& isValid) const;
+    float toFloat(bool& isValid) const;
 
-    bool is8Bit() const { return !(m_length & is16BitStringFlag); }
+    static void invalidate(const StringImpl&);
 
-    StringView substring(unsigned start, unsigned length = std::numeric_limits<unsigned>::max()) const
-    {
-        if (start >= this->length())
-            return empty();
-        unsigned maxLength = this->length() - start;
+    struct UnderlyingString;
 
-        if (length >= maxLength) {
-            if (!start)
-                return *this;
-            length = maxLength;
-        }
+private:
+    void initialize(const LChar*, unsigned length);
+    void initialize(const UChar*, unsigned length);
 
-        if (is8Bit())
-            return StringView(characters8() + start, length);
+    WTF_EXPORT_STRING_API bool underlyingStringIsValid() const;
+    WTF_EXPORT_STRING_API void setUnderlyingString(const StringImpl*);
+    WTF_EXPORT_STRING_API void setUnderlyingString(const StringView&);
 
-        return StringView(characters16() + start, length);
-    }
+    static const unsigned is16BitStringFlag = 1u << 31;
 
-    String toString() const;
+    const void* m_characters { nullptr };
+    unsigned m_length { 0 };
 
-    float toFloat(bool& isValid) const;
-    int toInt(bool& isValid) const;
+#if CHECK_STRINGVIEW_LIFETIME
+    void adoptUnderlyingString(UnderlyingString*);
+    UnderlyingString* m_underlyingString { nullptr };
+#endif
+};
 
-    String toStringWithoutCopying() const;
+template<typename CharacterType, size_t inlineCapacity> void append(Vector<CharacterType, inlineCapacity>&, StringView);
 
-    UChar operator[](unsigned index) const
-    {
-        ASSERT(index < length());
-        if (is8Bit())
-            return characters8()[index];
-        return characters16()[index];
-    }
+}
 
-    size_t find(UChar character, unsigned start = 0) const;
+#include <wtf/text/WTFString.h>
 
-    bool contains(UChar c) const { return find(c) != notFound; }
+namespace WTF {
 
-    class CodePoints;
-    class CodeUnits;
+inline StringView::StringView()
+{
+    // FIXME: It's peculiar that null strings are 16-bit and empty strings return 8-bit (according to the is8Bit function).
+}
 
-    CodePoints codePoints() const;
-    CodeUnits codeUnits() const;
+inline StringView::~StringView()
+{
+    setUnderlyingString(nullptr);
+}
 
-#if USE(CF)
-    // This function converts null strings to empty strings.
-    WTF_EXPORT_STRING_API RetainPtr<CFStringRef> createCFStringWithoutCopying() const;
-#endif
+inline StringView::StringView(StringView&& other)
+    : m_characters(other.m_characters)
+    , m_length(other.m_length)
+{
+    ASSERT(other.underlyingStringIsValid());
 
-#ifdef __OBJC__
-    // These functions convert null strings to empty strings.
-    WTF_EXPORT_STRING_API RetainPtr<NSString> createNSString() const;
-    WTF_EXPORT_STRING_API RetainPtr<NSString> createNSStringWithoutCopying() const;
-#endif
+    other.m_characters = nullptr;
+    other.m_length = 0;
 
-private:
-    void initialize(const LChar* characters, unsigned length)
-    {
-        ASSERT(!(length & is16BitStringFlag));
-        
-        m_characters = characters;
-        m_length = length;
-    }
+    setUnderlyingString(other);
+    other.setUnderlyingString(nullptr);
+}
 
-    void initialize(const UChar* characters, unsigned length)
-    {
-        ASSERT(!(length & is16BitStringFlag));
-        
-        m_characters = characters;
-        m_length = is16BitStringFlag | length;
-    }
+inline StringView::StringView(const StringView& other)
+    : m_characters(other.m_characters)
+    , m_length(other.m_length)
+{
+    ASSERT(other.underlyingStringIsValid());
 
-    static const unsigned is16BitStringFlag = 1u << 31;
+    setUnderlyingString(other);
+}
 
-    const void* m_characters;
-    unsigned m_length;
-};
+inline StringView& StringView::operator=(StringView&& other)
+{
+    ASSERT(other.underlyingStringIsValid());
 
+    m_characters = other.m_characters;
+    m_length = other.m_length;
+
+    other.m_characters = nullptr;
+    other.m_length = 0;
+
+    setUnderlyingString(other);
+    other.setUnderlyingString(nullptr);
+
+    return *this;
 }
 
-#include <wtf/text/WTFString.h>
+inline StringView& StringView::operator=(const StringView& other)
+{
+    ASSERT(other.underlyingStringIsValid());
 
-namespace WTF {
+    m_characters = other.m_characters;
+    m_length = other.m_length;
 
+    setUnderlyingString(other);
+
+    return *this;
+}
+
+inline void StringView::initialize(const LChar* characters, unsigned length)
+{
+    // FIXME: We need a better solution here, because there is no guarantee that
+    // the length here won't be too long. Maybe at least a RELEASE_ASSERT?
+    ASSERT(!(length & is16BitStringFlag));
+    m_characters = characters;
+    m_length = length;
+}
+
+inline void StringView::initialize(const UChar* characters, unsigned length)
+{
+    // FIXME: We need a better solution here, because there is no guarantee that
+    // the length here won't be too long. Maybe at least a RELEASE_ASSERT?
+    ASSERT(!(length & is16BitStringFlag));
+    m_characters = characters;
+    m_length = is16BitStringFlag | length;
+}
+
+inline StringView::StringView(const LChar* characters, unsigned length)
+{
+    initialize(characters, length);
+}
+
+inline StringView::StringView(const UChar* characters, unsigned length)
+{
+    initialize(characters, length);
+}
+
 inline StringView::StringView(const StringImpl& string)
 {
+    setUnderlyingString(&string);
     if (string.is8Bit())
         initialize(string.characters8(), string.length());
     else
@@ -194,6 +236,7 @@
 
 inline StringView::StringView(const String& string)
 {
+    setUnderlyingString(string.impl());
     if (!string.impl()) {
         m_characters = nullptr;
         m_length = 0;
@@ -206,55 +249,100 @@
     initialize(string.characters16(), string.length());
 }
 
-class StringView::CodePoints {
-public:
-    class Iterator {
-    public:
-        Iterator(const StringView&, unsigned index);
-        Iterator& operator++();
-        UChar32 operator*() const;
-        bool operator==(const Iterator&) const;
-        bool operator!=(const Iterator&) const;
+inline StringView StringView::empty()
+{
+    return StringView(reinterpret_cast<const LChar*>(""), 0);
+}
 
-    private:
-        const StringView& m_stringView;
-        mutable unsigned m_index;
-#if !ASSERT_DISABLED
-        mutable bool m_alreadyIncremented;
-#endif
-    };
+inline const LChar* StringView::characters8() const
+{
+    ASSERT(is8Bit());
+    ASSERT(underlyingStringIsValid());
+    return static_cast<const LChar*>(m_characters);
+}
 
-    explicit CodePoints(const StringView&);
-    Iterator begin() const;
-    Iterator end() const;
+inline const UChar* StringView::characters16() const
+{
+    ASSERT(!is8Bit());
+    ASSERT(underlyingStringIsValid());
+    return static_cast<const UChar*>(m_characters);
+}
 
+class StringView::UpconvertedCharacters {
+public:
+    explicit UpconvertedCharacters(const StringView&);
+    operator const UChar*() const { return m_characters; }
+    const UChar* get() const { return m_characters; }
 private:
-    StringView m_stringView;
+    Vector<UChar, 32> m_upconvertedCharacters;
+    const UChar* m_characters;
 };
 
-class StringView::CodeUnits {
-public:
-    class Iterator {
-    public:
-        Iterator(const StringView&, unsigned index);
-        Iterator& operator++();
-        UChar operator*() const;
-        bool operator==(const Iterator&) const;
-        bool operator!=(const Iterator&) const;
+inline StringView::UpconvertedCharacters StringView::upconvertedCharacters() const
+{
+    return UpconvertedCharacters(*this);
+}
 
-    private:
-        const StringView& m_stringView;
-        unsigned m_index;
-    };
+inline bool StringView::isNull() const
+{
+    return !m_characters;
+}
 
-    explicit CodeUnits(const StringView&);
-    Iterator begin() const;
-    Iterator end() const;
+inline bool StringView::isEmpty() const
+{
+    return !length();
+}
 
-private:
-    StringView m_stringView;
-};
+inline unsigned StringView::length() const
+{
+    return m_length & ~is16BitStringFlag;
+}
 
+inline StringView::operator bool() const
+{
+    return !isNull();
+}
+
+inline bool StringView::is8Bit() const
+{
+    return !(m_length & is16BitStringFlag);
+}
+
+inline StringView StringView::substring(unsigned start, unsigned length) const
+{
+    if (start >= this->length())
+        return empty();
+    unsigned maxLength = this->length() - start;
+
+    if (length >= maxLength) {
+        if (!start)
+            return *this;
+        length = maxLength;
+    }
+
+    if (is8Bit()) {
+        StringView result(characters8() + start, length);
+        result.setUnderlyingString(*this);
+        return result;
+    }
+    StringView result(characters16() + start, length);
+    result.setUnderlyingString(*this);
+    return result;
+}
+
+inline UChar StringView::operator[](unsigned index) const
+{
+    ASSERT(index < length());
+    if (is8Bit())
+        return characters8()[index];
+    return characters16()[index];
+}
+
+inline bool StringView::contains(UChar character) const
+{
+    return find(character) != notFound;
+}
+
 inline void StringView::getCharactersWithUpconvert(LChar* destination) const
 {
     ASSERT(is8Bit());
@@ -291,7 +379,6 @@
 {
     if (is8Bit())
         return String(characters8(), length());
-
     return String(characters16(), length());
 }
 
@@ -313,7 +400,6 @@
 {
     if (is8Bit())
         return StringImpl::createWithoutCopying(characters8(), length());
-
     return StringImpl::createWithoutCopying(characters16(), length());
 }
 
@@ -324,6 +410,27 @@
     return WTF::find(characters16(), length(), character, start);
 }
 
+#if !CHECK_STRINGVIEW_LIFETIME
+
+inline void StringView::invalidate(const StringImpl&)
+{
+}
+
+inline bool StringView::underlyingStringIsValid() const
+{
+    return true;
+}
+
+inline void StringView::setUnderlyingString(const StringImpl*)
+{
+}
+
+inline void StringView::setUnderlyingString(const StringView&)
+{
+}
+
+#endif
+
 template<typename StringType> class StringTypeAdapter;
 
 template<> class StringTypeAdapter<StringView> {
@@ -349,6 +456,63 @@
     string.getCharactersWithUpconvert(buffer.data() + oldSize);
 }
 
+class StringView::CodePoints {
+public:
+    explicit CodePoints(const StringView&);
+
+    class Iterator;
+    Iterator begin() const;
+    Iterator end() const;
+
+private:
+    StringView m_stringView;
+};
+
+class StringView::CodeUnits {
+public:
+    explicit CodeUnits(const StringView&);
+
+    class Iterator;
+    Iterator begin() const;
+    Iterator end() const;
+
+private:
+    StringView m_stringView;
+};
+
+class StringView::CodePoints::Iterator {
+public:
+    Iterator(const StringView&, unsigned index);
+
+    UChar32 operator*() const;
+    Iterator& operator++();
+
+    bool operator==(const Iterator&) const;
+    bool operator!=(const Iterator&) const;
+
+private:
+    const StringView& m_stringView;
+    mutable unsigned m_index;
+#if !ASSERT_DISABLED
+    mutable bool m_alreadyIncremented { false };
+#endif
+};
+
+class StringView::CodeUnits::Iterator {
+public:
+    Iterator(const StringView&, unsigned index);
+
+    UChar operator*() const;
+    Iterator& operator++();
+
+    bool operator==(const Iterator&) const;
+    bool operator!=(const Iterator&) const;
+
+private:
+    const StringView& m_stringView;
+    unsigned m_index;
+};
+
 inline auto StringView::codePoints() const -> CodePoints
 {
     return CodePoints(*this);
@@ -367,9 +531,6 @@
 inline StringView::CodePoints::Iterator::Iterator(const StringView& stringView, unsigned index)
     : m_stringView(stringView)
     , m_index(index)
-#if !ASSERT_DISABLED
-    , m_alreadyIncremented(false)
-#endif
 {
 }
 

Modified: trunk/Source/WebCore/ChangeLog (174387 => 174388)


--- trunk/Source/WebCore/ChangeLog	2014-10-07 06:54:48 UTC (rev 174387)
+++ trunk/Source/WebCore/ChangeLog	2014-10-07 06:58:41 UTC (rev 174388)
@@ -1,3 +1,13 @@
+2014-10-06  Darin Adler  <da...@apple.com>
+
+        Make StringView check the lifetime of the StringImpl it's created from
+        https://bugs.webkit.org/show_bug.cgi?id=137202
+
+        Reviewed by Anders Carlsson.
+
+        * platform/graphics/TextRun.cpp: Update since TextRun's definition now
+        uses a StringView.
+
 2014-10-06  Andy Estes  <aes...@apple.com>
 
         [iOS] Fix remaining misuses of abs() and fabsf()

Modified: trunk/Source/WebCore/platform/graphics/TextRun.cpp (174387 => 174388)


--- trunk/Source/WebCore/platform/graphics/TextRun.cpp	2014-10-07 06:54:48 UTC (rev 174387)
+++ trunk/Source/WebCore/platform/graphics/TextRun.cpp	2014-10-07 06:58:41 UTC (rev 174388)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2014 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -29,14 +29,14 @@
 namespace WebCore {
 
 struct ExpectedTextRunSize {
-    const void* pointer;
-    int integers[2];
+    void* renderingContext;
+    StringView text;
+    unsigned integer1;
+    unsigned integer2;
     float float1;
     float float2;
     float float3;
-    uint32_t bitfields : 10;
-    unsigned anUnsigned;
-    RefPtr<TextRun::RenderingContext> renderingContext;
+    unsigned bitfields : 9;
 };
 
 COMPILE_ASSERT(sizeof(TextRun) == sizeof(ExpectedTextRunSize), TextRun_is_not_of_expected_size);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to