Title: [241091] branches/safari-607-branch
Revision
241091
Author
alanc...@apple.com
Date
2019-02-06 14:18:10 -0800 (Wed, 06 Feb 2019)

Log Message

Cherry-pick r240946. rdar://problem/47830598

    Web Inspector: Styles: fix race conditions when editing
    https://bugs.webkit.org/show_bug.cgi?id=192739
    <rdar://problem/46752925>

    Reviewed by Devin Rousso.

    Source/WebInspectorUI:

    Editing CSS property in the style editor syncronously updates CSSStyleDeclaration on the front-end
    and asyncronously updates the backend by calling CSSAgent.setStyleText. After the new style text is applied
    on the backend, CSSStyleDeclaration (on the front-end) gets updated.

    Unsure there's no race conditions by introducing `_updatesInProgressCount`:

      - Increment it before calling CSSAgent.setStyleText.
      - Decrement it after CSSAgent.setStyleText is finished.

    Prevent updates of CSSStyleDeclaration when _updatesInProgressCount isn't 0.

    * UserInterface/Models/CSSProperty.js:
    (WI.CSSProperty.prototype._updateOwnerStyleText):
    * UserInterface/Models/CSSStyleDeclaration.js:
    (WI.CSSStyleDeclaration):
    (WI.CSSStyleDeclaration.prototype.set text): Removed.
    (WI.CSSStyleDeclaration.prototype.setText): Added.
    Change the setter to a method since it has side effects including an asynchronous backend call.

    * UserInterface/Models/DOMNodeStyles.js:
    (WI.DOMNodeStyles.prototype.changeStyleText):

    * UserInterface/Views/SpreadsheetStyleProperty.js:
    (WI.SpreadsheetStyleProperty.prototype.get nameTextField): Removed.
    (WI.SpreadsheetStyleProperty.prototype.get valueTextField): Removed.
    Drive-by: remove unused code.

    LayoutTests:

    * inspector/css/modify-css-property-expected.txt:
    * inspector/css/modify-css-property-race-expected.txt: Added.
    * inspector/css/modify-css-property-race.html: Added.
    * inspector/css/modify-css-property.html:

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

Modified Paths

Added Paths

Diff

Modified: branches/safari-607-branch/LayoutTests/ChangeLog (241090 => 241091)


--- branches/safari-607-branch/LayoutTests/ChangeLog	2019-02-06 22:18:06 UTC (rev 241090)
+++ branches/safari-607-branch/LayoutTests/ChangeLog	2019-02-06 22:18:10 UTC (rev 241091)
@@ -1,3 +1,64 @@
+2019-02-06  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r240946. rdar://problem/47830598
+
+    Web Inspector: Styles: fix race conditions when editing
+    https://bugs.webkit.org/show_bug.cgi?id=192739
+    <rdar://problem/46752925>
+    
+    Reviewed by Devin Rousso.
+    
+    Source/WebInspectorUI:
+    
+    Editing CSS property in the style editor syncronously updates CSSStyleDeclaration on the front-end
+    and asyncronously updates the backend by calling CSSAgent.setStyleText. After the new style text is applied
+    on the backend, CSSStyleDeclaration (on the front-end) gets updated.
+    
+    Unsure there's no race conditions by introducing `_updatesInProgressCount`:
+    
+      - Increment it before calling CSSAgent.setStyleText.
+      - Decrement it after CSSAgent.setStyleText is finished.
+    
+    Prevent updates of CSSStyleDeclaration when _updatesInProgressCount isn't 0.
+    
+    * UserInterface/Models/CSSProperty.js:
+    (WI.CSSProperty.prototype._updateOwnerStyleText):
+    * UserInterface/Models/CSSStyleDeclaration.js:
+    (WI.CSSStyleDeclaration):
+    (WI.CSSStyleDeclaration.prototype.set text): Removed.
+    (WI.CSSStyleDeclaration.prototype.setText): Added.
+    Change the setter to a method since it has side effects including an asynchronous backend call.
+    
+    * UserInterface/Models/DOMNodeStyles.js:
+    (WI.DOMNodeStyles.prototype.changeStyleText):
+    
+    * UserInterface/Views/SpreadsheetStyleProperty.js:
+    (WI.SpreadsheetStyleProperty.prototype.get nameTextField): Removed.
+    (WI.SpreadsheetStyleProperty.prototype.get valueTextField): Removed.
+    Drive-by: remove unused code.
+    
+    LayoutTests:
+    
+    * inspector/css/modify-css-property-expected.txt:
+    * inspector/css/modify-css-property-race-expected.txt: Added.
+    * inspector/css/modify-css-property-race.html: Added.
+    * inspector/css/modify-css-property.html:
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240946 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-02-04  Nikita Vasilyev  <nvasil...@apple.com>
+
+            Web Inspector: Styles: fix race conditions when editing
+            https://bugs.webkit.org/show_bug.cgi?id=192739
+            <rdar://problem/46752925>
+
+            Reviewed by Devin Rousso.
+
+            * inspector/css/modify-css-property-expected.txt:
+            * inspector/css/modify-css-property-race-expected.txt: Added.
+            * inspector/css/modify-css-property-race.html: Added.
+            * inspector/css/modify-css-property.html:
+
 2019-02-05  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r239752. rdar://problem/47776478

