Web Inspector: Use Array.shallowEqual instead of Object.shallowEqual in more places
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Sep 2016 20:12:07 +0000 (20:12 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Sep 2016 20:12:07 +0000 (20:12 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/inspector/unit-tests/array-utilities-expected.txt
LayoutTests/inspector/unit-tests/array-utilities.html
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Base/Utilities.js
Source/WebInspectorUI/UserInterface/Models/CSSRule.js
Source/WebInspectorUI/UserInterface/Models/Color.js
Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
Source/WebInspectorUI/UserInterface/Models/Geometry.js
Source/WebInspectorUI/UserInterface/Views/GeneralTreeElement.js
Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js

index 6d9d7e5..33d0df7 100644 (file)
@@ -1,3 +1,17 @@
+2016-09-13  Matt Baker  <mattbaker@apple.com>
+
+        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  <timothy_horton@apple.com>
 
         Undoing a candidate insertion results in the replaced text being selected
index fa2aea3..4fbb2ae 100644 (file)
@@ -33,3 +33,15 @@ PASS: partition should handle duplicates.
 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.
+
index e08469b..a0107a2 100644 (file)
@@ -12,17 +12,17 @@ function test()
         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 @@ function test()
         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 @@ function test()
             // 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,22 +66,47 @@ function test()
     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;
         }
index 1473110..b382205 100644 (file)
@@ -1,3 +1,31 @@
+2016-09-13  Matt Baker  <mattbaker@apple.com>
+
+        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  <pecoraro@apple.com>
 
         Web Inspector: Should be able to pretty print module code (import / export statements)
index 6edfb5c..ccfd2dc 100644 (file)
@@ -56,7 +56,7 @@ Object.defineProperty(Object, "shallowEqual",
             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 @@ Object.defineProperty(Array, "shallowEqual",
 {
     value: function(a, b)
     {
+        if (!Array.isArray(a) || !Array.isArray(b))
+            return false;
+
         if (a === b)
             return true;
 
index f14104c..b20a965 100644 (file)
@@ -67,8 +67,8 @@ WebInspector.CSSRule = class CSSRule extends WebInspector.Object
 
         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.
         }
index ab49540..6874152 100644 (file)
@@ -361,10 +361,10 @@ WebInspector.Color = class Color
             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()
index ec6f521..b00b743 100644 (file)
@@ -172,7 +172,7 @@ WebInspector.DOMNodeStyles = class DOMNodeStyles extends WebInspector.Object
             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").
index 7c775ab..2fde0a5 100644 (file)
@@ -397,7 +397,7 @@ WebInspector.CubicBezier = class CubicBezier
     {
         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;
         }
 
index e4a8895..c30bd7e 100644 (file)
@@ -81,7 +81,7 @@ WebInspector.GeneralTreeElement = class GeneralTreeElement extends WebInspector.
         if (typeof x === "string")
             x = [x];
 
-        if (Object.shallowEqual(this._classNames, x))
+        if (Array.shallowEqual(this._classNames, x))
             return;
 
         if (this._listItemNode && this._classNames)
index 94bcf99..e56ab5f 100644 (file)
@@ -134,7 +134,7 @@ WebInspector.NewTabContentView = class NewTabContentView extends WebInspector.Ta
         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;