Layout Test inspector/debugger/dom-breakpoints.html fails on chromium linux debug...
authoradamk@chromium.org <adamk@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Dec 2011 20:03:50 +0000 (20:03 +0000)
committeradamk@chromium.org <adamk@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Dec 2011 20:03:50 +0000 (20:03 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@102264 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/mutation/observe-attributes-expected.txt
LayoutTests/fast/mutation/observe-attributes.html
Source/WebCore/ChangeLog
Source/WebCore/css/CSSMutableStyleDeclaration.cpp

index 7a5aa4b..d8a6b04 100755 (executable)
@@ -1,3 +1,15 @@
+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>
 
         Crash from multicol spans with layers
index ee2e1c9..7fffc5f 100644 (file)
@@ -186,6 +186,9 @@ PASS mutations[2].oldValue is "color: red; width: 200px; "
 ...mutation record created.
 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
 
 TEST COMPLETE
index a182617..0ee36ff 100644 (file)
@@ -652,7 +652,7 @@ function testAttributeFilterNonHTMLDocument() {
 }
 
 function testStyleAttributePropertyAccess() {
-    var svgDoc, div, path;
+    var div, path;
     var observer;
 
     function start() {
@@ -704,7 +704,7 @@ function testStyleAttributePropertyAccess() {
 }
 
 function testStyleAttributePropertyAccessOldValue() {
-    var svgDoc, div, path;
+    var div, path;
     var observer;
 
     function start() {
@@ -755,6 +755,37 @@ function testStyleAttributePropertyAccessOldValue() {
     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');
+
+        observer.disconnect();
+        debug('');
+        runNextTest();
+    }
+
+    start();
+}
+
 var tests = [
     testBasic,
     testWrongType,
@@ -772,7 +803,8 @@ var tests = [
     testAttributeFilterNonHTMLElement,
     testAttributeFilterNonHTMLDocument,
     testStyleAttributePropertyAccess,
-    testStyleAttributePropertyAccessOldValue
+    testStyleAttributePropertyAccessOldValue,
+    testStyleAttributePropertyAccessIgnoreNoop
 ];
 var testIndex = 0;
 
index 3ce4060..e7ca35b 100755 (executable)
@@ -1,3 +1,24 @@
+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>
 
         Crash from multicol spans with layers
index 3feac84..3f235d4 100644 (file)
@@ -45,7 +45,6 @@ using namespace std;
 
 namespace WebCore {
 
-#if ENABLE(MUTATION_OBSERVERS)
 namespace {
 
 class StyleAttributeMutationScope {
@@ -63,6 +62,7 @@ public:
         ASSERT(!s_currentDecl);
         s_currentDecl = decl;
 
+#if ENABLE(MUTATION_OBSERVERS)
         if (!s_currentDecl->isInlineStyleDeclaration())
             return;
 
@@ -75,6 +75,7 @@ public:
 
         AtomicString oldValue = m_mutationRecipients->isOldValueRequested() ? inlineDecl->element()->getAttribute(HTMLNames::styleAttr) : nullAtom;
         m_mutation = MutationRecord::createAttributes(inlineDecl->element(), HTMLNames::styleAttr, oldValue);
+#endif
     }
 
     ~StyleAttributeMutationScope()
@@ -83,31 +84,57 @@ public:
         if (s_scopeCount)
             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()
     : CSSStyleDeclaration(0)
@@ -620,8 +647,7 @@ void CSSMutableStyleDeclaration::setNeedsStyleRecalc()
         else {
             element->setNeedsStyleRecalc(InlineStyleChange);
             element->invalidateStyleAttribute();
-            if (Document* document = element->document())
-                InspectorInstrumentation::didInvalidateStyleAttr(document, element);
+            StyleAttributeMutationScope(this).didInvalidateStyleAttr();
         }
         return;
     }