Title: [276975] trunk/Source/WebInspectorUI
- Revision
- 276975
- Author
- drou...@apple.com
- Date
- 2021-05-04 12:03:50 -0700 (Tue, 04 May 2021)
Log Message
Web Inspector: add assertion for `WI.View` re-entrancy
https://bugs.webkit.org/show_bug.cgi?id=224678
Reviewed by Joseph Pecoraro.
Add `console.assert` (which are removed from production builds) around each critical
`WI.View` subclass member function to prevent reentrancy.
- `initialLayout`
- `sizeDidChange`
- `layout`
- `didLayoutSubtree`
This is important because `WI.View.prototype._layoutSubtree` sets any related state after
calling any of the above (e.g. `_didInitialLayout` after `initialLayout`), meaning that any
checks for the related state (which sometimes control whether the function is called, such
as in the case of `initialLayout`) would think that the function hadn't been called yet. See
r276170 for an example of how this can happen and the potential problems it can cause.
* UserInterface/Views/View.js:
(WI.View.prototype._layoutSubtree):
* UserInterface/Base/Utilities.js:
(WI.setReentrantCheck): Added.
(WI.clearReentrantCheck): Added.
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (276974 => 276975)
--- trunk/Source/WebInspectorUI/ChangeLog 2021-05-04 18:51:25 UTC (rev 276974)
+++ trunk/Source/WebInspectorUI/ChangeLog 2021-05-04 19:03:50 UTC (rev 276975)
@@ -1,3 +1,29 @@
+2021-05-04 Devin Rousso <drou...@apple.com>
+
+ Web Inspector: add assertion for `WI.View` re-entrancy
+ https://bugs.webkit.org/show_bug.cgi?id=224678
+
+ Reviewed by Joseph Pecoraro.
+
+ Add `console.assert` (which are removed from production builds) around each critical
+ `WI.View` subclass member function to prevent reentrancy.
+ - `initialLayout`
+ - `sizeDidChange`
+ - `layout`
+ - `didLayoutSubtree`
+ This is important because `WI.View.prototype._layoutSubtree` sets any related state after
+ calling any of the above (e.g. `_didInitialLayout` after `initialLayout`), meaning that any
+ checks for the related state (which sometimes control whether the function is called, such
+ as in the case of `initialLayout`) would think that the function hadn't been called yet. See
+ r276170 for an example of how this can happen and the potential problems it can cause.
+
+ * UserInterface/Views/View.js:
+ (WI.View.prototype._layoutSubtree):
+
+ * UserInterface/Base/Utilities.js:
+ (WI.setReentrantCheck): Added.
+ (WI.clearReentrantCheck): Added.
+
2021-05-04 Razvan Caliman <rcali...@apple.com>
Web Inspector: Sources: Inconsistent selection in source tree when grouped by path
Modified: trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js (276974 => 276975)
--- trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js 2021-05-04 18:51:25 UTC (rev 276974)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js 2021-05-04 19:03:50 UTC (rev 276975)
@@ -1785,3 +1785,17 @@
{
array.splice(insertionIndexForObjectInListSortedByFunction(object, array, comparator), 0, object);
}
+
+WI.setReentrantCheck = function(object, key)
+{
+ key = "__checkReentrant_" + key;
+ object[key] = (object[key] || 0) + 1;
+ return object[key] === 1;
+};
+
+WI.clearReentrantCheck = function(object, key)
+{
+ key = "__checkReentrant_" + key;
+ object[key] = (object[key] || 0) - 1;
+ return object[key] === 0;
+};
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/View.js (276974 => 276975)
--- trunk/Source/WebInspectorUI/UserInterface/Views/View.js 2021-05-04 18:51:25 UTC (rev 276974)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/View.js 2021-05-04 19:03:50 UTC (rev 276975)
@@ -277,12 +277,16 @@
let isInitialLayout = !this._didInitialLayout;
if (isInitialLayout) {
+ console.assert(WI.setReentrantCheck(this, "initialLayout"), "ERROR: calling `initialLayout` while already in it", this);
this.initialLayout();
this._didInitialLayout = true;
}
- if (this._layoutReason === WI.View.LayoutReason.Resize || isInitialLayout)
+ if (this._layoutReason === WI.View.LayoutReason.Resize || isInitialLayout) {
+ console.assert(WI.setReentrantCheck(this, "sizeDidChange"), "ERROR: calling `sizeDidChange` while already in it", this);
this.sizeDidChange();
+ console.assert(WI.clearReentrantCheck(this, "sizeDidChange"), "ERROR: missing return from `sizeDidChange`", this);
+ }
let savedLayoutReason = this._layoutReason;
if (isInitialLayout) {
@@ -290,7 +294,9 @@
this._setLayoutReason();
}
+ console.assert(WI.setReentrantCheck(this, "layout"), "ERROR: calling `layout` while already in it", this);
this.layout();
+ console.assert(WI.clearReentrantCheck(this, "layout"), "ERROR: missing return from `layout`", this);
// Ensure that the initial layout override doesn't affects to subviews.
this._layoutReason = savedLayoutReason;
@@ -305,7 +311,9 @@
this._layoutReason = null;
+ console.assert(WI.setReentrantCheck(this, "didLayoutSubtree"), "ERROR: calling `didLayoutSubtree` while already in it", this);
this.didLayoutSubtree();
+ console.assert(WI.clearReentrantCheck(this, "didLayoutSubtree"), "ERROR: missing return from `didLayoutSubtree`", this);
}
_setLayoutReason(layoutReason)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