Modified: branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property-expected.txt (241090 => 241091)


--- branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property-expected.txt	2019-02-06 22:18:06 UTC (rev 241090)
+++ branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property-expected.txt	2019-02-06 22:18:10 UTC (rev 241091)
@@ -4,18 +4,27 @@
 == Running test suite: ModifyCSSProperty
 -- Running test case: Update value when CSSStyleDeclaration is not locked
 PASS: "font-size" property value should update immediately.
-PASS: Style declaration text should stay unchanged.
+PASS: Style declaration text should update immediately.
 
 -- Running test case: Update value when CSSStyleDeclaration is locked
 PASS: "font-size" property value should update immediately.
 PASS: Style declaration text should update immediately.
 
+-- Running test case: ModifyCSSProperty.PropertiesChangedEvent
+PASS: Style declaration is unlocked.
+PASS: "width" property value should update to "200px".
+PASS: Inline style declaration text should update when not locked.
+
 -- Running test case: Update inline style value when CSSStyleDeclaration locked and not locked
 PASS: Style declaration text should update immediately.
-PASS: Style declaration text should stay "width: 64px".
-PASS: "width" property value should update to "200px".
+PASS: Style declaration text should update immediately.
+PASS: Style declaration is unlocked.
+PASS: "width" property value should update to "128px".
 PASS: Inline style declaration text should update when not locked.
 
+-- Running test case: ModifyCSSProperty.ConsequentValueChanges
+PASS: Style declaration text should contain most recent value.
+
 -- Running test case: ModifyCSSProperty.CommentOutAndUncommentPropertyWithNewlines
 PASS: Commented out property should be disabled.
 PASS: Style declaration text should update immediately with uncommented property.

Added: branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property-race-expected.txt (0 => 241091)


--- branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property-race-expected.txt	                        (rev 0)
+++ branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property-race-expected.txt	2019-02-06 22:18:10 UTC (rev 241091)
@@ -0,0 +1,11 @@
+Testing that changes to "style" attribute made from page's _javascript_ are ignored when there's a pending change made via Web Inspector.
+
+
+== Running test suite: ModifyCSSProperty
+-- Running test case: ModifyCSSPropertyRace.ChangeInlineStyle
+PASS: expectGreaterThan(43, 42)
+PASS: expectGreaterThan(43, 42)
+PASS: Value updated to "10px".
+PASS: CSSStyleDeclaration text should update.
+PASS: expectGreaterThanOrEqual(10, 10)
+

