In Bug 222145 I converted serialization od CFURLRequest to NSURLRequest. As part of that change, I used a new encoder (WKSecureCodingURLWrapper) to wrap the NSURL to ensure proper handling of encoded URLs. I should have declared the WKSecureCodingURLWrapper as a subclass of NSURL, so that NSSecureCoding would respect it as a valid way of serializing an NSURL. This patch corrects that oversight.
Created attachment 422473 [details] Patch
Comment on attachment 422473 [details] Patch Can we test this somehow? Perhaps via some ArgumentCoder API/Unit tests?
(In reply to Sam Weinig from comment #2) > Comment on attachment 422473 [details] > Patch > > Can we test this somehow? Perhaps via some ArgumentCoder API/Unit tests? We could, if such infrastructure existed. It doesn't appear to. We would have caught this if there were tests for Apple Pay PaymentSetupConfiguration, but there don't appear to be any. It might require special server stuff to do so.
Comment on attachment 422473 [details] Patch It’s bad that there is no test. It’s non-obvious that this is required. It’s strange that all the fields of NSURL are allocated and all the methods of NSURL are available on this object. But we should land this code quickly to fix the major regression and think about those other things afterward.
(In reply to Brent Fulgham from comment #3) > > Can we test this somehow? Perhaps via some ArgumentCoder API/Unit tests? > > We could, if such infrastructure existed. It doesn't appear to. > > We would have caught this if there were tests for Apple Pay > PaymentSetupConfiguration, but there don't appear to be any. It might > require special server stuff to do so. I feel like we need to add this testing. It’s not optional. Every test we wrote was hard at first, time to make this testable.
Committed r274109: <https://commits.webkit.org/r274109> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422473 [details].
<rdar://problem/75188264>
(In reply to Sam Weinig from comment #2) > Comment on attachment 422473 [details] > Patch > > Can we test this somehow? Perhaps via some ArgumentCoder API/Unit tests? I got started trying to make this work in Bug 222957. Maybe you could take a look (well, once I get the tvOS/watchOS stuff fixed).
(In reply to Darin Adler from comment #5) > (In reply to Brent Fulgham from comment #3) > > > Can we test this somehow? Perhaps via some ArgumentCoder API/Unit tests? > > > > We could, if such infrastructure existed. It doesn't appear to. > > > > We would have caught this if there were tests for Apple Pay > > PaymentSetupConfiguration, but there don't appear to be any. It might > > require special server stuff to do so. > > I feel like we need to add this testing. It’s not optional. Every test we > wrote was hard at first, time to make this testable. I wrote a small test that exercises the specific Apple Pay thing that broke. I'm going to work with Andy/Alex to get something more full-featured in place. See Bug 222957.