ChangeLog

2011-11-23  Rafael Weinstein  <rafaelw@chromium.org>

        [MutationObservers] Modifications to the style property don't dispatch "attributes" Mutation Records

        https://bugs.webkit.org/show_bug.cgi?id=70137

        Reviewed by NOBODY (OOPS!).

        This patch adds a private AttributesMutationScope mechanism to CSSMutableStyleDeclaration (which uses

        the RAII pattern similar to the public ChildListMutationScope). This manages the (sometimes conditional)

        pre-change serialization of the style attribute (if an observer has requested oldValue), creation of

        the mutation record, and dispatch if the declaration was actual affected.

        * css/CSSMutableStyleDeclaration.cpp:

        (WebCore::CSSMutableStyleDeclaration::removeProperty):

        (WebCore::CSSMutableStyleDeclaration::setProperty):

        (WebCore::CSSMutableStyleDeclaration::setPropertyInternal):

        (WebCore::CSSMutableStyleDeclaration::parseDeclaration):

        (WebCore::CSSMutableStyleDeclaration::addParsedProperties):

        (WebCore::CSSMutableStyleDeclaration::addParsedProperty):

        (WebCore::CSSMutableStyleDeclaration::setCssText):

        (WebCore::CSSMutableStyleDeclaration::merge):

        (WebCore::CSSMutableStyleDeclaration::removePropertiesInSet):

        * dom/Element.cpp:

        (WebCore::Element::setAttribute):

2011-11-23  Yury Semikhatsky  <yurys@chromium.org>

        Unreviewed. Build fix. Added missing ENABLE(WORKERS) guards.

Source/WebCore/css/CSSMutableStyleDeclaration.cpp

#include "CSSValueList.h"

#include "Document.h"

#include "ExceptionCode.h"

35#include "HTMLNames.h"

3536#include "InspectorInstrumentation.h"

37#include "MutationRecord.h"

3638#include "StyledElement.h"

39#include "WebKitMutationObserver.h"

#include <wtf/text/StringBuilder.h>

#include <wtf/text/WTFString.h>

@@using namespace std;

namespace WebCore {

#if ENABLE(MUTATION_OBSERVERS)

namespace {

class StyleAttributeMutationScope {

    WTF_MAKE_NONCOPYABLE(StyleAttributeMutationScope);

public:

    StyleAttributeMutationScope(CSSMutableStyleDeclaration* decl)

    {

        ++s_scopeCount;

        if (s_scopeCount != 1) {

            ASSERT(s_currentDecl == decl);

            return;

        }

        ASSERT(!s_currentDecl);

        s_currentDecl = decl;

        if (!s_currentDecl->isInlineStyleDeclaration())

            return;

        s_mutationRecipients = MutationObserverInterestGroup::createForAttributesMutation(s_currentDecl->node(), HTMLNames::styleAttr);

        if (s_mutationRecipients->isEmpty()) {

            s_mutationRecipients.clear();

            return;

        }

        Element* element = toElement(s_currentDecl->node());

        AtomicString oldValue = s_mutationRecipients->isOldValueRequested() ? element->getAttribute(HTMLNames::styleAttr) : nullAtom;

        s_mutation = MutationRecord::createAttributes(element, HTMLNames::styleAttr, oldValue);

    }

    ~StyleAttributeMutationScope()

    {

        --s_scopeCount;

        if (!s_scopeCount)

            s_currentDecl = 0;

    }

    void enqueueMutationRecord()

    {

        if (!s_mutation)

            return;

        s_mutationRecipients->enqueueMutationRecord(s_mutation);

        s_mutation.clear();

        s_mutationRecipients.clear();

    }

private:

    static unsigned s_scopeCount;

    static OwnPtr<MutationObserverInterestGroup> s_mutationRecipients;

    static RefPtr<MutationRecord> s_mutation;

    static CSSMutableStyleDeclaration* s_currentDecl;

};

unsigned StyleAttributeMutationScope::s_scopeCount = 0;

OwnPtr<MutationObserverInterestGroup> StyleAttributeMutationScope::s_mutationRecipients;

RefPtr<MutationRecord> StyleAttributeMutationScope::s_mutation;

CSSMutableStyleDeclaration* StyleAttributeMutationScope::s_currentDecl = 0;

} // namespace

#endif // ENABLE(MUTATION_OBSERVERS)

CSSMutableStyleDeclaration::CSSMutableStyleDeclaration()

