Changeset 267352 in webkit
- Timestamp:
- Sep 21, 2020 12:17:04 PM (4 years ago)
- Location:
- trunk/Source/WebKit
- Files:
-
- 2 added
- 3 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebKit/ChangeLog
r267344 r267352 1 2020-09-21 James Savage <james.savage@apple.com> 2 3 WKWebView Swift overlay has mis-annotated nullability for evaluateJavaScript 4 <http://webkit.org/b/216198> 5 <rdar://problem/68035950> 6 7 Reviewed by Darin Adler. 8 9 Due to a mistranslation of evaluateJavaScript, we are vending an API which does not expect 10 to receive nil as a valid result value. This change fixes the crash, but does not yet fix 11 the API to have the correct signature. That will come in a later patch. 12 13 To fix the crash, we need to produce a valid Result<Any, Error> to pass back to clients. 14 Fortunately, instead of inventing something clever, we can just use nil. It's valid to box 15 optional values into Any, and clients can technically retrieve them with the right dynamic cast 16 as well. Since client code must be using dynamic casting to convert the result Any to a usable 17 type, and because in the case where we now return a new value at runtime we would have previously 18 crashed, this shouldn't have any binary compatibility impact either. 19 20 To better validate these changes, I also add new unit tests for the conversion of JavaScript 21 results into Swift values, including a test for the deprecated API. 22 23 * SwiftOverlay/SwiftOverlay/ObjectiveCBlockConversions.swift: Added. For clarity I'm factoring 24 helper methods into a single namespace, as it also makes a nice place to document their expectations. 25 (ObjectiveCBlockConversion.exclusive.exclusive(_:)): This is renamed from the free function, 26 makeResultHandler(_:). It still has the same fatalError (now precondition) as before, but 27 hopefully a better name to clarify that it expects exactly-one value. 28 (ObjectiveCBlockConversions.boxingNilAsAnyForCompatibility(_:)): This is a variant of exclusive(_:) 29 that makes the tradeoff of boxing any nil values as Any to avoid crashing. This is still safe, 30 since as mentioned our clients will need to cast the value they recieve to do anything with it, 31 and since the deprecated API expects `Any`, no one could have been successfully comparing it 32 to `nil` today anyways. 33 34 * SwiftOverlay/Tests/JavaScriptToSwiftTypeConversions.swift: Added. 35 (JavaScriptToSwiftConversions.setUp): Construct a new web view, and add it to a window so that it is 36 in an expected state. I'm using about:blank as the URL, since page content doesn't matter for 37 these tests and I want the web content to be ready immediately. 38 (JavaScriptToSwiftConversions.tearDown): Just perform some window cleanup. 39 (JavaScriptToSwiftConversions.evaluateJavaScript(_:andExpect:)): Helper method to evaluate script and 40 check its result. I'm using String.debugDescription because it escapes quotes and special characters 41 which makes the readout easier to parse. 42 (JavaScriptToSwiftConversions.testNull): JavaScript's null is actually mapped to NSNull, not nil. 43 (JavaScriptToSwiftConversions.testInteger): Some standard type coercion tests. The underlying value for 44 all number types should be NSNumber, so this is actually check against a float or integer type 45 without issue. 46 (JavaScriptToSwiftConversions.testDecimal): Ditto. 47 (JavaScriptToSwiftConversions.testBoolean): Ditto. 48 (JavaScriptToSwiftConversions.testString): Ditto. 49 (JavaScriptToSwiftConversions.testArray): Ditto. 50 (JavaScriptToSwiftConversions.testDictionary): Ditto, only you can't evaluate an object literal directly 51 so I need to store it in a temporary location first. 52 (JavaScriptToSwiftConversions.testUndefined): Test our boxing of nil. The exact value matters less than 53 not crashing at all. 54 55 * SwiftOverlay/WebKitSwiftOverlay.xcodeproj/project.pbxproj: Added new files to project. I kept the 56 new test file and helper files within the SwiftOverlay group, because they do not contribute any API. 57 58 * UIProcess/API/Cocoa/WebKitSwiftOverlay.swift: 59 (WKWebView.callAsyncJavaScript(_:arguments:in:in:completionHandler:)): Switch to a conversion which does 60 not trap on nil. 61 (WKWebView.createPDF(_:completionHandler:)): Updated to use new helper method name. 62 (WKWebView.createWebArchiveData(_:)): Ditto. 63 (WKWebView.evaluateJavaScript(_:in:in:completionHandler:Error:)): See above. 64 (makeResultHandler(_:): This has been subsumed by ObjCBlockConversions. 65 1 66 2020-09-21 Kate Cheney <katherine_cheney@apple.com> 2 67 -
trunk/Source/WebKit/SwiftOverlay/WebKitSwiftOverlay.xcodeproj/project.pbxproj
r262089 r267352 14 14 B3A5D39223F790DB00B17727 /* WebKitSwiftOverlay.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3A5D39123F790DB00B17727 /* WebKitSwiftOverlay.swift */; }; 15 15 B3A5D39323F790DB00B17727 /* WebKitSwiftOverlay.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3A5D39123F790DB00B17727 /* WebKitSwiftOverlay.swift */; }; 16 B3B8FECC250090B1006172CA /* JavaScriptToSwiftTypeConversions.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3B8FECB250090B1006172CA /* JavaScriptToSwiftTypeConversions.swift */; }; 17 B3B8FECD250090B1006172CA /* JavaScriptToSwiftTypeConversions.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3B8FECB250090B1006172CA /* JavaScriptToSwiftTypeConversions.swift */; }; 18 B3B8FEEF2502BAA0006172CA /* ObjectiveCBlockConversions.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3B8FEEE2502BAA0006172CA /* ObjectiveCBlockConversions.swift */; }; 19 B3B8FEF02502BAA0006172CA /* ObjectiveCBlockConversions.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3B8FEEE2502BAA0006172CA /* ObjectiveCBlockConversions.swift */; }; 16 20 /* End PBXBuildFile section */ 17 21 … … 47 51 B3A5D39923F790E100B17727 /* install-swiftmodules.sh */ = {isa = PBXFileReference; lastKnownFileType = text.script.sh; path = "install-swiftmodules.sh"; sourceTree = "<group>"; }; 48 52 B3A5D39A23F790E700B17727 /* WebKitSwiftOverlayTests-Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = "WebKitSwiftOverlayTests-Info.plist"; sourceTree = "<group>"; }; 53 B3B8FECB250090B1006172CA /* JavaScriptToSwiftTypeConversions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = JavaScriptToSwiftTypeConversions.swift; sourceTree = "<group>"; }; 54 B3B8FEEE2502BAA0006172CA /* ObjectiveCBlockConversions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ObjectiveCBlockConversions.swift; sourceTree = "<group>"; }; 49 55 /* End PBXFileReference section */ 50 56 … … 87 93 children = ( 88 94 B3A5D39923F790E100B17727 /* install-swiftmodules.sh */, 95 B3B8FEEE2502BAA0006172CA /* ObjectiveCBlockConversions.swift */, 89 96 B3A5D39123F790DB00B17727 /* WebKitSwiftOverlay.swift */, 90 97 ); … … 103 110 isa = PBXGroup; 104 111 children = ( 112 B3B8FECB250090B1006172CA /* JavaScriptToSwiftTypeConversions.swift */, 105 113 B3A5D39A23F790E700B17727 /* WebKitSwiftOverlayTests-Info.plist */, 106 114 B3A5D38823F78F5400B17727 /* WebKitTests.swift */, … … 345 353 buildActionMask = 2147483647; 346 354 files = ( 355 B3B8FEEF2502BAA0006172CA /* ObjectiveCBlockConversions.swift in Sources */, 347 356 B3A5D39223F790DB00B17727 /* WebKitSwiftOverlay.swift in Sources */, 348 357 ); … … 353 362 buildActionMask = 2147483647; 354 363 files = ( 364 B3B8FEF02502BAA0006172CA /* ObjectiveCBlockConversions.swift in Sources */, 355 365 B3A5D39323F790DB00B17727 /* WebKitSwiftOverlay.swift in Sources */, 356 366 ); … … 361 371 buildActionMask = 2147483647; 362 372 files = ( 373 B3B8FECC250090B1006172CA /* JavaScriptToSwiftTypeConversions.swift in Sources */, 363 374 B3A5D38F23F78F5400B17727 /* WebKitTests.swift in Sources */, 364 375 ); … … 369 380 buildActionMask = 2147483647; 370 381 files = ( 382 B3B8FECD250090B1006172CA /* JavaScriptToSwiftTypeConversions.swift in Sources */, 371 383 B3A5D39023F78F5400B17727 /* WebKitTests.swift in Sources */, 372 384 ); -
trunk/Source/WebKit/UIProcess/API/Cocoa/WebKitSwiftOverlay.swift
r263793 r267352 37 37 extension WKWebView { 38 38 public func callAsyncJavaScript(_ functionBody: String, arguments: [String:Any] = [:], in frame: WKFrameInfo? = nil, in contentWorld: WKContentWorld, completionHandler: ((Result<Any, Error>) -> Void)? = nil) { 39 __callAsyncJavaScript(functionBody, arguments: arguments, inFrame: frame, in: contentWorld, completionHandler: completionHandler.map( makeResultHandler))39 __callAsyncJavaScript(functionBody, arguments: arguments, inFrame: frame, in: contentWorld, completionHandler: completionHandler.map(ObjCBlockConversion.boxingNilAsAnyForCompatibility)) 40 40 } 41 41 42 42 public func createPDF(configuration: WKPDFConfiguration = .init(), completionHandler: @escaping (Result<Data, Error>) -> Void) { 43 __createPDF(with: configuration, completionHandler: makeResultHandler(completionHandler))43 __createPDF(with: configuration, completionHandler: ObjCBlockConversion.exclusive(completionHandler)) 44 44 } 45 45 46 46 public func createWebArchiveData(completionHandler: @escaping (Result<Data, Error>) -> Void) { 47 __createWebArchiveData(completionHandler: makeResultHandler(completionHandler))47 __createWebArchiveData(completionHandler: ObjCBlockConversion.exclusive(completionHandler)) 48 48 } 49 49 50 50 public func evaluateJavaScript(_ javaScript: String, in frame: WKFrameInfo? = nil, in contentWorld: WKContentWorld, completionHandler: ((Result<Any, Error>) -> Void)? = nil) { 51 __evaluateJavaScript(javaScript, inFrame: frame, in: contentWorld, completionHandler: completionHandler.map( makeResultHandler))51 __evaluateJavaScript(javaScript, inFrame: frame, in: contentWorld, completionHandler: completionHandler.map(ObjCBlockConversion.boxingNilAsAnyForCompatibility)) 52 52 } 53 53 … … 56 56 } 57 57 } 58 59 func makeResultHandler<Success, Failure>(_ handler: @escaping (Result<Success, Failure>) -> Void) -> (Success?, Failure?) -> Void {60 return { success, failure in61 if let success = success {62 handler(.success(success))63 } else if let failure = failure {64 handler(.failure(failure))65 } else {66 fatalError("Bug in WebKit: Received neither result or failure.")67 }68 }69 }
Note: See TracChangeset
for help on using the changeset viewer.