- Revision
- 133272
- Author
- yu...@chromium.org
- Date
- 2012-11-02 01:06:31 -0700 (Fri, 02 Nov 2012)
Log Message
Memory instrumentation: do not call checkCountedObject with wrong pointers
https://bugs.webkit.org/show_bug.cgi?id=100958
Reviewed by Alexander Pavlov.
Source/WebCore:
Removed redundant call to checkCountedObject.
* inspector/MemoryInstrumentationImpl.cpp:
(WebCore::MemoryInstrumentationClientImpl::countObjectSize):
Source/WTF:
Removed calls to checkCountedObject from places where the pointer may contain
an address of a base class which may differ from the actual object pointer. Instead
checkCountedObject is only called right after processing deferred pointer where
we know real address.
* wtf/MemoryInstrumentation.h:
(WTF::MemoryInstrumentation::addObjectImpl):
(WTF::::process):
Tools:
* TestWebKitAPI/Tests/WTF/MemoryInstrumentationTest.cpp: test that if there
if there is class A derived from classes B and C and we have pointer B* to an
object of class A then MemoryInstrumentationClient::checkoutCountedObject won't
be called for B* but only for A*
Also MemoryInstrumentation implementation used in the test was refactored to
allow passing client as a parameter. The client implementation was extracted into
a top-level class MemoryInstrumentationTestClient.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (133271 => 133272)
--- trunk/Source/WTF/ChangeLog 2012-11-02 07:54:27 UTC (rev 133271)
+++ trunk/Source/WTF/ChangeLog 2012-11-02 08:06:31 UTC (rev 133272)
@@ -1,3 +1,19 @@
+2012-11-01 Yury Semikhatsky <yu...@chromium.org>
+
+ Memory instrumentation: do not call checkCountedObject with wrong pointers
+ https://bugs.webkit.org/show_bug.cgi?id=100958
+
+ Reviewed by Alexander Pavlov.
+
+ Removed calls to checkCountedObject from places where the pointer may contain
+ an address of a base class which may differ from the actual object pointer. Instead
+ checkCountedObject is only called right after processing deferred pointer where
+ we know real address.
+
+ * wtf/MemoryInstrumentation.h:
+ (WTF::MemoryInstrumentation::addObjectImpl):
+ (WTF::::process):
+
2012-11-01 Alexey Proskuryakov <a...@apple.com>
Rename HAVE(NETWORK_CFDATA_ARRAY_CALLBACK) to USE(NETWORK_CFDATA_ARRAY_CALLBACK)
Modified: trunk/Source/WTF/wtf/MemoryInstrumentation.h (133271 => 133272)
--- trunk/Source/WTF/wtf/MemoryInstrumentation.h 2012-11-02 07:54:27 UTC (rev 133271)
+++ trunk/Source/WTF/wtf/MemoryInstrumentation.h 2012-11-02 08:06:31 UTC (rev 133272)
@@ -230,7 +230,6 @@
} else {
if (!object || visited(object))
return;
- checkCountedObject(object);
deferInstrumentedPointer(adoptPtr(new InstrumentedPointer<T>(object, ownerObjectType)));
}
}
@@ -262,6 +261,7 @@
if (pointer != m_pointer && memoryInstrumentation->visited(pointer))
return;
memoryInstrumentation->countObjectSize(pointer, memoryObjectInfo.objectType(), memoryObjectInfo.objectSize());
+ memoryInstrumentation->checkCountedObject(pointer);
}
// Link time guard for classes with external memory instrumentation.
Modified: trunk/Source/WebCore/ChangeLog (133271 => 133272)
--- trunk/Source/WebCore/ChangeLog 2012-11-02 07:54:27 UTC (rev 133271)
+++ trunk/Source/WebCore/ChangeLog 2012-11-02 08:06:31 UTC (rev 133272)
@@ -1,3 +1,15 @@
+2012-11-01 Yury Semikhatsky <yu...@chromium.org>
+
+ Memory instrumentation: do not call checkCountedObject with wrong pointers
+ https://bugs.webkit.org/show_bug.cgi?id=100958
+
+ Reviewed by Alexander Pavlov.
+
+ Removed redundant call to checkCountedObject.
+
+ * inspector/MemoryInstrumentationImpl.cpp:
+ (WebCore::MemoryInstrumentationClientImpl::countObjectSize):
+
2012-11-02 Peter Wang <peter.w...@torchmobile.com.cn>
Web Inspector: a small defect in "WorkersSidebarPanel.js"
Modified: trunk/Source/WebCore/inspector/MemoryInstrumentationImpl.cpp (133271 => 133272)
--- trunk/Source/WebCore/inspector/MemoryInstrumentationImpl.cpp 2012-11-02 07:54:27 UTC (rev 133271)
+++ trunk/Source/WebCore/inspector/MemoryInstrumentationImpl.cpp 2012-11-02 08:06:31 UTC (rev 133272)
@@ -70,10 +70,8 @@
if (!checkInstrumentedObjects())
return;
- if (object) {
+ if (object)
m_countedObjects.add(object, size);
- checkCountedObject(object);
- }
}
bool MemoryInstrumentationClientImpl::visited(const void* object)
Modified: trunk/Tools/ChangeLog (133271 => 133272)
--- trunk/Tools/ChangeLog 2012-11-02 07:54:27 UTC (rev 133271)
+++ trunk/Tools/ChangeLog 2012-11-02 08:06:31 UTC (rev 133272)
@@ -1,3 +1,19 @@
+2012-11-01 Yury Semikhatsky <yu...@chromium.org>
+
+ Memory instrumentation: do not call checkCountedObject with wrong pointers
+ https://bugs.webkit.org/show_bug.cgi?id=100958
+
+ Reviewed by Alexander Pavlov.
+
+ * TestWebKitAPI/Tests/WTF/MemoryInstrumentationTest.cpp: test that if there
+ if there is class A derived from classes B and C and we have pointer B* to an
+ object of class A then MemoryInstrumentationClient::checkoutCountedObject won't
+ be called for B* but only for A*
+
+ Also MemoryInstrumentation implementation used in the test was refactored to
+ allow passing client as a parameter. The client implementation was extracted into
+ a top-level class MemoryInstrumentationTestClient.
+
2012-11-01 Christophe Dumez <christophe.du...@intel.com>
[EFL][WK2] Make File Chooser dialog modal in MiniBrowser
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/MemoryInstrumentationTest.cpp (133271 => 133272)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/MemoryInstrumentationTest.cpp 2012-11-02 07:54:27 UTC (rev 133271)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/MemoryInstrumentationTest.cpp 2012-11-02 08:06:31 UTC (rev 133272)
@@ -81,57 +81,65 @@
MemoryObjectType TestType = "TestType";
-class InstrumentationTestHelper : public WTF::MemoryInstrumentation {
+class MemoryInstrumentationTestClient : public WTF::MemoryInstrumentationClient {
public:
- InstrumentationTestHelper()
- : MemoryInstrumentation(&m_client)
- { }
+ virtual void countObjectSize(const void*, MemoryObjectType objectType, size_t size)
+ {
+ TypeToSizeMap::AddResult result = m_totalSizes.add(objectType, size);
+ if (!result.isNewEntry)
+ result.iterator->value += size;
+ }
+ virtual bool visited(const void* object) { return !m_visitedObjects.add(object).isNewEntry; }
+ virtual void checkCountedObject(const void*) { }
- virtual void processDeferredInstrumentedPointers();
- virtual void deferInstrumentedPointer(PassOwnPtr<InstrumentedPointerBase>);
+ size_t visitedObjects() const { return m_visitedObjects.size(); }
+ size_t totalSize(const MemoryObjectType objectType) const
+ {
+ TypeToSizeMap::const_iterator i = m_totalSizes.find(objectType);
+ return i == m_totalSizes.end() ? 0 : i->value;
+ }
- size_t visitedObjects() const { return m_client.visitedObjects(); }
- size_t reportedSizeForAllTypes() const { return m_client.reportedSizeForAllTypes(); }
- size_t totalSize(const MemoryObjectType objectType) const { return m_client.totalSize(objectType); }
+ size_t reportedSizeForAllTypes() const
+ {
+ size_t size = 0;
+ for (TypeToSizeMap::const_iterator i = m_totalSizes.begin(); i != m_totalSizes.end(); ++i)
+ size += i->value;
+ return size;
+ }
private:
- class Client : public WTF::MemoryInstrumentationClient {
- public:
- virtual void countObjectSize(const void*, MemoryObjectType objectType, size_t size)
- {
- TypeToSizeMap::AddResult result = m_totalSizes.add(objectType, size);
- if (!result.isNewEntry)
- result.iterator->value += size;
- }
- virtual bool visited(const void* object) { return !m_visitedObjects.add(object).isNewEntry; }
- virtual void checkCountedObject(const void*) { }
+ typedef HashMap<MemoryObjectType, size_t> TypeToSizeMap;
+ TypeToSizeMap m_totalSizes;
+ WTF::HashSet<const void*> m_visitedObjects;
+};
- size_t visitedObjects() const { return m_visitedObjects.size(); }
- size_t totalSize(const MemoryObjectType objectType) const
- {
- TypeToSizeMap::const_iterator i = m_totalSizes.find(objectType);
- return i == m_totalSizes.end() ? 0 : i->value;
- }
+class InstrumentationTestImpl : public WTF::MemoryInstrumentation {
+public:
+ explicit InstrumentationTestImpl(MemoryInstrumentationTestClient* client)
+ : MemoryInstrumentation(client)
+ , m_client(client) { }
- size_t reportedSizeForAllTypes() const
- {
- size_t size = 0;
- for (TypeToSizeMap::const_iterator i = m_totalSizes.begin(); i != m_totalSizes.end(); ++i)
- size += i->value;
- return size;
- }
+ virtual void processDeferredInstrumentedPointers();
+ virtual void deferInstrumentedPointer(PassOwnPtr<InstrumentedPointerBase>);
- private:
- typedef HashMap<MemoryObjectType, size_t> TypeToSizeMap;
- TypeToSizeMap m_totalSizes;
- WTF::HashSet<const void*> m_visitedObjects;
- };
+ size_t visitedObjects() const { return m_client->visitedObjects(); }
+ size_t reportedSizeForAllTypes() const { return m_client->reportedSizeForAllTypes(); }
+ size_t totalSize(const MemoryObjectType objectType) const { return m_client->totalSize(objectType); }
- Client m_client;
+private:
+ MemoryInstrumentationTestClient* m_client;
Vector<OwnPtr<InstrumentedPointerBase> > m_deferredInstrumentedPointers;
};
-void InstrumentationTestHelper::processDeferredInstrumentedPointers()
+class InstrumentationTestHelper : public InstrumentationTestImpl {
+public:
+ InstrumentationTestHelper() : InstrumentationTestImpl(&m_client) { }
+
+private:
+ MemoryInstrumentationTestClient m_client;
+};
+
+void InstrumentationTestImpl::processDeferredInstrumentedPointers()
{
while (!m_deferredInstrumentedPointers.isEmpty()) {
OwnPtr<InstrumentedPointerBase> pointer = m_deferredInstrumentedPointers.last().release();
@@ -140,7 +148,7 @@
}
}
-void InstrumentationTestHelper::deferInstrumentedPointer(PassOwnPtr<InstrumentedPointerBase> pointer)
+void InstrumentationTestImpl::deferInstrumentedPointer(PassOwnPtr<InstrumentedPointerBase> pointer)
{
m_deferredInstrumentedPointers.append(pointer);
}
@@ -789,7 +797,6 @@
}
};
-
TEST(MemoryInstrumentationTest, instrumentedWithMultipleAncestors)
{
InstrumentationTestHelper helper;
@@ -799,11 +806,43 @@
Instrumented* ancestorPointer = descendantPointer;
InstrumentedOwner<Instrumented*> ancestorPointerOwner(ancestorPointer);
EXPECT_NE(static_cast<void*>(ancestorPointer), static_cast<void*>(descendantPointer));
+
helper.addRootObject(descendantPointerOwner);
helper.addRootObject(ancestorPointerOwner);
EXPECT_EQ(sizeof(ClassWithTwoAncestors), helper.reportedSizeForAllTypes());
EXPECT_EQ(2u, helper.visitedObjects());
}
+class CheckCountedObjectsClient : public MemoryInstrumentationTestClient {
+public:
+ CheckCountedObjectsClient(const void* expectedPointer) : m_expectedPointer(expectedPointer), m_expectedPointerFound(false) { }
+ virtual void checkCountedObject(const void* pointer)
+ {
+ EXPECT_EQ(pointer, m_expectedPointer);
+ m_expectedPointerFound = true;
+ }
+ bool expectedPointerFound() { return m_expectedPointerFound; }
+
+private:
+ const void* m_expectedPointer;
+ bool m_expectedPointerFound;
+};
+
+TEST(MemoryInstrumentationTest, checkCountedObjectWithMultipleAncestors)
+{
+ OwnPtr<ClassWithTwoAncestors> instance = adoptPtr(new ClassWithTwoAncestors());
+ ClassWithTwoAncestors* descendantPointer = instance.get();
+ InstrumentedOwner<ClassWithTwoAncestors*> descendantPointerOwner(descendantPointer);
+ Instrumented* ancestorPointer = descendantPointer;
+ InstrumentedOwner<Instrumented*> ancestorPointerOwner(ancestorPointer);
+ EXPECT_NE(static_cast<void*>(ancestorPointer), static_cast<void*>(descendantPointer));
+
+ CheckCountedObjectsClient client(instance.get());
+ InstrumentationTestImpl instrumentation(&client);
+ instrumentation.addRootObject(descendantPointerOwner);
+ instrumentation.addRootObject(ancestorPointerOwner);
+ EXPECT_TRUE(client.expectedPointerFound());
+}
+
} // namespace
