- Revision
- 130581
- Author
- [email protected]
- Date
- 2012-10-06 02:26:27 -0700 (Sat, 06 Oct 2012)
Log Message
Web Inspector: NMI fix String instrumentation the way it was discussed in WK97964
https://bugs.webkit.org/show_bug.cgi?id=98500
Reviewed by Benjamin Poulain.
Current instrumentation incorrectly covers the case when StringImpl object has been created via StringImpl::createWithTerminatingNullCharacter().
Looks like the only way to detect the strings that has been created from literals is to compare the addresses of buffer and stringImpl + 1.
Source/WebKit/chromium:
* tests/MemoryInstrumentationTest.cpp:
Source/WTF:
* wtf/MemoryInstrumentationString.h:
(WTF::reportMemoryUsage):
* wtf/text/StringImpl.h:
(WTF::StringImpl::hasInternalBuffer):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (130580 => 130581)
--- trunk/Source/WTF/ChangeLog 2012-10-06 09:03:57 UTC (rev 130580)
+++ trunk/Source/WTF/ChangeLog 2012-10-06 09:26:27 UTC (rev 130581)
@@ -1,3 +1,18 @@
+2012-10-06 Ilya Tikhonovsky <[email protected]>
+
+ Web Inspector: NMI fix String instrumentation the way it was discussed in WK97964
+ https://bugs.webkit.org/show_bug.cgi?id=98500
+
+ Reviewed by Benjamin Poulain.
+
+ Current instrumentation incorrectly covers the case when StringImpl object has been created via StringImpl::createWithTerminatingNullCharacter().
+ Looks like the only way to detect the strings that has been created from literals is to compare the addresses of buffer and stringImpl + 1.
+
+ * wtf/MemoryInstrumentationString.h:
+ (WTF::reportMemoryUsage):
+ * wtf/text/StringImpl.h:
+ (WTF::StringImpl::hasInternalBuffer):
+
2012-10-06 Benjamin Poulain <[email protected]>
Fix build of WTFURL after r130187
Modified: trunk/Source/WTF/wtf/MemoryInstrumentationString.h (130580 => 130581)
--- trunk/Source/WTF/wtf/MemoryInstrumentationString.h 2012-10-06 09:03:57 UTC (rev 130580)
+++ trunk/Source/WTF/wtf/MemoryInstrumentationString.h 2012-10-06 09:26:27 UTC (rev 130581)
@@ -43,24 +43,24 @@
{
size_t selfSize = sizeof(StringImpl);
+ size_t length = stringImpl->length() + (stringImpl->hasTerminatingNullCharacter() ? 1 : 0);
+ size_t bufferSize = length * (stringImpl->is8Bit() ? sizeof(LChar) : sizeof(UChar));
+ const void* buffer = stringImpl->is8Bit() ? static_cast<const void*>(stringImpl->characters8()) : static_cast<const void*>(stringImpl->characters16());
+
// Count size used by internal buffer but skip strings that were constructed from literals.
- if (stringImpl->hasInternalBuffer())
- selfSize += stringImpl->length() * (stringImpl->is8Bit() ? sizeof(LChar) : sizeof(UChar));
+ if (stringImpl->hasInternalBuffer() && buffer == stringImpl + 1)
+ selfSize += bufferSize;
MemoryClassInfo info(memoryObjectInfo, stringImpl, 0, selfSize);
if (StringImpl* baseString = stringImpl->baseString())
info.addMember(baseString);
else {
- if (stringImpl->hasOwnedBuffer()) {
- if (stringImpl->is8Bit())
- info.addRawBuffer(stringImpl->characters8(), stringImpl->length());
- else
- info.addRawBuffer(stringImpl->characters16(), stringImpl->length() * 2);
- }
+ if (stringImpl->hasOwnedBuffer())
+ info.addRawBuffer(buffer, bufferSize);
if (stringImpl->has16BitShadow())
- info.addRawBuffer(stringImpl->characters(), (stringImpl->length() + (stringImpl->hasTerminatingNullCharacter() ? 1 : 0)) * sizeof(UChar));
+ info.addRawBuffer(stringImpl->characters(), length * sizeof(UChar));
}
}
Modified: trunk/Source/WTF/wtf/text/StringImpl.h (130580 => 130581)
--- trunk/Source/WTF/wtf/text/StringImpl.h 2012-10-06 09:03:57 UTC (rev 130580)
+++ trunk/Source/WTF/wtf/text/StringImpl.h 2012-10-06 09:26:27 UTC (rev 130581)
@@ -453,7 +453,7 @@
unsigned length() const { return m_length; }
bool is8Bit() const { return m_hashAndFlags & s_hashFlag8BitBuffer; }
- bool hasInternalBuffer() const { return bufferOwnership() == BufferInternal && !hasTerminatingNullCharacter(); }
+ bool hasInternalBuffer() const { return bufferOwnership() == BufferInternal; }
bool hasOwnedBuffer() const { return bufferOwnership() == BufferOwned; }
StringImpl* baseString() const { return bufferOwnership() == BufferSubstring ? m_substringBuffer : 0; }
Modified: trunk/Source/WebKit/chromium/ChangeLog (130580 => 130581)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-10-06 09:03:57 UTC (rev 130580)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-10-06 09:26:27 UTC (rev 130581)
@@ -1,3 +1,15 @@
+2012-10-06 Ilya Tikhonovsky <[email protected]>
+
+ Web Inspector: NMI fix String instrumentation the way it was discussed in WK97964
+ https://bugs.webkit.org/show_bug.cgi?id=98500
+
+ Reviewed by Benjamin Poulain.
+
+ Current instrumentation incorrectly covers the case when StringImpl object has been created via StringImpl::createWithTerminatingNullCharacter().
+ Looks like the only way to detect the strings that has been created from literals is to compare the addresses of buffer and stringImpl + 1.
+
+ * tests/MemoryInstrumentationTest.cpp:
+
2012-10-05 Terry Anderson <[email protected]>
[chromium] Only define SK_SUPPORT_HINTING_SCALE_FACTOR when building for chromeOS
Modified: trunk/Source/WebKit/chromium/tests/MemoryInstrumentationTest.cpp (130580 => 130581)
--- trunk/Source/WebKit/chromium/tests/MemoryInstrumentationTest.cpp 2012-10-06 09:03:57 UTC (rev 130580)
+++ trunk/Source/WebKit/chromium/tests/MemoryInstrumentationTest.cpp 2012-10-06 09:26:27 UTC (rev 130581)
@@ -295,7 +295,7 @@
InstrumentedOwner<String> stringInstrumentedOwner("String");
stringInstrumentedOwner.m_value.characters();
helper.addRootObject(stringInstrumentedOwner);
- EXPECT_EQ(sizeof(StringImpl) + stringInstrumentedOwner.m_value.length() * 3, helper.reportedSizeForAllTypes());
+ EXPECT_EQ(sizeof(StringImpl) + stringInstrumentedOwner.m_value.length() * (sizeof(LChar) + sizeof(UChar)), helper.reportedSizeForAllTypes());
EXPECT_EQ(2, helper.visitedObjects());
}
@@ -304,7 +304,7 @@
String string("String");
InstrumentedOwner<String> stringInstrumentedOwner(String(string.characters(), string.length()));
helper.addRootObject(stringInstrumentedOwner);
- EXPECT_EQ(sizeof(StringImpl) + stringInstrumentedOwner.m_value.length() * 2, helper.reportedSizeForAllTypes());
+ EXPECT_EQ(sizeof(StringImpl) + stringInstrumentedOwner.m_value.length() * sizeof(UChar), helper.reportedSizeForAllTypes());
EXPECT_EQ(1, helper.visitedObjects());
}
@@ -317,13 +317,22 @@
EXPECT_EQ(1, helper.visitedObjects());
}
+ { // Zero terminated internal buffer.
+ InstrumentationTestHelper helper;
+ InstrumentedOwner<String> stringInstrumentedOwner("string");
+ stringInstrumentedOwner.m_value.charactersWithNullTermination();
+ helper.addRootObject(stringInstrumentedOwner);
+ EXPECT_EQ(sizeof(StringImpl) + (stringInstrumentedOwner.m_value.length() + 1) * (sizeof(LChar) + sizeof(UChar)), helper.reportedSizeForAllTypes());
+ EXPECT_EQ(2, helper.visitedObjects());
+ }
+
{ // Substring
InstrumentationTestHelper helper;
String baseString("String");
baseString.characters(); // Force 16 shadow creation.
InstrumentedOwner<String> stringInstrumentedOwner(baseString.substringSharingImpl(1, 4));
helper.addRootObject(stringInstrumentedOwner);
- EXPECT_EQ(sizeof(StringImpl) * 2 + baseString.length() * 3, helper.reportedSizeForAllTypes());
+ EXPECT_EQ(sizeof(StringImpl) * 2 + baseString.length() * (sizeof(LChar) + sizeof(UChar)), helper.reportedSizeForAllTypes());
EXPECT_EQ(3, helper.visitedObjects());
}
@@ -341,7 +350,7 @@
InstrumentedOwner<AtomicString> atomicStringInstrumentedOwner("AtomicString");
atomicStringInstrumentedOwner.m_value.string().characters(); // Force 16bit shadow creation.
helper.addRootObject(atomicStringInstrumentedOwner);
- EXPECT_EQ(sizeof(StringImpl) + atomicStringInstrumentedOwner.m_value.length() * 3, helper.reportedSizeForAllTypes());
+ EXPECT_EQ(sizeof(StringImpl) + atomicStringInstrumentedOwner.m_value.length() * (sizeof(LChar) + sizeof(UChar)), helper.reportedSizeForAllTypes());
EXPECT_EQ(2, helper.visitedObjects());
}
