[webkit-reviews] review granted: [Bug 21609] Make MessagePorts protect their peers across heaps : [Attachment 24358] proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 15 09:10:09 PDT 2008


Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 21609: Make MessagePorts protect their peers across heaps
https://bugs.webkit.org/show_bug.cgi?id=21609

Attachment 24358: proposed patch
https://bugs.webkit.org/attachment.cgi?id=24358&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+ objects in the heap, makin git possible to implement cross-heap dependencies.


Misplaces space here "makin git".

+ void markCrossHeapDependentObjectsForDocument(JSGlobalData& globalData,
Document* doc)

I'm slightly annoyed that we sometimes use references and other times pointers
for JSGlobalData. Which style do you think is preferred? I also prefer document
as the name for a document argument rather than doc.

It's a shame that m_jsWrapperIsKnownInaccessible is such a long name, but not
correct grammar. The "known inaccessible" part isn't good grammar. I don't have
a better name to suggest, but I think it's worth pondering a little more. Maybe
just "KnownToBeInaccessible"?

+ bool jsWrapperIsKnownInaccessible() { return m_jsWrapperIsKnownInaccessible;
}

Should be const?

+    JSGlobalObject* globalObject = m_globalData->head;
+    do {
+	 globalObject->markCrossHeapDependentObjects();
+	 globalObject = globalObject->next();
+    } while (globalObject != m_globalData->head);

The loop is just complicated enough that I think it should either be in its own
function, or paragraphed separately from the other marking work.

r=me


More information about the webkit-reviews mailing list