Added: branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property-race.html (0 => 241091)


--- branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property-race.html	                        (rev 0)
+++ branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property-race.html	2019-02-06 22:18:10 UTC (rev 241091)
@@ -0,0 +1,117 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script>
+let requestAnimationFrameID;
+
+function expand() {
+    let element = document.getElementById("x");
+    let initialHeight = parseInt(element.style.height);
+    let i = 0;
+
+    let loop = () => {
+        ++i;
+        let height = initialHeight + i;
+        element.style.height = height + "px";
+        TestPage.dispatchEventToFrontend("TestPage-styleChanged", height + "px");
+
+        requestAnimationFrameID = requestAnimationFrame(loop);
+    };
+
+    loop();
+}
+
+function stopExpanding() {
+    cancelAnimationFrame(requestAnimationFrameID);
+}
+
+function test() {
+    InspectorTest.redirectRequestAnimationFrame();
+
+    let nodeStyles = null;
+    let suite = InspectorTest.createAsyncSuite("ModifyCSSProperty");
+
+    InspectorTest.addEventListener("TestPage-styleChanged", (event) => {
+        // Normally, this would get called if the styles sidebar is visible.
+        // This doesn't work in tests.
+        nodeStyles.refresh();
+    });
+
+    suite.addTestCase({
+        name: "ModifyCSSPropertyRace.ChangeInlineStyle",
+        test(resolve, reject) {
+            let getInlineStyleDeclaration = () => {
+                for (let styleDeclaration of nodeStyles.orderedStyles) {
+                    if (styleDeclaration.type === styleDeclaration.constructor.Type.Inline)
+                        return styleDeclaration;
+                }
+                InspectorTest.fail("No declaration found.");
+                resolve();
+            };
+
+            let getProperty = (propertyName) => {
+                let styleDeclaration = getInlineStyleDeclaration();
+                for (let property of styleDeclaration.properties) {
+                    if (property.name === propertyName)
+                        return property;
+                }
+                InspectorTest.fail("No property found.");
+                resolve();
+            };
+
+            InspectorTest.evaluateInPage("expand()");
+            let updateCount = 0;
+
+            function styleDecorationUpdated(event) {
+                if (!updateCount) {
+                    let valueNumber = parseInt(getProperty("height").rawValue);
+                    InspectorTest.expectGreaterThan(valueNumber, 42);
+                } else if (updateCount === 1) {
+                    let valueNumber = parseInt(getProperty("height").rawValue);
+                    InspectorTest.expectGreaterThan(valueNumber, 42);
+                    getProperty("height").rawValue = "10px";
+                } else if (updateCount === 2) {
+                    InspectorTest.expectEqual(getProperty("height").rawValue, "10px", `Value updated to "10px".`);
+                    InspectorTest.expectEqual(getInlineStyleDeclaration().text, "height: 10px;", "CSSStyleDeclaration text should update.");
+                } else {
+                    let valueNumber = parseInt(getProperty("height").rawValue);
+                    InspectorTest.expectGreaterThanOrEqual(valueNumber, 10);
+
+                    InspectorTest.evaluateInPage("stopExpanding()");
+                    WI.CSSStyleDeclaration.removeEventListener(WI.CSSStyleDeclaration.Event.PropertiesChanged, styleDecorationUpdated);
+                    resolve();
+                }
+                ++updateCount;
+            }
+
+            WI.CSSStyleDeclaration.addEventListener(WI.CSSStyleDeclaration.Event.PropertiesChanged, styleDecorationUpdated);
+        }
+    });
+
+    WI.domManager.requestDocument((documentNode) => {
+        WI.domManager.querySelector(documentNode.id, "#x", (contentNodeId) => {
+            if (contentNodeId) {
+                let domNode = WI.domManager.nodeForId(contentNodeId);
+                nodeStyles = WI.cssManager.stylesForNode(domNode);
+
+                if (nodeStyles.needsRefresh) {
+                    nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
+                        suite.runTestCasesAndFinish()
+                    });
+                } else
+                    suite.runTestCasesAndFinish();
+            } else {
+                InspectorTest.fail("DOM node not found.");
+                InspectorTest.completeTest();
+            }
+        });
+    });
+}
+</script>
+</head>
+<body _onload_="runTest()">
+    <p>Testing that changes to "style" attribute made from page's _javascript_ are ignored when there's a pending change made via Web Inspector.</p>
+    <div id="x" style="height: 42px"></div>
+</body>
+</html>

