Changeset 1912

Timestamp:

08/24/02 23:41:13 (6 years ago)

Author:

darin

Message:

• fixed 3032072 -- Crash on reload in DOM::DocumentImpl::~DocumentImpl [unified]()

This was a tough one to debug.
Turned out to be a subtle bug in our new QConstString implementation.

• kwq/KWQString.mm: (QStringData::makeAscii): Don't invalidate the Unicode buffer unless the ASCII buffer is taking over the internal buffer from the Unicode. In other cases, it's important not to invalidate the Unicode because it can result in the Unicode being freed and re-created when unicode() is called, which is disastrous in the QConstString case because we can't free the Unicode. (QStringData::makeUnicode): Make the corresponding change for ASCII too. In this case, it's just an optimization, not a bug fix. (QString::insert), (QString::remove), (QString::fill), (QString::operator+=): Mark the other string invalid whenever we modify either the ASCII or the Unicode string. This was handled correctly for some operations before, but not all, and it now matters because makeAscii and makeUnicode will now allow this state.

Fixed a separate QString problem; I think I may have introduced this one.

• kwq/KWQString.mm: (QString::getNSString): Since by ASCII, we actually mean ISO Latin 1, we can't use [NSString stringWithCString:]. Use CFStringCreateWithCString instead, pass CFStringCreateWithCString as the encoding, and use autorelease. We shouldn't really use the term ASCII in this class for the 8-bit-per-character buffer. Something more like Latin1 would be a more accurate way to refer to it. Maybe I'll do that renaming after talking to Richard about it on Monday.

Fixed a small storage leak.

• khtml/css/css_valueimpl.cpp: (CSSPrimitiveValueImpl::cleanup): Added braces to fix obviously-incorrect if/else grouping.

Location:

trunk/WebCore

Files:

• ChangeLog-2002-12-03 (modified) (1 diff)
• ChangeLog-2003-10-25 (modified) (1 diff)
• ChangeLog-2005-08-23 (modified) (1 diff)
• khtml/css/css_valueimpl.cpp (modified) (1 diff)
• kwq/KWQString.mm (modified) (13 diffs)

trunk/WebCore/ChangeLog-2002-12-03

@@ +1 @@
+2002-08-24  Darin Adler  <darin@apple.com>
+
+        - fixed 3032072 -- Crash on reload in DOM::DocumentImpl::~DocumentImpl [unified]()
+
+        This was a tough one to debug.
+        Turned out to be a subtle bug in our new QConstString implementation.
+
+        * kwq/KWQString.mm:
+        (QStringData::makeAscii): Don't invalidate the Unicode buffer unless the ASCII
+        buffer is taking over the internal buffer from the Unicode. In other cases, it's
+        important not to invalidate the Unicode because it can result in the Unicode being
+        freed and re-created when unicode() is called, which is disastrous in the QConstString
+        case because we can't free the Unicode.
+        (QStringData::makeUnicode): Make the corresponding change for ASCII too. In this
+        case, it's just an optimization, not a bug fix.
+        (QString::insert), (QString::remove), (QString::fill), (QString::operator+=):
+        Mark the other string invalid whenever we modify either the ASCII or the Unicode
+        string. This was handled correctly for some operations before, but not all, and it
+        now matters because makeAscii and makeUnicode will now allow this state.
+
+        Fixed a separate QString problem; I think I may have introduced this one.
+
+        * kwq/KWQString.mm:
+        (QString::getNSString): Since by ASCII, we actually mean ISO Latin 1, we can't use
+        [NSString stringWithCString:]. Use CFStringCreateWithCString instead, pass
+        CFStringCreateWithCString as the encoding, and use autorelease. We shouldn't really
+        use the term ASCII in this class for the 8-bit-per-character buffer. Something more
+        like Latin1 would be a more accurate way to refer to it. Maybe I'll do that renaming
+        after talking to Richard about it on Monday.
+
+        Fixed a small storage leak.
+
+        * khtml/css/css_valueimpl.cpp: (CSSPrimitiveValueImpl::cleanup): Added braces to
+        fix obviously-incorrect if/else grouping.
+
 2002-08-23  Darin Adler  <darin@apple.com>
 

trunk/WebCore/ChangeLog-2003-10-25

@@ +1 @@
+2002-08-24  Darin Adler  <darin@apple.com>
+
+        - fixed 3032072 -- Crash on reload in DOM::DocumentImpl::~DocumentImpl [unified]()
+
+        This was a tough one to debug.
+        Turned out to be a subtle bug in our new QConstString implementation.
+
+        * kwq/KWQString.mm:
+        (QStringData::makeAscii): Don't invalidate the Unicode buffer unless the ASCII
+        buffer is taking over the internal buffer from the Unicode. In other cases, it's
+        important not to invalidate the Unicode because it can result in the Unicode being
+        freed and re-created when unicode() is called, which is disastrous in the QConstString
+        case because we can't free the Unicode.
+        (QStringData::makeUnicode): Make the corresponding change for ASCII too. In this
+        case, it's just an optimization, not a bug fix.
+        (QString::insert), (QString::remove), (QString::fill), (QString::operator+=):
+        Mark the other string invalid whenever we modify either the ASCII or the Unicode
+        string. This was handled correctly for some operations before, but not all, and it
+        now matters because makeAscii and makeUnicode will now allow this state.
+
+        Fixed a separate QString problem; I think I may have introduced this one.
+
+        * kwq/KWQString.mm:
+        (QString::getNSString): Since by ASCII, we actually mean ISO Latin 1, we can't use
+        [NSString stringWithCString:]. Use CFStringCreateWithCString instead, pass
+        CFStringCreateWithCString as the encoding, and use autorelease. We shouldn't really
+        use the term ASCII in this class for the 8-bit-per-character buffer. Something more
+        like Latin1 would be a more accurate way to refer to it. Maybe I'll do that renaming
+        after talking to Richard about it on Monday.
+
+        Fixed a small storage leak.
+
+        * khtml/css/css_valueimpl.cpp: (CSSPrimitiveValueImpl::cleanup): Added braces to
+        fix obviously-incorrect if/else grouping.
+
 2002-08-23  Darin Adler  <darin@apple.com>
 

