Don't queue mutation records when attributes don't change value.

Don't queue mutation records when attributes don't change value.

There's no reason to queue records when an attribute didn't really
change value.

R=ojan@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/089b8f46de8985d4389ad3a06a74ab7f43212882

[ojan, 2015-01-17 03:21:21]

lgtm

This makes sense to me, but I vaguely remember there was a reason this wasn't
done originally. Raf/Adam, do you remember?

[esprehn, 2015-01-17 03:21:32]

Committed patchset #1 (id:1) manually as
089b8f46de8985d4389ad3a06a74ab7f43212882 (presubmit successful).

[adamk, 2015-01-20 19:55:52]

adamk@chromium.org changed reviewers:
+ adamk@chromium.org

[adamk, 2015-01-20 19:55:52]

I don't think there was any big reason not to do this, though in the case of
something like:

{ attributes: true, attributeOldValue: false }

speccing this behavior could have been expensive for "style" attributes.
Basically, emitting mutation records in cases like this just kept the spec
simpler. But Sky should do whatever it pleases.

[rafaelw, 2015-01-21 02:25:47]

rafaelw@chromium.org changed reviewers:
+ rafaelw@chromium.org

[rafaelw, 2015-01-21 02:25:48]

Yup. This was largely about style not always being stored as a serialized
string. If it's not, you'd prefer not to have to serialize the old state and the
new state just to compare whether they are identical and not emit a mutation
record. Also, given that style is parsed, the attribute string values could be
different and the parsed representation be equivalent.

I'm curious what practical win this change accomplishes.

[esprehn, 2015-01-21 02:55:28]

On 2015/01/21 at 02:25:48, rafaelw wrote:
> Yup. This was largely about style not always being stored as a serialized
string. If it's not, you'd prefer not to have to serialize the old state and the
new state just to compare whether they are identical and not emit a mutation
record. Also, given that style is parsed, the attribute string values could be
different and the parsed representation be equivalent.
> 
> I'm curious what practical win this change accomplishes.

Less code, and less complexity inside frameworks. I would not expect that
calling setAttribute() with identical values would malloc a new MutationRecord,
queue a micro task, and run all the code related to it. Note that
attributeChangedCallback already compares the old/new values and doesn't fire if
they're the same. We always create the serialized style value in blink inside
setAttribute(), so this wasn't actually saving any work.

Do you mean there's a theoretical optimization that this avoids having to
serialize the style attr?