Summary: Use Array.shallowEqual instead of Object.shallowEqual in more places. Crude perf testing shows the Array version to be ~30x faster. Array.shallowEqual should also check the type of its arguments before checking for equality. The following currently return true: Array.shallowEqual(null, null) Array.shallowEqual("abc", "abc") Array.shallowEqual(1.23, 1.23) ...
<rdar://problem/28261328>
Created attachment 288579 [details] [Patch] Proposed Fix
Comment on attachment 288579 [details] [Patch] Proposed Fix Attachment 288579 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2060313 New failing tests: inspector/unit-tests/array-utilities.html
Created attachment 288590 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 288579 [details] [Patch] Proposed Fix Attachment 288579 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2060311 New failing tests: inspector/unit-tests/array-utilities.html
Created attachment 288592 [details] Archive of layout-test-results from ews112 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 288579 [details] [Patch] Proposed Fix Attachment 288579 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2060363 New failing tests: inspector/unit-tests/array-utilities.html
Created attachment 288593 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Created attachment 288596 [details] [Patch] Proposed Fix
Comment on attachment 288596 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=288596&action=review > Source/WebInspectorUI/UserInterface/Base/Utilities.js:439 > + if (!(a instanceof Array) || !(b instanceof Array)) Why not just use Array.isArray()?
(In reply to comment #10) > Comment on attachment 288596 [details] > [Patch] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=288596&action=review > > > Source/WebInspectorUI/UserInterface/Base/Utilities.js:439 > > + if (!(a instanceof Array) || !(b instanceof Array)) > > Why not just use Array.isArray()? `instanceof Array` is far more prevalent in the inspector, but apparently has some shortcomings addressed by `Array.isArray()`: http://web.mit.edu/jwalden/www/isArray.html. I think it has nicer syntax too. Will change.
Created attachment 288606 [details] [Patch] Proposed Fix
Comment on attachment 288606 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=288606&action=review r=me > LayoutTests/inspector/unit-tests/array-utilities.html:102 > + InspectorTest.expectThat(Array.shallowEqual(new Array(...arr1), new Array(...arr1)), "shallowEqual of equal arrays should be true."); > + InspectorTest.expectThat(Array.shallowEqual(new Array(...arr1), arr1), "shallowEqual of equal arrays should be true."); Would it be clearer writing these as [...arr1]? I don't thin we ever use the Array constructor, and I did a double take.
Comment on attachment 288606 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=288606&action=review >> LayoutTests/inspector/unit-tests/array-utilities.html:102 >> + InspectorTest.expectThat(Array.shallowEqual(new Array(...arr1), arr1), "shallowEqual of equal arrays should be true."); > > Would it be clearer writing these as [...arr1]? I don't thin we ever use the Array constructor, and I did a double take. I'll remove or rewrite the tests that use the Array constructor.
I'm also going to replace this function used by the Array.prototype.partition test: let quickEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b); with: Array.shallowEqual, which is faster.
Created attachment 288718 [details] [Patch] For Landing
Comment on attachment 288718 [details] [Patch] For Landing Clearing flags on attachment: 288718 Committed r205872: <http://trac.webkit.org/changeset/205872>
All reviewed patches have been landed. Closing bug.