Log Message
Web Inspector: Use Array.shallowEqual instead of Object.shallowEqual in more places https://bugs.webkit.org/show_bug.cgi?id=161867 <rdar://problem/28261328>
Reviewed by Joseph Pecoraro. Source/WebInspectorUI: * UserInterface/Base/Utilities.js: (value): Array.shallowEqual should return false if passed a non-array. * UserInterface/Models/CSSRule.js: (WebInspector.CSSRule.prototype.update): * UserInterface/Models/Color.js: (WebInspector.Color.prototype.isKeyword): * UserInterface/Models/DOMNodeStyles.js: (WebInspector.DOMNodeStyles.prototype.refresh.fetchedComputedStyle): (WebInspector.DOMNodeStyles.prototype.refresh): * UserInterface/Models/Geometry.js: (WebInspector.CubicBezier.prototype.toString): * UserInterface/Views/GeneralTreeElement.js: (WebInspector.GeneralTreeElement.prototype.set classNames): * UserInterface/Views/NewTabContentView.js: (WebInspector.NewTabContentView.prototype._updateShownTabs): Prefer Array.shallowEqual over Obejct.shallowEqual if the arguments will always be arrays. LayoutTests: * inspector/unit-tests/array-utilities-expected.txt: * inspector/unit-tests/array-utilities.html: Add test coverage for Array.shallowEqual. Use Array.shallowEqual instead of JSON.stringify in tests. Use expectFalse and expectEqual in tests where appropriate.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/LayoutTests/inspector/unit-tests/array-utilities-expected.txt
- trunk/LayoutTests/inspector/unit-tests/array-utilities.html
- trunk/Source/WebInspectorUI/ChangeLog
- trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js
- trunk/Source/WebInspectorUI/UserInterface/Models/CSSRule.js
- trunk/Source/WebInspectorUI/UserInterface/Models/Color.js
- trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
- trunk/Source/WebInspectorUI/UserInterface/Models/Geometry.js
- trunk/Source/WebInspectorUI/UserInterface/Views/GeneralTreeElement.js
- trunk/Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js
Diff
Modified: trunk/LayoutTests/ChangeLog (205871 => 205872)
--- trunk/LayoutTests/ChangeLog 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/LayoutTests/ChangeLog 2016-09-13 20:12:07 UTC (rev 205872)
@@ -1,3 +1,17 @@
+2016-09-13 Matt Baker <[email protected]>
+
+ Web Inspector: Use Array.shallowEqual instead of Object.shallowEqual in more places
+ https://bugs.webkit.org/show_bug.cgi?id=161867
+ <rdar://problem/28261328>
+
+ Reviewed by Joseph Pecoraro.
+
+ * inspector/unit-tests/array-utilities-expected.txt:
+ * inspector/unit-tests/array-utilities.html:
+ Add test coverage for Array.shallowEqual.
+ Use Array.shallowEqual instead of JSON.stringify in tests.
+ Use expectFalse and expectEqual in tests where appropriate.
+
2016-09-13 Tim Horton <[email protected]>
Undoing a candidate insertion results in the replaced text being selected
Modified: trunk/LayoutTests/inspector/unit-tests/array-utilities-expected.txt (205871 => 205872)
--- trunk/LayoutTests/inspector/unit-tests/array-utilities-expected.txt 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/LayoutTests/inspector/unit-tests/array-utilities-expected.txt 2016-09-13 20:12:07 UTC (rev 205872)
@@ -33,3 +33,15 @@
PASS: partition should produce an empty list for the negative side.
PASS: partition should produce an empty list for the positive side.
+-- Running test case: Array.shallowEqual
+PASS: shallowEqual of empty arrays should be true.
+PASS: shallowEqual of an array with itself should be true.
+PASS: shallowEqual of equal arrays should be true.
+PASS: shallowEqual of equal arrays should be true.
+PASS: shallowEqual of inequal arrays should be false.
+PASS: shallowEqual of inequal arrays should be false.
+PASS: shallowEqual of an array and null should be false.
+PASS: shallowEqual of an array and non-array should be false.
+PASS: shallowEqual of a non-array with itself should be false.
+PASS: shallowEqual of non-arrays should be false.
+
Modified: trunk/LayoutTests/inspector/unit-tests/array-utilities.html (205871 => 205872)
--- trunk/LayoutTests/inspector/unit-tests/array-utilities.html 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/LayoutTests/inspector/unit-tests/array-utilities.html 2016-09-13 20:12:07 UTC (rev 205872)
@@ -12,17 +12,17 @@
test: () => {
// Index: 0 1 2 3 4 5 6 7 8 9
let arr = [0, 1, 2, 2, 2, 2, 2, 2, 6, 7];
- InspectorTest.expectThat(arr.lowerBound(-100) === 0, "lowerBound of a value before the first value should produce the first index.");
- InspectorTest.expectThat(arr.lowerBound(0) === 0, "lowerBound of a value in the list should return the value's index.");
- InspectorTest.expectThat(arr.lowerBound(1) === 1, "lowerBound of a value in the list should return the value's index.");
- InspectorTest.expectThat(arr.lowerBound(7) === 9, "lowerBound of a value in the list should return the value's index.");
- InspectorTest.expectThat(arr.lowerBound(2) === 2, "lowerBound of a duplicate value in the list should return the value's first index.");
- InspectorTest.expectThat(arr.lowerBound(5) === 8, "lowerBound of a value in a gap in the list should return the index where the value would be.");
- InspectorTest.expectThat(arr.lowerBound(100) === arr.length, "lowerBound of a value after the last value should produce the index after the last index (length).");
+ InspectorTest.expectEqual(arr.lowerBound(-100), 0, "lowerBound of a value before the first value should produce the first index.");
+ InspectorTest.expectEqual(arr.lowerBound(0), 0, "lowerBound of a value in the list should return the value's index.");
+ InspectorTest.expectEqual(arr.lowerBound(1), 1, "lowerBound of a value in the list should return the value's index.");
+ InspectorTest.expectEqual(arr.lowerBound(7), 9, "lowerBound of a value in the list should return the value's index.");
+ InspectorTest.expectEqual(arr.lowerBound(2), 2, "lowerBound of a duplicate value in the list should return the value's first index.");
+ InspectorTest.expectEqual(arr.lowerBound(5), 8, "lowerBound of a value in a gap in the list should return the index where the value would be.");
+ InspectorTest.expectEqual(arr.lowerBound(100), arr.length, "lowerBound of a value after the last value should produce the index after the last index (length).");
let objs = [{size: 100}, {size: 200}, {size: 300}];
let comparator = (value, object) => value - object.size;
- InspectorTest.expectThat(objs.lowerBound(150, comparator) === 1, "lowerBound with a comparator should invoke the comparator with the search value and a value in the list.");
+ InspectorTest.expectEqual(objs.lowerBound(150, comparator), 1, "lowerBound with a comparator should invoke the comparator with the search value and a value in the list.");
return true;
}
@@ -33,17 +33,17 @@
test: () => {
// Index: 0 1 2 3 4 5 6 7 8 9
let arr = [0, 1, 2, 2, 2, 2, 2, 2, 6, 7];
- InspectorTest.expectThat(arr.upperBound(-100) === 0, "upperBound of a value before the first value should produce the first index.");
- InspectorTest.expectThat(arr.upperBound(0) === 1, "upperBound of a value in the list should return the next index after the value.");
- InspectorTest.expectThat(arr.upperBound(1) === 2, "upperBound of a value in the list should return the next index after the value.");
- InspectorTest.expectThat(arr.upperBound(7) === 10, "upperBound of a value in the list should return the next index after the value.");
- InspectorTest.expectThat(arr.upperBound(2) === 8, "upperBound of a duplicate value in the list should return the next index after the value's last index.");
- InspectorTest.expectThat(arr.upperBound(5) === 8, "upperBound of a value in a gap in the list should return the index where the value would be.");
- InspectorTest.expectThat(arr.upperBound(100) === arr.length, "upperBound of a value after the last value should produce the index after the last index (length).");
+ InspectorTest.expectEqual(arr.upperBound(-100), 0, "upperBound of a value before the first value should produce the first index.");
+ InspectorTest.expectEqual(arr.upperBound(0), 1, "upperBound of a value in the list should return the next index after the value.");
+ InspectorTest.expectEqual(arr.upperBound(1), 2, "upperBound of a value in the list should return the next index after the value.");
+ InspectorTest.expectEqual(arr.upperBound(7), 10, "upperBound of a value in the list should return the next index after the value.");
+ InspectorTest.expectEqual(arr.upperBound(2), 8, "upperBound of a duplicate value in the list should return the next index after the value's last index.");
+ InspectorTest.expectEqual(arr.upperBound(5), 8, "upperBound of a value in a gap in the list should return the index where the value would be.");
+ InspectorTest.expectEqual(arr.upperBound(100), arr.length, "upperBound of a value after the last value should produce the index after the last index (length).");
let objs = [{size: 100}, {size: 200}, {size: 300}];
let comparator = (value, object) => value - object.size;
- InspectorTest.expectThat(objs.upperBound(150, comparator) === 1, "upperBound with a comparator should invoke the comparator with the search value and a value in the list.");
+ InspectorTest.expectEqual(objs.upperBound(150, comparator), 1, "upperBound with a comparator should invoke the comparator with the search value and a value in the list.");
return true;
}
@@ -55,10 +55,10 @@
// Index: 0 1 2 3 4 5 6 7 8 9
let arr = [0, 1, 2, 2, 2, 2, 2, 2, 6, 7];
let defaultComparator = (a, b) => a - b;
- InspectorTest.expectThat(arr.binaryIndexOf(-100, defaultComparator) === -1, "binaryIndexOf of a value not in the list should be -1.");
- InspectorTest.expectThat(arr.binaryIndexOf(100, defaultComparator) === -1, "binaryIndexOf of a value not in the list should be -1.");
- InspectorTest.expectThat(arr.binaryIndexOf(0, defaultComparator) === arr.lowerBound(0), "binaryIndexOf of a value in the list should return the index of the value.");
- InspectorTest.expectThat(arr.binaryIndexOf(2, defaultComparator) === arr.lowerBound(2), "binaryIndexOf of a duplicate value in the list should return the first index of the value.");
+ InspectorTest.expectEqual(arr.binaryIndexOf(-100, defaultComparator), -1, "binaryIndexOf of a value not in the list should be -1.");
+ InspectorTest.expectEqual(arr.binaryIndexOf(100, defaultComparator), -1, "binaryIndexOf of a value not in the list should be -1.");
+ InspectorTest.expectEqual(arr.binaryIndexOf(0, defaultComparator), arr.lowerBound(0), "binaryIndexOf of a value in the list should return the index of the value.");
+ InspectorTest.expectEqual(arr.binaryIndexOf(2, defaultComparator), arr.lowerBound(2), "binaryIndexOf of a duplicate value in the list should return the first index of the value.");
return true;
}
});
@@ -66,27 +66,52 @@
suite.addTestCase({
name: "Array.prototype.partition",
test: () => {
- let quickEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b);
-
let arr1 = [1, 2, 3, 4];
let [even, odd] = arr1.partition((x) => x % 2 === 0);
- InspectorTest.expectThat(even.length + odd.length === arr1.length, "partition should not lose any elements.");
- InspectorTest.expectThat(quickEqual(even, [2, 4]) && quickEqual(odd, [1, 3]), "partition should keep the order of elements in the sublists.");
+ InspectorTest.expectEqual(even.length + odd.length, arr1.length, "partition should not lose any elements.");
+ InspectorTest.expectThat(Array.shallowEqual(even, [2, 4]) && Array.shallowEqual(odd, [1, 3]), "partition should keep the order of elements in the sublists.");
let arr2 = [1, 1, -1, -2, 5, -2, -6, 0];
let [positive, negative] = arr2.partition((x) => x >= 0);
- InspectorTest.expectThat(quickEqual(positive, [1, 1, 5, 0]) && quickEqual(negative, [-1, -2, -2, -6]), "partition should handle duplicates.");
+ InspectorTest.expectThat(Array.shallowEqual(positive, [1, 1, 5, 0]) && Array.shallowEqual(negative, [-1, -2, -2, -6]), "partition should handle duplicates.");
let arr3 = [1, 2];
let [full, empty] = arr3.partition((x) => true);
- InspectorTest.expectThat(quickEqual(full, [1, 2]) && !empty.length, "partition should produce an empty list for the negative side.");
+ InspectorTest.expectThat(Array.shallowEqual(full, [1, 2]) && !empty.length, "partition should produce an empty list for the negative side.");
[empty, full] = arr3.partition((x) => false);
- InspectorTest.expectThat(quickEqual(full, [1, 2]) && !empty.length, "partition should produce an empty list for the positive side.");
+ InspectorTest.expectThat(Array.shallowEqual(full, [1, 2]) && !empty.length, "partition should produce an empty list for the positive side.");
return true;
}
});
+ suite.addTestCase({
+ name: "Array.shallowEqual",
+ test: () => {
+ InspectorTest.expectThat(Array.shallowEqual([], []), "shallowEqual of empty arrays should be true.");
+
+ let arr1 = [1, 2, 3, 4];
+ InspectorTest.expectThat(Array.shallowEqual(arr1, arr1), "shallowEqual of an array with itself should be true.");
+
+ let arr2 = [1, 2, 3, 4];
+ InspectorTest.expectThat(Array.shallowEqual(arr1, arr2), "shallowEqual of equal arrays should be true.");
+ InspectorTest.expectThat(Array.shallowEqual(arr2, arr1), "shallowEqual of equal arrays should be true.");
+
+ let arr3 = [1, 2, 3, 4, 5];
+ InspectorTest.expectFalse(Array.shallowEqual(arr1, arr3), "shallowEqual of inequal arrays should be false.");
+ InspectorTest.expectFalse(Array.shallowEqual(arr3, arr1), "shallowEqual of inequal arrays should be false.");
+
+ InspectorTest.expectFalse(Array.shallowEqual([], null), "shallowEqual of an array and null should be false.");
+ InspectorTest.expectFalse(Array.shallowEqual([], 1.23), "shallowEqual of an array and non-array should be false.");
+
+ let str = "abc";
+ InspectorTest.expectFalse(Array.shallowEqual(str, str), "shallowEqual of a non-array with itself should be false.");
+ InspectorTest.expectFalse(Array.shallowEqual({}, {}), "shallowEqual of non-arrays should be false.");
+
+ return true;
+ }
+ });
+
suite.runTestCasesAndFinish();
}
</script>
Modified: trunk/Source/WebInspectorUI/ChangeLog (205871 => 205872)
--- trunk/Source/WebInspectorUI/ChangeLog 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/Source/WebInspectorUI/ChangeLog 2016-09-13 20:12:07 UTC (rev 205872)
@@ -1,3 +1,31 @@
+2016-09-13 Matt Baker <[email protected]>
+
+ Web Inspector: Use Array.shallowEqual instead of Object.shallowEqual in more places
+ https://bugs.webkit.org/show_bug.cgi?id=161867
+ <rdar://problem/28261328>
+
+ Reviewed by Joseph Pecoraro.
+
+ * UserInterface/Base/Utilities.js:
+ (value):
+ Array.shallowEqual should return false if passed a non-array.
+
+ * UserInterface/Models/CSSRule.js:
+ (WebInspector.CSSRule.prototype.update):
+ * UserInterface/Models/Color.js:
+ (WebInspector.Color.prototype.isKeyword):
+ * UserInterface/Models/DOMNodeStyles.js:
+ (WebInspector.DOMNodeStyles.prototype.refresh.fetchedComputedStyle):
+ (WebInspector.DOMNodeStyles.prototype.refresh):
+ * UserInterface/Models/Geometry.js:
+ (WebInspector.CubicBezier.prototype.toString):
+ * UserInterface/Views/GeneralTreeElement.js:
+ (WebInspector.GeneralTreeElement.prototype.set classNames):
+ * UserInterface/Views/NewTabContentView.js:
+ (WebInspector.NewTabContentView.prototype._updateShownTabs):
+ Prefer Array.shallowEqual over Obejct.shallowEqual if the arguments
+ will always be arrays.
+
2016-09-13 Joseph Pecoraro <[email protected]>
Web Inspector: Should be able to pretty print module code (import / export statements)
Modified: trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js (205871 => 205872)
--- trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js 2016-09-13 20:12:07 UTC (rev 205872)
@@ -56,7 +56,7 @@
return true;
// Use an optimized version of shallowEqual for arrays.
- if ((a instanceof Array) && (b instanceof Array))
+ if (Array.isArray(a) && Array.isArray(b))
return Array.shallowEqual(a, b);
if (a.constructor !== b.constructor)
@@ -436,6 +436,9 @@
{
value: function(a, b)
{
+ if (!Array.isArray(a) || !Array.isArray(b))
+ return false;
+
if (a === b)
return true;
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSRule.js (205871 => 205872)
--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSRule.js 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSRule.js 2016-09-13 20:12:07 UTC (rev 205872)
@@ -67,8 +67,8 @@
var changed = false;
if (!dontFireEvents) {
- changed = this._selectorText !== selectorText || !Object.shallowEqual(this._selectors, selectors) ||
- !Object.shallowEqual(this._matchedSelectorIndices, matchedSelectorIndices) || this._style !== style ||
+ changed = this._selectorText !== selectorText || !Array.shallowEqual(this._selectors, selectors) ||
+ !Array.shallowEqual(this._matchedSelectorIndices, matchedSelectorIndices) || this._style !== style ||
!!this._sourceCodeLocation !== !!sourceCodeLocation || this._mediaList.length !== mediaList.length;
// FIXME: Look for differences in the media list arrays.
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/Color.js (205871 => 205872)
--- trunk/Source/WebInspectorUI/UserInterface/Models/Color.js 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/Color.js 2016-09-13 20:12:07 UTC (rev 205872)
@@ -361,10 +361,10 @@
return true;
if (!this.simple)
- return Object.shallowEqual(this._rgba, [0, 0, 0, 0]) || Object.shallowEqual(this._hsla, [0, 0, 0, 0]);
+ return Array.shallowEqual(this._rgba, [0, 0, 0, 0]) || Array.shallowEqual(this._hsla, [0, 0, 0, 0]);
let rgb = (this._rgba && this._rgba.slice(0, 3)) || this._hslToRGB(this._hsla);
- return Object.keys(WebInspector.Color.Keywords).some(key => Object.shallowEqual(WebInspector.Color.Keywords[key], rgb));
+ return Object.keys(WebInspector.Color.Keywords).some(key => Array.shallowEqual(WebInspector.Color.Keywords[key], rgb));
}
canBeSerializedAsShortHEX()
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js (205871 => 205872)
--- trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js 2016-09-13 20:12:07 UTC (rev 205872)
@@ -172,7 +172,7 @@
for (let key in this._styleDeclarationsMap) {
// Check if the same key exists in the previous map and has the same style objects.
if (key in this._previousStyleDeclarationsMap) {
- if (Object.shallowEqual(this._styleDeclarationsMap[key], this._previousStyleDeclarationsMap[key]))
+ if (Array.shallowEqual(this._styleDeclarationsMap[key], this._previousStyleDeclarationsMap[key]))
continue;
// Some styles have selectors such that they will match with the DOM node twice (for example "::before, ::after").
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/Geometry.js (205871 => 205872)
--- trunk/Source/WebInspectorUI/UserInterface/Models/Geometry.js 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/Geometry.js 2016-09-13 20:12:07 UTC (rev 205872)
@@ -397,7 +397,7 @@
{
var values = [this._inPoint.x, this._inPoint.y, this._outPoint.x, this._outPoint.y];
for (var key in WebInspector.CubicBezier.keywordValues) {
- if (Object.shallowEqual(WebInspector.CubicBezier.keywordValues[key], values))
+ if (Array.shallowEqual(WebInspector.CubicBezier.keywordValues[key], values))
return key;
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/GeneralTreeElement.js (205871 => 205872)
--- trunk/Source/WebInspectorUI/UserInterface/Views/GeneralTreeElement.js 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/GeneralTreeElement.js 2016-09-13 20:12:07 UTC (rev 205872)
@@ -81,7 +81,7 @@
if (typeof x === "string")
x = [x];
- if (Object.shallowEqual(this._classNames, x))
+ if (Array.shallowEqual(this._classNames, x))
return;
if (this._listItemNode && this._classNames)
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js (205871 => 205872)
--- trunk/Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js 2016-09-13 19:54:53 UTC (rev 205871)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js 2016-09-13 20:12:07 UTC (rev 205872)
@@ -134,7 +134,7 @@
let allowedTabClasses = allTabClasses.filter((tabClass) => tabClass.isTabAllowed() && !tabClass.isEphemeral());
allowedTabClasses.sort((a, b) => a.tabInfo().title.localeCompare(b.tabInfo().title));
- if (Object.shallowEqual(this._shownTabClasses, allowedTabClasses))
+ if (Array.shallowEqual(this._shownTabClasses, allowedTabClasses))
return;
this._shownTabClasses = allowedTabClasses;
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
