[webkit-reviews] review granted: [Bug 70861] [MutationObservers] Support attributeOldValue for attribute mutations : [Attachment 112799] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Adam Klein <adamk at chromium.org>'s
request for review:
Bug 70861: [MutationObservers] Support attributeOldValue for attribute
mutations
https://bugs.webkit.org/show_bug.cgi?id=70861

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=112799&action=review


> Source/WebCore/dom/Element.cpp:642
> +    RefPtr<MutationRecord> mutation =
MutationRecord::createAttributes(element, name, isOldValueRequested(observers)
? oldValue : nullAtom);
> +    RefPtr<MutationRecord> mutationWithNullOldValue =
MutationRecord::createWithNullOldValue(mutation);

It'll be nice if we could lazily allocate these objects since they live in heap
but that might be a pre-mature optimization given createWithNullOldValue only
stores a pointer.

> LayoutTests/fast/mutation/observe-attributes.html:346
> +	   observer = new WebKitMutationObserver(function(mutations) {
> +	       window.mutations = mutations;
> +	   });
> +	   observer2 = new WebKitMutationObserver(function(mutations) {
> +	       window.mutations2 = mutations;
> +	   });

Can we give more descriptive names to observer and observer2?

> LayoutTests/fast/mutation/observe-attributes.html:358
> +	   shouldBe('mutations[0].oldValue', '"bar"');
> +	   shouldBe('mutations2.length', '1');

Ditto on mutations vs. mutations2.