trunk/WebCore/ChangeLog-2005-08-23

@@ +1 @@
+2002-08-24  Darin Adler  <darin@apple.com>
+
+        - fixed 3032072 -- Crash on reload in DOM::DocumentImpl::~DocumentImpl [unified]()
+
+        This was a tough one to debug.
+        Turned out to be a subtle bug in our new QConstString implementation.
+
+        * kwq/KWQString.mm:
+        (QStringData::makeAscii): Don't invalidate the Unicode buffer unless the ASCII
+        buffer is taking over the internal buffer from the Unicode. In other cases, it's
+        important not to invalidate the Unicode because it can result in the Unicode being
+        freed and re-created when unicode() is called, which is disastrous in the QConstString
+        case because we can't free the Unicode.
+        (QStringData::makeUnicode): Make the corresponding change for ASCII too. In this
+        case, it's just an optimization, not a bug fix.
+        (QString::insert), (QString::remove), (QString::fill), (QString::operator+=):
+        Mark the other string invalid whenever we modify either the ASCII or the Unicode
+        string. This was handled correctly for some operations before, but not all, and it
+        now matters because makeAscii and makeUnicode will now allow this state.
+
+        Fixed a separate QString problem; I think I may have introduced this one.
+
+        * kwq/KWQString.mm:
+        (QString::getNSString): Since by ASCII, we actually mean ISO Latin 1, we can't use
+        [NSString stringWithCString:]. Use CFStringCreateWithCString instead, pass
+        CFStringCreateWithCString as the encoding, and use autorelease. We shouldn't really
+        use the term ASCII in this class for the 8-bit-per-character buffer. Something more
+        like Latin1 would be a more accurate way to refer to it. Maybe I'll do that renaming
+        after talking to Richard about it on Monday.
+
+        Fixed a small storage leak.
+
+        * khtml/css/css_valueimpl.cpp: (CSSPrimitiveValueImpl::cleanup): Added braces to
+        fix obviously-incorrect if/else grouping.
+
 2002-08-23  Darin Adler  <darin@apple.com>
 

trunk/WebCore/khtml/css/css_valueimpl.cpp

@@ -372 +372 @@
     { }
     else if(m_type < CSSPrimitiveValue::CSS_COUNTER)
-        if(m_value.string) m_value.string->deref();
+        { if(m_value.string) m_value.string->deref(); }
     else if(m_type == CSSPrimitiveValue::CSS_COUNTER)
         m_value.counter->deref();

trunk/WebCore/kwq/KWQString.mm

@@ -596 +596 @@
                     *tp++ = *fp++;
                 str = &copyBuf[0];
+                _isUnicodeValid = 0;
             }
             else
@@ -616 +617 @@
         *cp = 0;
         
-        _isUnicodeValid = 0;
         _isAsciiValid = 1;
     }
@@ -644 +644 @@
                     *tp++ = *fp++;
                 str = &copyBuf[0];
+                _isAsciiValid = 0;
             }
             else
@@ -663 +664 @@
         
         _isUnicodeValid = 1;
-        _isAsciiValid = 0;
     }
     else if (!_isUnicodeValid)
@@ -778 +778 @@
     
     if (dataHandle[0]->_isAsciiValid) {
-        return [NSString stringWithCString:(const char *)ascii()];
+        return [(NSString *)CFStringCreateWithCString(kCFAllocatorDefault, ascii(), kCFStringEncodingISOLatin1) autorelease];
     }
     
@@ -2062 +2062 @@
         // Insert characters.
         memcpy (targetChars+index, insertChars, insertLength);
+        
+        dataHandle[0]->_isUnicodeValid = 0;
     }
     else if (dataHandle[0]->_isUnicodeValid){
@@ -2128 +2130 @@
         }
         
+        dataHandle[0]->_isAsciiValid = 0;
     }
     
@@ -2153 +2156 @@
         targetChars[index] = insertChar;
         targetChars[dataHandle[0]->_length] = 0;
+
+        dataHandle[0]->_isUnicodeValid = 0;
     }
     else {
@@ -2191 +2196 @@
         targetChars[index] = ch;
         targetChars[dataHandle[0]->_length] = 0;
+
+        dataHandle[0]->_isUnicodeValid = 0;
     }
     else if (dataHandle[0]->_isUnicodeValid){
@@ -2292 +2299 @@
                     sizeof(char)*(olen-index-len) );
             setLength( olen-len );
+            dataHandle[0]->_isUnicodeValid = 0;
         }
         else if (dataHandle[0]->_isUnicodeValid){
@@ -2417 +2425 @@
             while (len--) 
                 *nd++ = (char)qc;
+            dataHandle[0]->_isUnicodeValid = 0;
         }
         else {
@@ -2461 +2470 @@
             QSTRING_FAILURE("invalid character cache");
         dataHandle[0]->_length += qs.data()->_length;
+        dataHandle[0]->_isAsciiValid = 0;
         return *this;
     }
@@ -2471 +2481 @@
         *tp = 0;
         dataHandle[0]->_length += qs.data()->_length;
+        dataHandle[0]->_isUnicodeValid = 0;
         return *this;
     }