[webkit-reviews] review granted: [Bug 70862] [MutationObservers] Support characterDataOldValue for characterData mutations : [Attachment 113077] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Adam Klein <adamk at chromium.org>'s
request for review:
Bug 70862: [MutationObservers] Support characterDataOldValue for characterData
mutations
https://bugs.webkit.org/show_bug.cgi?id=70862

Attachment 113077: Patch
https://bugs.webkit.org/attachment.cgi?id=113077&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=113077&action=review


> Source/WebCore/dom/CharacterData.cpp:216
> +    RefPtr<MutationRecord> mutation =
MutationRecord::createCharacterData(node, isOldValueRequested(observers) ?
oldData : 0);

Can we create this lazily in the for loop below? That would obviate the need
for the isOldValueRequested and would avoid doing a second loop over the
observers.

I guess you're trying to have a single record for all observers. I think that's
fine, but you could invert the way you create mutations...create the null old
value record first and create the mutation record with the old value from the
one without.

It's not a big deal, but it seems the code is less complicated (or am I missing
something?) and it avoids another O(n) operation, not that I actually expect
that to affect real-world performance.