We got too strict.
<rdar://problem/32338354>
Created attachment 311046 [details] Patch
Comment on attachment 311046 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311046&action=review > Source/JavaScriptCore/ChangeLog:14 > + WIRAutomatically is an optional key in the setup message. In the relay, this key gets copied > + into an NSDictionary as NSNull if the key isn't present in a forwarded command. I don't see how this can happen. It could be optional but as it exists right now I don't think it is. > Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm:58 > +#define BAIL_IF_UNEXPECTED_TYPE_ALLOWING_NIL(expr, classExpr) \ > + do { \ > + id value = (expr); \ > + if ([value isEqualTo:[NSNull null]]) \ > + expr = value = nil; \ This should be named differently since we have the ALLOWING_NIL name in WebInspector which doesn't check for NSNull. Also its hiding an operation that I don't think should be hidden. I'd rather see two operations: CONVERT_NSNULL_TO_NIL(automaticallyPauseNumber); BAIL_IF_UNEXPECTED_TYPE_ALLOWING_NIL(...); That said, I still don't believe it should even be NSNull.
(In reply to Joseph Pecoraro from comment #3) > Comment on attachment 311046 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=311046&action=review > > > Source/JavaScriptCore/ChangeLog:14 > > + WIRAutomatically is an optional key in the setup message. In the relay, this key gets copied > > + into an NSDictionary as NSNull if the key isn't present in a forwarded command. > > I don't see how this can happen. It could be optional but as it exists right > now I don't think it is. Oh. Apparently this is possible only for Web Driver. Thats unfortunate. > > Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm:58 > > +#define BAIL_IF_UNEXPECTED_TYPE_ALLOWING_NIL(expr, classExpr) \ > > + do { \ > > + id value = (expr); \ > > + if ([value isEqualTo:[NSNull null]]) \ > > + expr = value = nil; \ > > This should be named differently since we have the ALLOWING_NIL name in > WebInspector which doesn't check for NSNull. Also its hiding an operation > that I don't think should be hidden. I'd rather see two operations: > > CONVERT_NSNULL_TO_NIL(automaticallyPauseNumber); > BAIL_IF_UNEXPECTED_TYPE_ALLOWING_NIL(...); > > That said, I still don't believe it should even be NSNull. Splitting this up into two operations would be much clearer. r=me with that change
Created attachment 311053 [details] For landing
Created attachment 311067 [details] For landing
Comment on attachment 311067 [details] For landing Attachment 311067 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3802999 New failing tests: fetch/closing-while-fetching-blob.html fast/css/target-fragment-match.html
Created attachment 311074 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Committed r217309: <http://trac.webkit.org/changeset/217309>