RESOLVED FIXED 140209
Web Inspector: Type check NSArray's in ObjC Interfaces have the right object types
https://bugs.webkit.org/show_bug.cgi?id=140209
Summary Web Inspector: Type check NSArray's in ObjC Interfaces have the right object ...
Joseph Pecoraro
Reported 2015-01-07 14:20:22 PST
* SUMMARY Lacking generics, we should type check NSArray's contain the right object types. For example: @protocol RWIProtocolCSSDomainHandler <NSObject> - (void)getMatchedStylesForNodeWithErrorCallback:(void(^)(NSString *error))errorCallback successCallback:(void(^)(NSArray/*<RWIProtocolCSSRuleMatch>*/ **matchedCSSRules, NSArray/*<RWIProtocolCSSPseudoIdMatches>*/ **pseudoElements, NSArray/*<RWIProtocolCSSInheritedStyleEntry>*/ **inherited))successCallback nodeId:(int)nodeId includePseudo:(BOOL *)includePseudo includeInherited:(BOOL *)includeInherited; @end We should type check in the successCallback that "matchedCSSRules" really has RWIProtocolCSSRuleMatch objects and not RWIProtocolCSSRule objects.
Attachments
[PATCH] Proposed Fix (18.53 KB, patch)
2015-01-07 17:59 PST, Joseph Pecoraro
timothy: review+
joepeck: commit-queue-
Radar WebKit Bug Importer
Comment 1 2015-01-07 14:20:56 PST
Joseph Pecoraro
Comment 2 2015-01-07 17:58:50 PST
These are the macros being added, to be used by this patch: #define THROW_EXCEPTION_FOR_BAD_TYPE_IN_ARRAY(arrayExpr, classExpr, typeName) \ for (id arrayObject in (arrayExpr)) { \ if (![arrayObject isKindOfClass:(classExpr)]) \ [NSException raise:NSInvalidArgumentException format:@"array should contain objects of type '%@', found bad value: %@", typeName, arrayObject]; \ } #define THROW_EXCEPTION_FOR_BAD_TYPE_IN_OPTIONAL_ARRAY(arrayExpr, classExpr, typeName) \ if (arrayExpr) { \ for (id arrayObject in (*arrayExpr)) { \ if (![arrayObject isKindOfClass:(classExpr)]) \ [NSException raise:NSInvalidArgumentException format:@"array should contain objects of type '%@', found bad value: %@", typeName, arrayObject]; \ } \ } Here is an example exception: 2015-01-07 17:55:12.977 JSContextWithDomains[98468:6148319] array should contain objects of type 'NSString', found bad value: 1 2015-01-07 17:55:12.979 JSContextWithDomains[98468:6148319] ( 0 CoreFoundation 0x00007fff95d9d58c __exceptionPreprocess + 172 1 libobjc.A.dylib 0x00007fff8ee2376e objc_exception_throw + 43 2 CoreFoundation 0x00007fff95d9d43d +[NSException raise:format:] + 205 3 WebInspector 0x0000000100085129 -[RWIProtocolDOMNode setAttributes:] + 285 ...
Joseph Pecoraro
Comment 3 2015-01-07 17:59:47 PST
Created attachment 244231 [details] [PATCH] Proposed Fix Needs to correspond with an internal patch adding the macros.
WebKit Commit Bot
Comment 4 2015-01-07 18:01:31 PST
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)
Timothy Hatcher
Comment 5 2015-01-08 03:08:09 PST
Comment on attachment 244231 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=244231&action=review > Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_backend_dispatcher_implementation.py:122 > + lines.append(' THROW_EXCEPTION_FOR_BAD_TYPE_IN_ARRAY(%s, [%s class], @"%s");' % (var_name, objc_array_class, objc_array_class)) I suggested using NSStringFromClass in the macro, which would eliminate the type name string param.
Joseph Pecoraro
Comment 6 2015-01-08 13:05:07 PST
Modified this before landing (getting a quick review of the changes). Only type check for Objects, since all other types are already type checked in the ObjC <-> C++ conversion helpers. http://trac.webkit.org/changeset/178127
Note You need to log in before you can comment on or make changes to this bug.