From 2ff68a9c9edfe6df3673026b397a36f427fc0b99 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Fri, 8 Nov 2024 09:45:44 +1100 Subject: [PATCH] [objective_c] Handle missing protocol methods more gracefully (#1705) --- .../lib/src/code_generator/objc_protocol.dart | 1 + .../test/native_objc_test/protocol_test.dart | 20 ++++-- .../test/native_objc_test/protocol_test.h | 8 +++ .../test/native_objc_test/protocol_test.m | 2 + pkgs/objective_c/CHANGELOG.md | 5 ++ pkgs/objective_c/ffigen_c.yaml | 2 + .../lib/src/c_bindings_generated.dart | 6 ++ pkgs/objective_c/lib/src/internal.dart | 72 ++++++++++++++++--- .../src/objective_c_bindings_generated.dart | 1 + .../objective_c/lib/src/protocol_builder.dart | 26 +++++-- pkgs/objective_c/src/objective_c_runtime.h | 1 + 11 files changed, 124 insertions(+), 20 deletions(-) diff --git a/pkgs/ffigen/lib/src/code_generator/objc_protocol.dart b/pkgs/ffigen/lib/src/code_generator/objc_protocol.dart index 2a1438d84..89b311201 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_protocol.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_protocol.dart @@ -97,6 +97,7 @@ class ObjCProtocol extends NoLookUpBinding with ObjCMethods { methodFields.write(makeDartDoc(method.dartDoc ?? method.originalName)); methodFields.write('''static final $fieldName = $methodClass<$funcType>( + ${_protocolPointer.name}, ${method.selObject.name}, $getSignature( ${_protocolPointer.name}, diff --git a/pkgs/ffigen/test/native_objc_test/protocol_test.dart b/pkgs/ffigen/test/native_objc_test/protocol_test.dart index 6669d80a3..d6176506e 100644 --- a/pkgs/ffigen/test/native_objc_test/protocol_test.dart +++ b/pkgs/ffigen/test/native_objc_test/protocol_test.dart @@ -258,7 +258,7 @@ void main() { final sel = registerName('instanceMethod:withDouble:'); final signature = getProtocolMethodSignature(protocol, sel, - isRequired: true, isInstanceMethod: true); + isRequired: true, isInstanceMethod: true)!; final block = InstanceMethodBlock.fromFunction( (Pointer p, NSString s, double x) { return 'DartProxy: $s: $x'.toNSString(); @@ -268,7 +268,7 @@ void main() { final optSel = registerName('optionalMethod:'); final optSignature = getProtocolMethodSignature(protocol, optSel, - isRequired: false, isInstanceMethod: true); + isRequired: false, isInstanceMethod: true)!; final optBlock = OptionalMethodBlock.fromFunction((Pointer p, SomeStruct s) { return s.y - s.x; @@ -279,7 +279,7 @@ void main() { final otherSel = registerName('otherMethod:b:c:d:'); final otherSignature = getProtocolMethodSignature( secondProtocol, otherSel, - isRequired: true, isInstanceMethod: true); + isRequired: true, isInstanceMethod: true)!; final otherBlock = OtherMethodBlock.fromFunction( (Pointer p, int a, int b, int c, int d) { return a * b * c * d; @@ -339,7 +339,7 @@ void main() { final sel = registerName('instanceMethod:withDouble:'); final signature = getProtocolMethodSignature(protocol, sel, - isRequired: true, isInstanceMethod: true); + isRequired: true, isInstanceMethod: true)!; final block = InstanceMethodBlock.fromFunction( (Pointer p, NSString s, double x) => 'Hello'.toNSString()); proxyBuilder.implementMethod_withSignature_andBlock_( @@ -407,5 +407,17 @@ void main() { final proto = UnusedProtocol.implement(someMethod: () => 123); expect(proto, isNotNull); }); + + test('Disabled method', () { + // Regression test for https://github.com/dart-lang/native/issues/1702. + expect(MyProtocol.instanceMethod_withDouble_.isAvailable, isTrue); + expect(MyProtocol.optionalMethod_.isAvailable, isTrue); + expect(MyProtocol.disabledMethod.isAvailable, isFalse); + + expect( + () => MyProtocol.disabledMethod + .implement(ObjCProtocolBuilder(), () => 123), + throwsA(isA())); + }); }); } diff --git a/pkgs/ffigen/test/native_objc_test/protocol_test.h b/pkgs/ffigen/test/native_objc_test/protocol_test.h index 94f079c04..21262a248 100644 --- a/pkgs/ffigen/test/native_objc_test/protocol_test.h +++ b/pkgs/ffigen/test/native_objc_test/protocol_test.h @@ -36,6 +36,14 @@ typedef struct { @optional + (int32_t)unimplementedOtionalClassMethod; +// For https://github.com/dart-lang/native/issues/1702 regression test, disable +// a method (in practice this would be due to API versioning) and verify that +// the protocol builder fails gracefully. +#ifndef DISABLE_METHOD +@optional +- (int32_t)disabledMethod; +#endif + @end diff --git a/pkgs/ffigen/test/native_objc_test/protocol_test.m b/pkgs/ffigen/test/native_objc_test/protocol_test.m index cc5029d4a..91b7a7f40 100644 --- a/pkgs/ffigen/test/native_objc_test/protocol_test.m +++ b/pkgs/ffigen/test/native_objc_test/protocol_test.m @@ -4,6 +4,8 @@ #import +#define DISABLE_METHOD 1 + #include "protocol_test.h" @implementation ProtocolConsumer : NSObject diff --git a/pkgs/objective_c/CHANGELOG.md b/pkgs/objective_c/CHANGELOG.md index b78bc13ef..b0813f671 100644 --- a/pkgs/objective_c/CHANGELOG.md +++ b/pkgs/objective_c/CHANGELOG.md @@ -17,6 +17,11 @@ generate bindings for that category yourself in your own package. 2. If the category is common/important enough that it should be included in package:objective_c, file a bug and we'll consider adding it back in. +- Fixed [a bug](https://github.com/dart-lang/native/issues/1702) where missing + methods could cause runtime errors, even if they weren't being implemented. +- Throw more useful errors in all internal failure cases. +- Added `ObjCProtocolMethod.isAvailable` getter, to make it easier to implement + fallback logic if a method is missing at runtime. ## 3.0.0 diff --git a/pkgs/objective_c/ffigen_c.yaml b/pkgs/objective_c/ffigen_c.yaml index a3e39a576..d9831533f 100644 --- a/pkgs/objective_c/ffigen_c.yaml +++ b/pkgs/objective_c/ffigen_c.yaml @@ -19,6 +19,7 @@ functions: - 'sel_registerName' - 'sel_getName' - 'protocol_getMethodDescription' + - 'protocol_getName' - 'disposeObjCBlockWithClosure' - 'isValidBlock' - 'isValidObject' @@ -51,6 +52,7 @@ functions: 'objc_copyClassList': 'copyClassList' 'objc_getProtocol': 'getProtocol' 'protocol_getMethodDescription': 'getMethodDescription' + 'protocol_getName': 'getProtocolName' globals: include: - '_NSConcrete.*Block' diff --git a/pkgs/objective_c/lib/src/c_bindings_generated.dart b/pkgs/objective_c/lib/src/c_bindings_generated.dart index fa8461378..15b471697 100644 --- a/pkgs/objective_c/lib/src/c_bindings_generated.dart +++ b/pkgs/objective_c/lib/src/c_bindings_generated.dart @@ -120,6 +120,12 @@ external ffi.Pointer getProtocol( ffi.Pointer name, ); +@ffi.Native Function(ffi.Pointer)>( + symbol: "protocol_getName", isLeaf: true) +external ffi.Pointer getProtocolName( + ffi.Pointer proto, +); + @ffi.Native)>(isLeaf: true) external bool isValidBlock( ffi.Pointer block, diff --git a/pkgs/objective_c/lib/src/internal.dart b/pkgs/objective_c/lib/src/internal.dart index 5a9b7054d..b65b58b95 100644 --- a/pkgs/objective_c/lib/src/internal.dart +++ b/pkgs/objective_c/lib/src/internal.dart @@ -9,6 +9,7 @@ import 'package:ffi/ffi.dart'; import 'c_bindings_generated.dart' as c; import 'objective_c_bindings_generated.dart' as objc; +import 'selector.dart'; final class UseAfterReleaseError extends StateError { UseAfterReleaseError() : super('Use after release error'); @@ -19,12 +20,64 @@ final class DoubleReleaseError extends StateError { } final class UnimplementedOptionalMethodException implements Exception { - String clazz; - String method; + final String clazz; + final String method; UnimplementedOptionalMethodException(this.clazz, this.method); @override - String toString() => 'Instance of $clazz does not implement $method'; + String toString() => + '$runtimeType: Instance of $clazz does not implement $method'; +} + +final class FailedToLoadClassException implements Exception { + final String clazz; + FailedToLoadClassException(this.clazz); + + @override + String toString() => '$runtimeType: Failed to load Objective-C class: $clazz'; +} + +final class FailedToLoadProtocolException implements Exception { + final String protocol; + FailedToLoadProtocolException(this.protocol); + + @override + String toString() => + '$runtimeType: Failed to load Objective-C protocol: $protocol'; +} + +/// Failed to load a method of a protocol. +/// +/// This means that a method that was seen in the protocol declaration at +/// compile time was missing from the protocol at runtime. This is usually +/// caused by a version mismatch between the compile time header and the runtime +/// framework (eg, running an app on an older iOS device). +/// +/// To fix this, check whether the method exists at runtime, using +/// `ObjCProtocolMethod.isAvailable`, and implement fallback logic if it's +/// missing. +final class FailedToLoadProtocolMethodException implements Exception { + final String protocol; + final String method; + FailedToLoadProtocolMethodException(this.protocol, this.method); + + @override + String toString() => + '$runtimeType: Failed to load Objective-C protocol method: ' + '$protocol.$method'; +} + +final class ObjCRuntimeError extends Error { + final String message; + ObjCRuntimeError(this.message); + + @override + String toString() => '$runtimeType: $message'; +} + +extension GetProtocolName on Pointer { + /// Returns the name of the protocol. + String get name => c.getProtocolName(this).cast().toDartString(); } /// Only for use by ffigen bindings. @@ -41,7 +94,7 @@ Pointer getClass(String name) { final clazz = c.getClass(cstr.cast()); calloc.free(cstr); if (clazz == nullptr) { - throw Exception('Failed to load Objective-C class: $name'); + throw FailedToLoadClassException(name); } return clazz; } @@ -52,13 +105,13 @@ Pointer getProtocol(String name) { final clazz = c.getProtocol(cstr.cast()); calloc.free(cstr); if (clazz == nullptr) { - throw Exception('Failed to load Objective-C protocol: $name'); + throw FailedToLoadProtocolException(name); } return clazz; } /// Only for use by ffigen bindings. -objc.NSMethodSignature getProtocolMethodSignature( +objc.NSMethodSignature? getProtocolMethodSignature( Pointer protocol, Pointer sel, { required bool isRequired, @@ -67,12 +120,13 @@ objc.NSMethodSignature getProtocolMethodSignature( final sig = c.getMethodDescription(protocol, sel, isRequired, isInstanceMethod).types; if (sig == nullptr) { - throw Exception('Failed to load method of Objective-C protocol'); + return null; } final sigObj = objc.NSMethodSignature.signatureWithObjCTypes_(sig); if (sigObj == null) { - throw Exception( - 'Failed to construct signature for Objective-C protocol method'); + throw ObjCRuntimeError( + 'Failed to construct signature for Objective-C protocol method: ' + '${protocol.name}.${sel.toDartString()}'); } return sigObj; } diff --git a/pkgs/objective_c/lib/src/objective_c_bindings_generated.dart b/pkgs/objective_c/lib/src/objective_c_bindings_generated.dart index cb07072d2..ba44df465 100644 --- a/pkgs/objective_c/lib/src/objective_c_bindings_generated.dart +++ b/pkgs/objective_c/lib/src/objective_c_bindings_generated.dart @@ -6116,6 +6116,7 @@ abstract final class NSStreamDelegate { /// stream:handleEvent: static final stream_handleEvent_ = objc.ObjCProtocolListenableMethod( + _protocol_NSStreamDelegate, _sel_stream_handleEvent_, objc.getProtocolMethodSignature( _protocol_NSStreamDelegate, diff --git a/pkgs/objective_c/lib/src/protocol_builder.dart b/pkgs/objective_c/lib/src/protocol_builder.dart index 65079e694..c70a12b8f 100644 --- a/pkgs/objective_c/lib/src/protocol_builder.dart +++ b/pkgs/objective_c/lib/src/protocol_builder.dart @@ -5,8 +5,10 @@ import 'dart:ffi'; import 'c_bindings_generated.dart' as c; -import 'internal.dart' show ObjCBlockBase; +import 'internal.dart' + show FailedToLoadProtocolMethodException, GetProtocolName, ObjCBlockBase; import 'objective_c_bindings_generated.dart' as objc; +import 'selector.dart'; /// Helper class for building Objective C objects that implement protocols. class ObjCProtocolBuilder { @@ -35,12 +37,14 @@ class ObjCProtocolBuilder { /// want to implement. The generated bindings will include a /// [ObjCProtocolMethod] for each method of the protocol. class ObjCProtocolMethod { + final Pointer _proto; final Pointer _sel; - final objc.NSMethodSignature _signature; + final objc.NSMethodSignature? _signature; final ObjCBlockBase Function(T) _createBlock; /// Only for use by ffigen bindings. - ObjCProtocolMethod(this._sel, this._signature, this._createBlock); + ObjCProtocolMethod( + this._proto, this._sel, this._signature, this._createBlock); /// Implement this method on the protocol [builder] using a Dart [function]. /// @@ -49,9 +53,17 @@ class ObjCProtocolMethod { /// the wrong thread will result in a crash. void implement(ObjCProtocolBuilder builder, T? function) { if (function != null) { - builder.implementMethod(_sel, _signature, _createBlock(function)); + builder.implementMethod(_sel, _sig, _createBlock(function)); } } + + bool get isAvailable => _signature != null; + + objc.NSMethodSignature get _sig { + final sig = _signature; + if (sig != null) return sig; + throw FailedToLoadProtocolMethodException(_proto.name, _sel.toDartString()); + } } /// A method in an ObjC protocol that can be implemented as a listener. @@ -65,8 +77,8 @@ class ObjCProtocolListenableMethod final ObjCBlockBase Function(T) _createListenerBlock; /// Only for use by ffigen bindings. - ObjCProtocolListenableMethod(super._sel, super._signature, super._createBlock, - this._createListenerBlock); + ObjCProtocolListenableMethod(super._proto, super._sel, super._signature, + super._createBlock, this._createListenerBlock); /// Implement this method on the protocol [builder] as a listener using a Dart /// [function]. @@ -77,7 +89,7 @@ class ObjCProtocolListenableMethod /// See NativeCallable.listener for more details. void implementAsListener(ObjCProtocolBuilder builder, T? function) { if (function != null) { - builder.implementMethod(_sel, _signature, _createListenerBlock(function)); + builder.implementMethod(_sel, _sig, _createListenerBlock(function)); } } } diff --git a/pkgs/objective_c/src/objective_c_runtime.h b/pkgs/objective_c/src/objective_c_runtime.h index e4a248560..3849ccd55 100644 --- a/pkgs/objective_c/src/objective_c_runtime.h +++ b/pkgs/objective_c/src/objective_c_runtime.h @@ -69,5 +69,6 @@ ObjCProtocol* objc_getProtocol(const char* name); ObjCMethodDesc protocol_getMethodDescription( ObjCProtocol* protocol, ObjCSelector* sel, bool isRequiredMethod, bool isInstanceMethod); +const char *protocol_getName(ObjCProtocol *proto); #endif // OBJECTIVE_C_SRC_OBJECTIVE_C_RUNTIME_H_