Changeset 102264 in webkit

Timestamp:

Dec 7, 2011 12:03:50 PM (12 years ago)

Author:

adamk@chromium.org

Message:

Layout Test inspector/debugger/dom-breakpoints.html fails on chromium linux debug with ENABLE(MUTATION_OBSERVERS)
​https://bugs.webkit.org/show_bug.cgi?id=73919

Reviewed by Ojan Vafai.

Source/WebCore:

Use StyleAttributeMutationScope to manage DOM breakpoints for style property changes.

Instead of calling InspectorInstrumentation::didInvalidateStyleAttr()
directly in setNeedsStyleRecalc, set a bool in the current
StyleAttributeMutationScope, and delay the call until the scope's
counter runs down to zero. This keeps the inspector JS from re-entering
CSSMutableStyleDeclaration until all StyleAttributeMutationScope work is done.

Also fix a small bug in StyleAttributeMutationScope, where
s_shouldDeliver wasn't getting reset properly to false.

• css/CSSMutableStyleDeclaration.cpp:

(WebCore::CSSMutableStyleDeclaration::setNeedsStyleRecalc):

LayoutTests:

Added test that no-op style mutations don't create MutationRecords.

• fast/mutation/observe-attributes-expected.txt:
• fast/mutation/observe-attributes.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/mutation/observe-attributes-expected.txt (modified) (1 diff)
• LayoutTests/fast/mutation/observe-attributes.html (modified) (4 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSMutableStyleDeclaration.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-12-07  Adam Klein  <adamk@chromium.org>
+
+        Layout Test inspector/debugger/dom-breakpoints.html fails on chromium linux debug with ENABLE(MUTATION_OBSERVERS)
+        https://bugs.webkit.org/show_bug.cgi?id=73919
+
+        Reviewed by Ojan Vafai.
+
+        Added test that no-op style mutations don't create MutationRecords.
+
+        * fast/mutation/observe-attributes-expected.txt:
+        * fast/mutation/observe-attributes.html:
+
 2011-12-07  Ken Buchanan <kenrb@chromium.org>
 

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

@@ -187 +187 @@
 PASS mutations is null
 
+Testing that a no-op style property mutation does not create Mutation Records.
+PASS mutations is null
+
 PASS successfullyParsed is true
 

trunk/LayoutTests/fast/mutation/observe-attributes.html

@@ -653 +653 @@
 
 function testStyleAttributePropertyAccess() {
-    var svgDoc, div, path;
+    var div, path;
     var observer;
 
@@ -705 +705 @@
 
 function testStyleAttributePropertyAccessOldValue() {
-    var svgDoc, div, path;
+    var div, path;
     var observer;
 
@@ -746 +746 @@
         debug('...mutation record created.');
 
+        shouldBe('mutations', 'null');
+
+        observer.disconnect();
+        debug('');
+        runNextTest();
+    }
+
+    start();
+}
+
+function testStyleAttributePropertyAccessIgnoreNoop() {
+    var div, path;
+    var observer;
+
+    function start() {
+        debug('Testing that a no-op style property mutation does not create 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.removeProperty('height');
+
+        setTimeout(finish, 0);
+    }
+
+    function finish() {
         shouldBe('mutations', 'null');
 
@@ -773 +804 @@
     testAttributeFilterNonHTMLDocument,
     testStyleAttributePropertyAccess,
-    testStyleAttributePropertyAccessOldValue
+    testStyleAttributePropertyAccessOldValue,
+    testStyleAttributePropertyAccessIgnoreNoop
 ];
 var testIndex = 0;

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-12-07  Adam Klein  <adamk@chromium.org>
+
+        Layout Test inspector/debugger/dom-breakpoints.html fails on chromium linux debug with ENABLE(MUTATION_OBSERVERS)
+        https://bugs.webkit.org/show_bug.cgi?id=73919
+
+        Reviewed by Ojan Vafai.
+
+        Use StyleAttributeMutationScope to manage DOM breakpoints for style property changes.
+
+        Instead of calling InspectorInstrumentation::didInvalidateStyleAttr()
+        directly in setNeedsStyleRecalc, set a bool in the current
+        StyleAttributeMutationScope, and delay the call until the scope's
+        counter runs down to zero. This keeps the inspector JS from re-entering
+        CSSMutableStyleDeclaration until all StyleAttributeMutationScope work is done.
+
+        Also fix a small bug in StyleAttributeMutationScope, where
+        s_shouldDeliver wasn't getting reset properly to false.
+
+        * css/CSSMutableStyleDeclaration.cpp:
+        (WebCore::CSSMutableStyleDeclaration::setNeedsStyleRecalc):
+
 2011-12-07  Ken Buchanan <kenrb@chromium.org>
 

trunk/Source/WebCore/css/CSSMutableStyleDeclaration.cpp

@@ -46 +46 @@
 namespace WebCore {
 
-#if ENABLE(MUTATION_OBSERVERS)
 namespace {
 
@@ -64 +63 @@
         s_currentDecl = decl;
 
+#if ENABLE(MUTATION_OBSERVERS)
         if (!s_currentDecl->isInlineStyleDeclaration())
             return;
@@ -76 +76 @@
         AtomicString oldValue = m_mutationRecipients->isOldValueRequested() ? inlineDecl->element()->getAttribute(HTMLNames::styleAttr) : nullAtom;
         m_mutation = MutationRecord::createAttributes(inlineDecl->element(), HTMLNames::styleAttr, oldValue);
+#endif
     }
 
@@ -84 +85 @@
             return;
 
-        s_currentDecl = 0;
+#if ENABLE(MUTATION_OBSERVERS)
         if (m_mutation && s_shouldDeliver)
             m_mutationRecipients->enqueueMutationRecord(m_mutation);
-    }
-
+        s_shouldDeliver = false;
+#endif
+
+        if (!s_shouldNotifyInspector) {
+            s_currentDecl = 0;
+            return;
+        }
+
+        CSSInlineStyleDeclaration* inlineDecl = toCSSInlineStyleDeclaration(s_currentDecl);
+        s_currentDecl = 0;
+        s_shouldNotifyInspector = false;
+        if (inlineDecl->element()->document())
+            InspectorInstrumentation::didInvalidateStyleAttr(inlineDecl->element()->document(), inlineDecl->element());
+    }
+
+#if ENABLE(MUTATION_OBSERVERS)
     void enqueueMutationRecord()
     {
         s_shouldDeliver = true;
     }
+#endif
+
+    void didInvalidateStyleAttr()
+    {
+        ASSERT(s_currentDecl->isInlineStyleDeclaration());
+        s_shouldNotifyInspector = true;
+    }
 
 private:
     static unsigned s_scopeCount;
     static CSSMutableStyleDeclaration* s_currentDecl;
+    static bool s_shouldNotifyInspector;
+#if ENABLE(MUTATION_OBSERVERS)
     static bool s_shouldDeliver;
 
     OwnPtr<MutationObserverInterestGroup> m_mutationRecipients;
     RefPtr<MutationRecord> m_mutation;
+#endif
 };
 
 unsigned StyleAttributeMutationScope::s_scopeCount = 0;
 CSSMutableStyleDeclaration* StyleAttributeMutationScope::s_currentDecl = 0;
+bool StyleAttributeMutationScope::s_shouldNotifyInspector = false;
+#if ENABLE(MUTATION_OBSERVERS)
 bool StyleAttributeMutationScope::s_shouldDeliver = false;
+#endif
 
 } // namespace
-#endif // ENABLE(MUTATION_OBSERVERS)
 
 CSSMutableStyleDeclaration::CSSMutableStyleDeclaration()
@@ -621 +648 @@
             element->setNeedsStyleRecalc(InlineStyleChange);
             element->invalidateStyleAttribute();
-            if (Document* document = element->document())
-                InspectorInstrumentation::didInvalidateStyleAttr(document, element);
+            StyleAttributeMutationScope(this).didInvalidateStyleAttr();
         }
         return;