    : CSSStyleDeclaration(0, /* isMutable */ true)

    , m_node(0)

@@String CSSMutableStyleDeclaration::removeProperty(int propertyID, bool notifyCha

{

    ASSERT(!m_iteratorCount);

#if ENABLE(MUTATION_OBSERVERS)

    StyleAttributeMutationScope mutationScope(this);

#endif

    if (removeShorthandProperty(propertyID, notifyChanged)) {

        // FIXME: Return an equivalent shorthand when possible.

        return String();

@@String CSSMutableStyleDeclaration::removeProperty(int propertyID, bool notifyCha

    // and sweeping them when the vector grows too big.

    m_properties.remove(foundProperty - m_properties.data());

#if ENABLE(MUTATION_OBSERVERS)

    mutationScope.enqueueMutationRecord();

#endif

    if (notifyChanged)

        setNeedsStyleRecalc();

@@bool CSSMutableStyleDeclaration::setProperty(int propertyID, const String& value

{

    ASSERT(!m_iteratorCount);

#if ENABLE(MUTATION_OBSERVERS)

    StyleAttributeMutationScope mutationScope(this);

#endif

    // Setting the value to an empty string just removes the property in both IE and Gecko.

    // Setting it to null seems to produce less consistent results, but we treat it just the same.

    if (value.isEmpty()) {

@@bool CSSMutableStyleDeclaration::setProperty(int propertyID, const String& value

    if (!success) {

        // CSS DOM requires raising SYNTAX_ERR here, but this is too dangerous for compatibility,

        // see <http://bugs.webkit.org/show_bug.cgi?id=7296>.

618 } else if (notifyChanged)

        return false;

    }

#if ENABLE(MUTATION_OBSERVERS)

    mutationScope.enqueueMutationRecord();

#endif

    if (notifyChanged)

619704 setNeedsStyleRecalc();

620705

621 return s~~uccess~~;

706 return true;

}

void CSSMutableStyleDeclaration::setPropertyInternal(const CSSProperty& property, CSSProperty* slot)

{

    ASSERT(!m_iteratorCount);

#if ENABLE(MUTATION_OBSERVERS)

    StyleAttributeMutationScope mutationScope(this);

#endif

    if (!removeShorthandProperty(property.id(), false)) {

        CSSProperty* toReplace = slot ? slot : findPropertyWithId(property.id());

        if (toReplace) {

@@void CSSMutableStyleDeclaration::setPropertyInternal(const CSSProperty& property

        }

    }

    m_properties.append(property);

#if ENABLE(MUTATION_OBSERVERS)

    mutationScope.enqueueMutationRecord();

#endif

}

bool CSSMutableStyleDeclaration::setProperty(int propertyID, int value, bool important, bool notifyChanged)

@@void CSSMutableStyleDeclaration::parseDeclaration(const String& styleDeclaration

{

    ASSERT(!m_iteratorCount);

#if ENABLE(MUTATION_OBSERVERS)

    StyleAttributeMutationScope mutationScope(this);

#endif

    m_properties.clear();

    CSSParser parser(useStrictParsing());

    parser.parseDeclaration(this, styleDeclaration);

#if ENABLE(MUTATION_OBSERVERS)

    mutationScope.enqueueMutationRecord();

#endif

    setNeedsStyleRecalc();

}

@@void CSSMutableStyleDeclaration::addParsedProperties(const CSSProperty* const* p

{

    ASSERT(!m_iteratorCount);

#if ENABLE(MUTATION_OBSERVERS)

    StyleAttributeMutationScope mutationScope(this);

#endif

    m_properties.reserveCapacity(numProperties);

    for (int i = 0; i < numProperties; ++i)

        addParsedProperty(*properties[i]);

#if ENABLE(MUTATION_OBSERVERS)

    mutationScope.enqueueMutationRecord();

#endif

    // FIXME: This probably should have a call to setNeedsStyleRecalc() if something changed. We may also wish to add

    // a notifyChanged argument to this function to follow the model of other functions in this class.

}

@@void CSSMutableStyleDeclaration::addParsedProperty(const CSSProperty& property)

{

    ASSERT(!m_iteratorCount);

#if ENABLE(MUTATION_OBSERVERS)

    StyleAttributeMutationScope mutationScope(this);

#endif

    // Only add properties that have no !important counterpart present

    if (!getPropertyPriority(property.id()) || property.isImportant()) {

        removeProperty(property.id(), false, false);

        m_properties.append(property);

    }

#if ENABLE(MUTATION_OBSERVERS)

    mutationScope.enqueueMutationRecord();

#endif

}

void CSSMutableStyleDeclaration::setLengthProperty(int propertyId, const String& value, bool important, bool /*multiLength*/)

@@void CSSMutableStyleDeclaration::setCssText(const String& text, ExceptionCode& e

{

    ASSERT(!m_iteratorCount);

#if ENABLE(MUTATION_OBSERVERS)

    StyleAttributeMutationScope mutationScope(this);

#endif

    ec = 0;

    m_properties.clear();

    CSSParser parser(useStrictParsing());

    parser.parseDeclaration(this, text);

#if ENABLE(MUTATION_OBSERVERS)

    mutationScope.enqueueMutationRecord();

#endif

    // FIXME: Detect syntax errors and set ec.

    setNeedsStyleRecalc();

}

@@void CSSMutableStyleDeclaration::merge(const CSSMutableStyleDeclaration* other,

{

    ASSERT(!m_iteratorCount);

#if ENABLE(MUTATION_OBSERVERS)

    StyleAttributeMutationScope mutationScope(this);

#endif

    unsigned size = other->m_properties.size();

    for (unsigned n = 0; n < size; ++n) {

        const CSSProperty& toMerge = other->m_properties[n];

@@void CSSMutableStyleDeclaration::merge(const CSSMutableStyleDeclaration* other,

        } else

            m_properties.append(toMerge);

    }

#if ENABLE(MUTATION_OBSERVERS)

    mutationScope.enqueueMutationRecord();

#endif

    // FIXME: This probably should have a call to setNeedsStyleRecalc() if something changed. We may also wish to add

    // a notifyChanged argument to this function to follow the model of other functions in this class.

}

@@void CSSMutableStyleDeclaration::removePropertiesInSet(const int* set, unsigned

    if (m_properties.isEmpty())

        return;

#if ENABLE(MUTATION_OBSERVERS)

    StyleAttributeMutationScope mutationScope(this);

#endif

    // FIXME: This is always used with static sets and in that case constructing the hash repeatedly is pretty pointless.

    HashSet<int> toRemove;

    for (unsigned i = 0; i < length; ++i)

@@void CSSMutableStyleDeclaration::removePropertiesInSet(const int* set, unsigned

    bool changed = newProperties.size() != m_properties.size();

    m_properties = newProperties;

895 if (changed && notifyChanged)

896 setNeedsStyleRecalc();

    if (!changed || !notifyChanged)

        return;

#if ENABLE(MUTATION_OBSERVERS)

    mutationScope.enqueueMutationRecord();

#endif

    setNeedsStyleRecalc();

}

PassRefPtr<CSSMutableStyleDeclaration> CSSMutableStyleDeclaration::makeMutable()

Source/WebCore/dom/Element.cpp

@@const AtomicString& Element::getAttributeNS(const String& namespaceURI, const St

#if ENABLE(MUTATION_OBSERVERS)

static void enqueueAttributesMutationRecord(Element* target, const QualifiedName& attributeName, const AtomicString& oldValue)

{

622 OwnPtr<MutationObserverInterestGroup> mutationRecipients = MutationObserverInterestGroup::createForAttributesMutation(target, attributeName~~.localName()~~);

622 OwnPtr<MutationObserverInterestGroup> mutationRecipients = MutationObserverInterestGroup::createForAttributesMutation(target, attributeName);

    mutationRecipients->enqueueMutationRecord(MutationRecord::createAttributes(target, attributeName, oldValue));

}

#endif

@@void Element::setAttribute(const AtomicString& name, const AtomicString& value,

#if ENABLE(MUTATION_OBSERVERS)

    // The call to attributeChanged below may dispatch DOMSubtreeModified, so it's important to enqueue a MutationRecord now.

649 enqueueAttributesMutationRecord(this, attributeName, old ? old->value() : nullAtom);

649 if (!isSynchronizingStyleAttribute())

650 enqueueAttributesMutationRecord(this, attributeName, old ? old->value() : nullAtom);

#endif

    if (isIdAttributeName(old ? old->name() : attributeName))

@@void Element::setAttribute(const QualifiedName& name, const AtomicString& value,

#if ENABLE(MUTATION_OBSERVERS)

    // The call to attributeChanged below may dispatch DOMSubtreeModified, so it's important to enqueue a MutationRecord now.

687 enqueueAttributesMutationRecord(this, name, old ? old->value() : nullAtom);

688 if (!isSynchronizingStyleAttribute())

689 enqueueAttributesMutationRecord(this, name, old ? old->value() : nullAtom);

#endif

    if (isIdAttributeName(name))

Source/WebCore/dom/WebKitMutationObserver.cpp

#include "MutationObserverRegistration.h"

#include "MutationRecord.h"

#include "Node.h"

43#include "QualifiedName.h"

#include <wtf/ListHashSet.h>

namespace WebCore {

@@PassOwnPtr<MutationObserverInterestGroup> MutationObserverInterestGroup::createF

    return adoptPtr(new MutationObserverInterestGroup(target, WebKitMutationObserver::CharacterData));

}

154 PassOwnPtr<MutationObserverInterestGroup> MutationObserverInterestGroup::createForAttributesMutation(Node* target, const ~~AtomicString~~& attributeName)

155PassOwnPtr<MutationObserverInterestGroup> MutationObserverInterestGroup::createForAttributesMutation(Node* target, const QualifiedName& attributeName)

155156{

156 return adoptPtr(new MutationObserverInterestGroup(target, WebKitMutationObserver::Attributes, attributeName));

157 return adoptPtr(new MutationObserverInterestGroup(target, WebKitMutationObserver::Attributes, attributeName.localName()));

}

MutationObserverInterestGroup::MutationObserverInterestGroup(Node* target, WebKitMutationObserver::MutationType type, const AtomicString& attributeName)

Source/WebCore/dom/WebKitMutationObserver.h

@@class MutationCallback;

class MutationObserverRegistration;

class MutationRecord;

class Node;

50class QualifiedName;

typedef int ExceptionCode;

@@class MutationObserverInterestGroup {

public:

    static PassOwnPtr<MutationObserverInterestGroup> createForChildListMutation(Node* target);

    static PassOwnPtr<MutationObserverInterestGroup> createForCharacterDataMutation(Node* target);

102 static PassOwnPtr<MutationObserverInterestGroup> createForAttributesMutation(Node* target, const ~~AtomicString~~& attributeName);

103 static PassOwnPtr<MutationObserverInterestGroup> createForAttributesMutation(Node* target, const QualifiedName& attributeName);

    bool isOldValueRequested();

    bool isEmpty() { return m_observers.isEmpty(); }

LayoutTests/ChangeLog

2011-11-23  Rafael Weinstein  <rafaelw@chromium.org>

        [MutationObservers] Modifications to the style property don't dispatch "attributes" Mutation Records

        https://bugs.webkit.org/show_bug.cgi?id=70137

        Reviewed by NOBODY (OOPS!).

        Added tests asserting that changes to the style property are observable and work correctly if oldValue is requested.

        * fast/mutation/observe-attributes-expected.txt:

        * fast/mutation/observe-attributes.html:

2011-11-23  Philippe Normand  <pnormand@igalia.com>

        Unreviewed, skip another svg test on GTK showing signed zero issues.

LayoutTests/fast/mutation/observe-attributes-expected.txt

@@PASS mutations[3].type is "attributes"

PASS mutations[3].attributeName is "pathLength"

PASS mutations[3].attributeNamespace is "http://www.w3.org/2000/svg"

Testing that modifying an elements style property dispatches Mutation Records.

PASS mutations.length is 3

PASS mutations[0].type is "attributes"

PASS mutations[0].attributeName is "style"

PASS mutations[0].oldValue is null

PASS mutations[1].type is "attributes"

PASS mutations[1].attributeName is "style"

PASS mutations[1].oldValue is null

PASS mutations[2].type is "attributes"

PASS mutations[2].attributeName is "style"

PASS mutations[2].oldValue is null

...mutation record created.

PASS mutations is null

Testing that modifying an elements style property dispatches Mutation Records with correct oldValues.

PASS mutations.length is 3

PASS mutations[0].type is "attributes"

PASS mutations[0].attributeName is "style"

PASS mutations[0].oldValue is "color: yellow; width: 100px; "

PASS mutations[1].type is "attributes"

PASS mutations[1].attributeName is "style"

PASS mutations[1].oldValue is "width: 100px; color: red; "

PASS mutations[2].type is "attributes"

PASS mutations[2].attributeName is "style"

PASS mutations[2].oldValue is "color: red; width: 200px; "

...mutation record created.

PASS mutations is null

PASS successfullyParsed is true

TEST COMPLETE

LayoutTests/fast/mutation/observe-attributes.html

@@function testAttributeFilterNonHTMLDocument() {

    start();

}

function testStyleAttributePropertyAccess() {

    var svgDoc, div, path;

    var observer;

    function start() {

        debug('Testing that modifying an elements style property dispatches Mutation Records.');

        mutations = null;

        observer = new WebKitMutationObserver(function(m) {

            mutations = m;

        });

        div = document.createElement('div');

        div.setAttribute('style', 'color: yellow; width: 100px; ');

        observer.observe(div, { attributes: true });

        div.style.color = 'red';

        div.style.width = '200px';

        div.style.color = 'blue';

        setTimeout(checkAndContinue, 0);

    }

    function checkAndContinue() {

        shouldBe('mutations.length', '3');

        shouldBe('mutations[0].type', '"attributes"');

        shouldBe('mutations[0].attributeName', '"style"');

        shouldBe('mutations[0].oldValue', 'null');

        shouldBe('mutations[1].type', '"attributes"');

        shouldBe('mutations[1].attributeName', '"style"');

        shouldBe('mutations[1].oldValue', 'null');

        shouldBe('mutations[2].type', '"attributes"');

        shouldBe('mutations[2].attributeName', '"style"');

        shouldBe('mutations[2].oldValue', 'null');

        mutations = null;

        div.getAttribute('style');

        setTimeout(finish, 0);

    }

    function finish() {

        debug('...mutation record created.');

        shouldBe('mutations', 'null');

        observer.disconnect();

        debug('');

        runNextTest();

    }

    start();

}

function testStyleAttributePropertyAccessOldValue() {

    var svgDoc, div, path;

    var observer;

    function start() {

        debug('Testing that modifying an elements style property dispatches Mutation Records with correct oldValues.');

        mutations = null;

        observer = new WebKitMutationObserver(function(m) {

            mutations = m;

        });

        div = document.createElement('div');

        div.setAttribute('style', 'color: yellow; width: 100px; ');

        observer.observe(div, { attributes: true, attributeOldValue: true });

        div.style.color = 'red';

        div.style.width = '200px';

        div.style.color = 'blue';

        setTimeout(checkAndContinue, 0);

    }

    function checkAndContinue() {

        shouldBe('mutations.length', '3');

        shouldBe('mutations[0].type', '"attributes"');

        shouldBe('mutations[0].attributeName', '"style"');

        shouldBe('mutations[0].oldValue', '"color: yellow; width: 100px; "');

        shouldBe('mutations[1].type', '"attributes"');

        shouldBe('mutations[1].attributeName', '"style"');

        shouldBe('mutations[1].oldValue', '"width: 100px; color: red; "');

        shouldBe('mutations[2].type', '"attributes"');

        shouldBe('mutations[2].attributeName', '"style"');

        shouldBe('mutations[2].oldValue', '"color: red; width: 200px; "');

        mutations = null;

        div.getAttribute('style');

        setTimeout(finish, 0);

    }

    function finish() {

        debug('...mutation record created.');

        shouldBe('mutations', 'null');

        observer.disconnect();

        debug('');

        runNextTest();

    }

    start();

}

var tests = [

    testBasic,

    testWrongType,

@@var tests = [

    testAttributeFilter,

    testAttributeFilterSubtree,

    testAttributeFilterNonHTMLElement,

656 testAttributeFilterNonHTMLDocument

    testAttributeFilterNonHTMLDocument,

    testStyleAttributePropertyAccess,

    testStyleAttributePropertyAccessOldValue

];

var testIndex = 0;