[webkit-reviews] review granted: [Bug 190115] GC can collect JS wrappers of nodes in the mutation records waiting to be delivered : [Attachment 351318] Fixes the bug

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 190115: GC can collect JS wrappers of nodes in the mutation records waiting
to be delivered
https://bugs.webkit.org/show_bug.cgi?id=190115

Attachment 351318: Fixes the bug

https://bugs.webkit.org/attachment.cgi?id=351318&action=review




--- Comment #12 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 351318
  --> https://bugs.webkit.org/attachment.cgi?id=351318
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=351318&action=review

r=me

>> Source/WebCore/dom/MutationObserver.cpp:118
>> +	// between the time takeRecords are called and nodes in records are
accesssed.
> 
> correct => collect
> are => is
> 
> What will access the nodes referenced by takeRecords(), and what guarantees
that those node wrappers will remain valid after MutationObserver::disconnect()
or MutationObserver::deliver()?

I think a nice follow-up would be to change takeRecords() to be a custom
binding, have it return a pair of the vector of mutation records and the hash
table of pending targets, and have the caller retain the hash table of pending
targets until conversion to a JS array is complete.