Bug 167154

Summary: Implement URLSearchParams's sort()
Product: WebKit Reporter: Anne van Kesteren <annevk>
Component: DOMAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, commit-queue, esprehn+autocc, gyuyoung.kim, kondapallykalyan
Priority: P2    
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch sam: review+

Anne van Kesteren
Reported 2017-01-18 00:06:49 PST
Attachments
Patch (5.26 KB, patch)
2017-01-18 18:19 PST, Alex Christensen
sam: review+
Alex Christensen
Comment 1 2017-01-18 18:19:25 PST
Alex Christensen
Comment 2 2017-01-18 18:30:04 PST
By the way, Anne, all the existing tests pass if I use std::sort instead of std::stable_sort. I would suggest adding a longer test like this to the web platform tests to verify the stability of sorting algorithms: { "input": "z=z&a=a&z=y&a=b&z=x&a=c&z=w&a=d&z=v&a=e&z=u&a=f&z=t&a=g", "output": [["a", "a"], ["a", "b"], ["a", "c"], ["a", "d"], ["a", "e"], ["a", "f"], ["a", "g"], ["z", "z"], ["z", "y"], ["z", "x"], ["z", "w"], ["z", "v"], ["z", "u"], ["z", "t"]] },
Sam Weinig
Comment 3 2017-01-18 18:41:48 PST
Comment on attachment 299208 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299208&action=review > Source/WebCore/html/URLSearchParams.cpp:64 > + std::stable_sort(m_pairs.begin(), m_pairs.end(), [] (auto a, auto b) { Can we add some type of modifier to these autos as I am not sure if this is causing copying / a.k.a. ref-churn for each lambda call.
Anne van Kesteren
Comment 4 2017-01-18 23:27:47 PST
Alex: https://github.com/w3c/web-platform-tests/pull/4563 (feel free to create them yourself next time). Also, I'm curious, since I did try to test for stable sort, when does the difference happen? Or does it vary on implementation strategy?
Alex Christensen
Comment 5 2017-01-18 23:39:30 PST
(In reply to comment #4) > Alex: https://github.com/w3c/web-platform-tests/pull/4563 (feel free to > create them yourself next time). I'm going to get better at this > > Also, I'm curious, since I did try to test for stable sort, when does the > difference happen? Or does it vary on implementation strategy? I think it's stl-implementation dependent. I think our stl implementation is from libc++ and it definitely passed all the existing tests with std::sort but the longer test failed unless it used std::stable_sort.
Alex Christensen
Comment 6 2017-01-18 23:46:59 PST
Note You need to log in before you can comment on or make changes to this bug.