Skip to content

Commit

Permalink
[objective_c] Handle missing protocol methods more gracefully (#1705)
Browse files Browse the repository at this point in the history
  • Loading branch information
liamappelbe authored Nov 7, 2024
1 parent 80a005d commit 2ff68a9
Show file tree
Hide file tree
Showing 11 changed files with 124 additions and 20 deletions.
1 change: 1 addition & 0 deletions pkgs/ffigen/lib/src/code_generator/objc_protocol.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
20 changes: 16 additions & 4 deletions pkgs/ffigen/test/native_objc_test/protocol_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<Void> p, NSString s, double x) {
return 'DartProxy: $s: $x'.toNSString();
Expand All @@ -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<Void> p, SomeStruct s) {
return s.y - s.x;
Expand All @@ -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<Void> p, int a, int b, int c, int d) {
return a * b * c * d;
Expand Down Expand Up @@ -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<Void> p, NSString s, double x) => 'Hello'.toNSString());
proxyBuilder.implementMethod_withSignature_andBlock_(
Expand Down Expand Up @@ -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<FailedToLoadProtocolMethodException>()));
});
});
}
8 changes: 8 additions & 0 deletions pkgs/ffigen/test/native_objc_test/protocol_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 2 additions & 0 deletions pkgs/ffigen/test/native_objc_test/protocol_test.m
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#import <dispatch/dispatch.h>

#define DISABLE_METHOD 1

#include "protocol_test.h"

@implementation ProtocolConsumer : NSObject
Expand Down
5 changes: 5 additions & 0 deletions pkgs/objective_c/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions pkgs/objective_c/ffigen_c.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ functions:
- 'sel_registerName'
- 'sel_getName'
- 'protocol_getMethodDescription'
- 'protocol_getName'
- 'disposeObjCBlockWithClosure'
- 'isValidBlock'
- 'isValidObject'
Expand Down Expand Up @@ -51,6 +52,7 @@ functions:
'objc_copyClassList': 'copyClassList'
'objc_getProtocol': 'getProtocol'
'protocol_getMethodDescription': 'getMethodDescription'
'protocol_getName': 'getProtocolName'
globals:
include:
- '_NSConcrete.*Block'
Expand Down
6 changes: 6 additions & 0 deletions pkgs/objective_c/lib/src/c_bindings_generated.dart
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ external ffi.Pointer<ObjCProtocol> getProtocol(
ffi.Pointer<ffi.Char> name,
);

@ffi.Native<ffi.Pointer<ffi.Char> Function(ffi.Pointer<ObjCProtocol>)>(
symbol: "protocol_getName", isLeaf: true)
external ffi.Pointer<ffi.Char> getProtocolName(
ffi.Pointer<ObjCProtocol> proto,
);

@ffi.Native<ffi.Bool Function(ffi.Pointer<ObjCBlockImpl>)>(isLeaf: true)
external bool isValidBlock(
ffi.Pointer<ObjCBlockImpl> block,
Expand Down
72 changes: 63 additions & 9 deletions pkgs/objective_c/lib/src/internal.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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<c.ObjCProtocol> {
/// Returns the name of the protocol.
String get name => c.getProtocolName(this).cast<Utf8>().toDartString();
}

/// Only for use by ffigen bindings.
Expand All @@ -41,7 +94,7 @@ Pointer<c.ObjCObject> 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;
}
Expand All @@ -52,13 +105,13 @@ Pointer<c.ObjCProtocol> 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<c.ObjCProtocol> protocol,
Pointer<c.ObjCSelector> sel, {
required bool isRequired,
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6116,6 +6116,7 @@ abstract final class NSStreamDelegate {
/// stream:handleEvent:
static final stream_handleEvent_ =
objc.ObjCProtocolListenableMethod<void Function(NSStream, NSStreamEvent)>(
_protocol_NSStreamDelegate,
_sel_stream_handleEvent_,
objc.getProtocolMethodSignature(
_protocol_NSStreamDelegate,
Expand Down
26 changes: 19 additions & 7 deletions pkgs/objective_c/lib/src/protocol_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -35,12 +37,14 @@ class ObjCProtocolBuilder {
/// want to implement. The generated bindings will include a
/// [ObjCProtocolMethod] for each method of the protocol.
class ObjCProtocolMethod<T extends Function> {
final Pointer<c.ObjCProtocol> _proto;
final Pointer<c.ObjCSelector> _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].
///
Expand All @@ -49,9 +53,17 @@ class ObjCProtocolMethod<T extends Function> {
/// 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.
Expand All @@ -65,8 +77,8 @@ class ObjCProtocolListenableMethod<T extends Function>
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].
Expand All @@ -77,7 +89,7 @@ class ObjCProtocolListenableMethod<T extends Function>
/// 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));
}
}
}
1 change: 1 addition & 0 deletions pkgs/objective_c/src/objective_c_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_

0 comments on commit 2ff68a9

Please sign in to comment.