Modified: branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property.html (241090 => 241091)


--- branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property.html	2019-02-06 22:18:06 UTC (rev 241090)
+++ branches/safari-607-branch/LayoutTests/inspector/css/modify-css-property.html	2019-02-06 22:18:10 UTC (rev 241091)
@@ -44,7 +44,7 @@
 
             InspectorTest.expectEqual(getProperty("font-size").rawValue, "10px", `"font-size" property value should update immediately.`);
 
-            InspectorTest.expectEqual(styleDeclaration.text, `font-size: 12px; color: antiquewhite`, `Style declaration text should stay unchanged.`);
+            InspectorTest.expectEqual(styleDeclaration.text, `font-size: 10px; color: antiquewhite`, `Style declaration text should update immediately.`);
 
             resolve();
         }
@@ -94,6 +94,46 @@
     });
 
     suite.addTestCase({
+        name: "ModifyCSSProperty.PropertiesChangedEvent",
+        test(resolve, reject) {
+            let getInlineStyleDeclaration = () => {
+                for (let styleDeclaration of nodeStyles.orderedStyles) {
+                    if (styleDeclaration.type === styleDeclaration.constructor.Type.Inline)
+                        return styleDeclaration;
+                }
+                InspectorTest.fail("No declaration found.");
+                resolve();
+            };
+
+            let getProperty = (propertyName) => {
+                let styleDeclaration = getInlineStyleDeclaration();
+                for (let property of styleDeclaration.properties) {
+                    if (property.name === propertyName)
+                        return property;
+                }
+                InspectorTest.fail("No property found.");
+                resolve();
+            };
+
+            let styleDeclaration = getInlineStyleDeclaration();
+
+            styleDeclaration.awaitEvent(WI.CSSStyleDeclaration.Event.PropertiesChanged).then((event) => {
+                InspectorTest.expectThat(!styleDeclaration.locked, `Style declaration is unlocked.`);
+                InspectorTest.expectEqual(getProperty("width").rawValue, "200px", `"width" property value should update to "200px".`);
+                InspectorTest.expectEqual(styleDeclaration.text, `width: 200px;`, `Inline style declaration text should update when not locked.`);
+                resolve();
+            });
+
+            styleDeclaration.locked = true;
+            // WI.CSSStyleDeclaration.Event.PropertiesChanged event should not fire when the style declaration is locked.
+            InspectorTest.evaluateInPage(`makeNarrow()`);
+
+            styleDeclaration.locked = false;
+            InspectorTest.evaluateInPage(`makeWide()`);
+        }
+    });
+
+    suite.addTestCase({
         name: "Update inline style value when CSSStyleDeclaration locked and not locked",
         test(resolve, reject) {
             let getInlineStyleDeclaration = () => {
@@ -117,12 +157,12 @@
 
             let styleDeclaration = getInlineStyleDeclaration();
 
-            styleDeclaration.awaitEvent(WI.CSSStyleDeclaration.Event.PropertiesChanged)
-                .then((event) => {
-                    InspectorTest.expectEqual(getProperty("width").rawValue, "200px", `"width" property value should update to "200px".`);
-                    InspectorTest.expectEqual(styleDeclaration.text, `width: 200px;`, `Inline style declaration text should update when not locked.`);
-                })
-                .then(resolve, reject);
+            WI.CSSStyleDeclaration.awaitEvent(WI.CSSStyleDeclaration.Event.PropertiesChanged).then((event) => {
+                InspectorTest.expectThat(!styleDeclaration.locked, `Style declaration is unlocked.`);
+                InspectorTest.expectEqual(getProperty("width").rawValue, "128px", `"width" property value should update to "128px".`);
+                InspectorTest.expectEqual(styleDeclaration.text, `width: 128px;`, `Inline style declaration text should update when not locked.`);
+                resolve();
+            });
 
             styleDeclaration.locked = true;
             getProperty("width").rawValue = "64px";
@@ -133,7 +173,7 @@
 
             styleDeclaration.locked = false;
             getProperty("width").rawValue = "128px";
-            InspectorTest.expectEqual(styleDeclaration.text, `width: 64px;`, `Style declaration text should stay "width: 64px".`);
+            InspectorTest.expectEqual(styleDeclaration.text, `width: 128px;`, `Style declaration text should update immediately.`);
 
             InspectorTest.evaluateInPage(`makeWide()`);
         }
@@ -140,6 +180,40 @@
     });
 
     suite.addTestCase({
+        name: "ModifyCSSProperty.ConsequentValueChanges",
+        test(resolve, reject) {
+            let getInlineStyleDeclaration = () => {
+                for (let styleDeclaration of nodeStyles.orderedStyles) {
+                    if (styleDeclaration.type === styleDeclaration.constructor.Type.Inline)
+                        return styleDeclaration;
+                }
+                InspectorTest.fail("No declaration found.");
+                resolve();
+            };
+
+            let getProperty = (propertyName) => {
+                let styleDeclaration = getInlineStyleDeclaration();
+                for (let property of styleDeclaration.properties) {
+                    if (property.name === propertyName)
+                        return property;
+                }
+                InspectorTest.fail("No property found.");
+                resolve();
+            };
+
+            let styleDeclaration = getInlineStyleDeclaration();
+            styleDeclaration.locked = false;
+
+            for (let i = 0; i <= 20; ++i)
+                getProperty("width").rawValue = i + "px";
+
+            InspectorTest.expectEqual(styleDeclaration.text, `width: 20px;`, `Style declaration text should contain most recent value.`);
+
+            resolve();
+        }
+    });
+
+    suite.addTestCase({
         name: "ModifyCSSProperty.CommentOutAndUncommentPropertyWithNewlines",
         test(resolve, reject) {
             let getMatchedStyleDeclaration = () => {

Modified: branches/safari-607-branch/Source/WebInspectorUI/ChangeLog (241090 => 241091)


--- branches/safari-607-branch/Source/WebInspectorUI/ChangeLog	2019-02-06 22:18:06 UTC (rev 241090)
+++ branches/safari-607-branch/Source/WebInspectorUI/ChangeLog	2019-02-06 22:18:10 UTC (rev 241091)
@@ -1,3 +1,86 @@
+2019-02-06  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r240946. rdar://problem/47830598
+
+    Web Inspector: Styles: fix race conditions when editing
+    https://bugs.webkit.org/show_bug.cgi?id=192739
+    <rdar://problem/46752925>
+    
+    Reviewed by Devin Rousso.
+    
+    Source/WebInspectorUI:
+    
+    Editing CSS property in the style editor syncronously updates CSSStyleDeclaration on the front-end
+    and asyncronously updates the backend by calling CSSAgent.setStyleText. After the new style text is applied
+    on the backend, CSSStyleDeclaration (on the front-end) gets updated.
+    
+    Unsure there's no race conditions by introducing `_updatesInProgressCount`:
+    
+      - Increment it before calling CSSAgent.setStyleText.
+      - Decrement it after CSSAgent.setStyleText is finished.
+    
+    Prevent updates of CSSStyleDeclaration when _updatesInProgressCount isn't 0.
+    
+    * UserInterface/Models/CSSProperty.js:
+    (WI.CSSProperty.prototype._updateOwnerStyleText):
+    * UserInterface/Models/CSSStyleDeclaration.js:
+    (WI.CSSStyleDeclaration):
+    (WI.CSSStyleDeclaration.prototype.set text): Removed.
+    (WI.CSSStyleDeclaration.prototype.setText): Added.
+    Change the setter to a method since it has side effects including an asynchronous backend call.
+    
+    * UserInterface/Models/DOMNodeStyles.js:
+    (WI.DOMNodeStyles.prototype.changeStyleText):
+    
+    * UserInterface/Views/SpreadsheetStyleProperty.js:
+    (WI.SpreadsheetStyleProperty.prototype.get nameTextField): Removed.
+    (WI.SpreadsheetStyleProperty.prototype.get valueTextField): Removed.
+    Drive-by: remove unused code.
+    
+    LayoutTests:
+    
+    * inspector/css/modify-css-property-expected.txt:
+    * inspector/css/modify-css-property-race-expected.txt: Added.
+    * inspector/css/modify-css-property-race.html: Added.
+    * inspector/css/modify-css-property.html:
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240946 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-02-04  Nikita Vasilyev  <nvasil...@apple.com>
+
+            Web Inspector: Styles: fix race conditions when editing
+            https://bugs.webkit.org/show_bug.cgi?id=192739
+            <rdar://problem/46752925>
+
+            Reviewed by Devin Rousso.
+
+            Editing CSS property in the style editor syncronously updates CSSStyleDeclaration on the front-end
+            and asyncronously updates the backend by calling CSSAgent.setStyleText. After the new style text is applied
+            on the backend, CSSStyleDeclaration (on the front-end) gets updated.
+
+            Unsure there's no race conditions by introducing `_updatesInProgressCount`:
+
+              - Increment it before calling CSSAgent.setStyleText.
+              - Decrement it after CSSAgent.setStyleText is finished.
+
+            Prevent updates of CSSStyleDeclaration when _updatesInProgressCount isn't 0.
+
+            * UserInterface/Models/CSSProperty.js:
+            (WI.CSSProperty.prototype._updateOwnerStyleText):
+            * UserInterface/Models/CSSStyleDeclaration.js:
+            (WI.CSSStyleDeclaration):
+            (WI.CSSStyleDeclaration.prototype.set text): Removed.
+            (WI.CSSStyleDeclaration.prototype.setText): Added.
+            Change the setter to a method since it has side effects including an asynchronous backend call.
+
+            * UserInterface/Models/DOMNodeStyles.js:
+            (WI.DOMNodeStyles.prototype.changeStyleText):
+
+            * UserInterface/Views/SpreadsheetStyleProperty.js:
+            (WI.SpreadsheetStyleProperty.prototype.get nameTextField): Removed.
+            (WI.SpreadsheetStyleProperty.prototype.get valueTextField): Removed.
+            Drive-by: remove unused code.
+
 2019-02-05  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r240997. rdar://problem/47838594

Modified: branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js (241090 => 241091)


--- branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js	2019-02-06 22:18:06 UTC (rev 241090)
+++ branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js	2019-02-06 22:18:10 UTC (rev 241091)
@@ -358,6 +358,10 @@
             return;
         }
 
+        console.assert(this._ownerStyle);
+        if (!this._ownerStyle)
+            return;
+
         this._prependSemicolonIfNeeded();
 
         let styleText = this._ownerStyle.text || "";

Modified: branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js (241090 => 241091)


--- branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js	2019-02-06 22:18:06 UTC (rev 241090)
+++ branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js	2019-02-06 22:18:10 UTC (rev 241091)
@@ -40,6 +40,7 @@
         this._node = node || null;
         this._inherited = inherited || false;
 
+        this._updatesInProgressCount = 0;
         this._locked = false;
         this._pendingProperties = [];
         this._propertyNameMap = {};
@@ -102,11 +103,24 @@
     update(text, properties, styleSheetTextRange, options = {})
     {
         let dontFireEvents = options.dontFireEvents || false;
-        let suppressLock = options.suppressLock || false;
 
-        if (this._locked && !suppressLock && text !== this._text)
+        // When two consequent setText calls happen (A and B), only update when the last call (B) is finished.
+        //               Front-end:   A B
+        //                Back-end:       A B
+        // _updatesInProgressCount: 0 1 2 1 0
+        //                                  ^
+        //                                  update only happens here
+        if (this._updatesInProgressCount > 0 && !options.forceUpdate) {
+            if (WI.settings.enableStyleEditingDebugMode.value && text !== this._text)
+                console.warn("Style modified while editing:", text);
+
             return;
+        }
 
+        // Allow updates from the backend when text matches because `properties` may contain warnings that need to be shown.
+        if (this._locked && !options.forceUpdate && text !== this._text)
+            return;
+
         text = text || "";
         properties = properties || [];
 
@@ -213,11 +227,24 @@
         if (!trimmedText.length || this._type === WI.CSSStyleDeclaration.Type.Inline)
             text = trimmedText;
 
-        // Update text immediately when it was modified via the styles sidebar.
-        if (this._locked)
-            this._text = text;
+        this._text = text;
+        ++this._updatesInProgressCount;
 
-        this._nodeStyles.changeStyleText(this, text);
+        let timeoutId = setTimeout(() => {
+            console.error("Timed out when setting style text:", text);
+            styleTextDidChange();
+        }, 2000);
+
+        let styleTextDidChange = () => {
+            if (!timeoutId)
+                return;
+
+            clearTimeout(timeoutId);
+            timeoutId = null;
+            this._updatesInProgressCount = Math.max(0, this._updatesInProgressCount - 1);
+        };
+
+        this._nodeStyles.changeStyleText(this, text, styleTextDidChange);
     }
 
     get properties()
@@ -332,7 +359,7 @@
         for (let index = propertyIndex + 1; index < this._allProperties.length; index++)
             this._allProperties[index].index = index;
 
-        this.update(this._text, this._allProperties, this._styleSheetTextRange, {dontFireEvents: true, suppressLock: true});
+        this.update(this._text, this._allProperties, this._styleSheetTextRange, {dontFireEvents: true, forceUpdate: true});
 
         return property;
     }

Modified: branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js (241090 => 241091)


--- branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js	2019-02-06 22:18:06 UTC (rev 241090)
+++ branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js	2019-02-06 22:18:10 UTC (rev 241091)
@@ -449,21 +449,25 @@
         return result.promise;
     }
 
-    changeStyleText(style, text)
+    changeStyleText(style, text, callback)
     {
-        if (!style.ownerStyleSheet || !style.styleSheetTextRange)
+        if (!style.ownerStyleSheet || !style.styleSheetTextRange) {
+            callback();
             return;
+        }
 
         text = text || "";
 
-        function styleChanged(error, stylePayload)
-        {
-            if (error)
+        let didSetStyleText = (error, stylePayload) => {
+            if (error) {
+                callback(error);
                 return;
-            this.refresh();
-        }
+            }
 
-        CSSAgent.setStyleText(style.id, text, styleChanged.bind(this));
+            this.refresh().then(callback);
+        };
+
+        CSSAgent.setStyleText(style.id, text, didSetStyleText);
     }
 
     // Private

Modified: branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js (241090 => 241091)


--- branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js	2019-02-06 22:18:06 UTC (rev 241090)
+++ branches/safari-607-branch/Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js	2019-02-06 22:18:10 UTC (rev 241091)
@@ -75,8 +75,6 @@
 
     get element() { return this._element; }
     get property() { return this._property; }
-    get nameTextField() { return this._nameTextField; }
-    get valueTextField() { return this._valueTextField; }
     get enabled() { return this._property.enabled; }
 
     set index(index)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to