From 3464a50822b2b77e7be205f5aea505d8dfa10ecf Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 26 Sep 2019 15:57:52 -0700 Subject: [PATCH 1/5] Started asserting the FlutterEngine is running before communicating over channels. This changes a null pointer exception to an NSException that will provide some meaningful data to clients incorrectly using the engine in an add-to-app situations. --- .../platform/darwin/ios/framework/Source/FlutterEngine.mm | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index b2374fa0d147c..38b68a45b2c01 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -533,7 +533,9 @@ - (void)sendOnChannel:(NSString*)channel message:(NSData*)message { - (void)sendOnChannel:(NSString*)channel message:(NSData*)message binaryReply:(FlutterBinaryReply)callback { - NSAssert(channel, @"The channel must not be null"); + NSAssert(channel, @"The channel must not be nil"); + NSAssert(_shell && _shell->IsSetup(), + @"Sending a message before the FlutterEngine has been run."); fml::RefPtr response = (callback == nil) ? nullptr : fml::MakeRefCounted( @@ -552,7 +554,8 @@ - (void)sendOnChannel:(NSString*)channel - (void)setMessageHandlerOnChannel:(NSString*)channel binaryMessageHandler:(FlutterBinaryMessageHandler)handler { NSAssert(channel, @"The channel must not be null"); - FML_DCHECK(_shell && _shell->IsSetup()); + NSAssert(_shell && _shell->IsSetup(), + @"Setting a message handler before the FlutterEngine has been run."); self.iosPlatformView->GetPlatformMessageRouter().SetMessageHandler(channel.UTF8String, handler); } From 5ef69cb4d94e61853f2f2433acd3a3de00e31f43 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 26 Sep 2019 17:00:24 -0700 Subject: [PATCH 2/5] Added unit tests. --- .../ios/framework/Source/FlutterEngineTest.mm | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm index 47f718ae81c2c..a886da339bc42 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm @@ -24,4 +24,27 @@ - (void)testCreate { XCTAssertNotNil(engine); } +- (void)testSendMessageBeforeRun { + id project = OCMClassMock([FlutterDartProject class]); + FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"foobar" + project:project] autorelease]; + XCTAssertNotNil(engine); + XCTAssertThrows([engine.binaryMessenger + sendOnChannel:@"foo" + message:[@"bar" dataUsingEncoding:NSUTF8StringEncoding] + binaryReply:nil]); +} + +- (void)testSetMessageHandlerBeforeRun { + id project = OCMClassMock([FlutterDartProject class]); + FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"foobar" + project:project] autorelease]; + XCTAssertNotNil(engine); + XCTAssertThrows([engine.binaryMessenger + setMessageHandlerOnChannel:@"foo" + binaryMessageHandler:^(NSData* _Nullable message, FlutterBinaryReply _Nonnull reply){ + + }]); +} + @end From f8dd376dc127fc9b94e9c3b454d2e350b87a2178 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 26 Sep 2019 17:04:19 -0700 Subject: [PATCH 3/5] use NSParameterAssert --- shell/platform/darwin/ios/framework/Source/FlutterEngine.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 38b68a45b2c01..f0b908c34d668 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -533,7 +533,7 @@ - (void)sendOnChannel:(NSString*)channel message:(NSData*)message { - (void)sendOnChannel:(NSString*)channel message:(NSData*)message binaryReply:(FlutterBinaryReply)callback { - NSAssert(channel, @"The channel must not be nil"); + NSParameterAssert(channel); NSAssert(_shell && _shell->IsSetup(), @"Sending a message before the FlutterEngine has been run."); fml::RefPtr response = @@ -553,7 +553,7 @@ - (void)sendOnChannel:(NSString*)channel - (void)setMessageHandlerOnChannel:(NSString*)channel binaryMessageHandler:(FlutterBinaryMessageHandler)handler { - NSAssert(channel, @"The channel must not be null"); + NSParameterAssert(channel); NSAssert(_shell && _shell->IsSetup(), @"Setting a message handler before the FlutterEngine has been run."); self.iosPlatformView->GetPlatformMessageRouter().SetMessageHandler(channel.UTF8String, handler); From 060de29e39072b40575245f31fa0232cabfeeca2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 27 Sep 2019 13:07:08 -0700 Subject: [PATCH 4/5] Turned arc on for FlutterEngineTest. --- .../ios/framework/Source/FlutterEngineTest.mm | 21 +++++++++++++------ .../IosUnitTests.xcodeproj/project.pbxproj | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm index a886da339bc42..0ffe5a710decc 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm @@ -6,6 +6,15 @@ #import #import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterEngine.h" +#ifndef __has_feature +#define __has_feature(x) 0 /* for non-clang compilers */ +#endif + +#if !__has_feature(objc_arc) +#error ARC must be enabled! +#endif + + @interface FlutteEngineTest : XCTestCase @end @@ -19,15 +28,15 @@ - (void)tearDown { - (void)testCreate { id project = OCMClassMock([FlutterDartProject class]); - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"foobar" - project:project] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" + project:project]; XCTAssertNotNil(engine); } - (void)testSendMessageBeforeRun { id project = OCMClassMock([FlutterDartProject class]); - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"foobar" - project:project] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" + project:project]; XCTAssertNotNil(engine); XCTAssertThrows([engine.binaryMessenger sendOnChannel:@"foo" @@ -37,8 +46,8 @@ - (void)testSendMessageBeforeRun { - (void)testSetMessageHandlerBeforeRun { id project = OCMClassMock([FlutterDartProject class]); - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"foobar" - project:project] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" + project:project]; XCTAssertNotNil(engine); XCTAssertThrows([engine.binaryMessenger setMessageHandlerOnChannel:@"foo" diff --git a/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj b/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj index c323e77a6a032..82f36199ac95a 100644 --- a/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj +++ b/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj @@ -17,7 +17,7 @@ 0D6AB6BE22BB05E200EEE540 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 0D6AB6BD22BB05E200EEE540 /* Assets.xcassets */; }; 0D6AB6C122BB05E200EEE540 /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 0D6AB6BF22BB05E200EEE540 /* LaunchScreen.storyboard */; }; 0D6AB6C422BB05E200EEE540 /* main.m in Sources */ = {isa = PBXBuildFile; fileRef = 0D6AB6C322BB05E200EEE540 /* main.m */; }; - 0D6AB6EB22BB40E700EEE540 /* FlutterEngineTest.mm in Sources */ = {isa = PBXBuildFile; fileRef = 0D6AB6E722BB40CF00EEE540 /* FlutterEngineTest.mm */; }; + 0D6AB6EB22BB40E700EEE540 /* FlutterEngineTest.mm in Sources */ = {isa = PBXBuildFile; fileRef = 0D6AB6E722BB40CF00EEE540 /* FlutterEngineTest.mm */; settings = {COMPILER_FLAGS = "-fobjc-arc"; }; }; 0D6AB72C22BC339F00EEE540 /* libOCMock.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 0D6AB72522BC336100EEE540 /* libOCMock.a */; }; 0D6AB73F22BD8F0200EEE540 /* FlutterEngineConfig.xcconfig in Resources */ = {isa = PBXBuildFile; fileRef = 0D6AB73E22BD8F0200EEE540 /* FlutterEngineConfig.xcconfig */; }; /* End PBXBuildFile section */ From df437444aa9c7eb2121519e8dfdb8a3dd3d2fc2f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 27 Sep 2019 13:15:15 -0700 Subject: [PATCH 5/5] format code --- .../darwin/ios/framework/Source/FlutterEngineTest.mm | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm index 0ffe5a710decc..cf96c2e716d4c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm @@ -14,7 +14,6 @@ #error ARC must be enabled! #endif - @interface FlutteEngineTest : XCTestCase @end @@ -28,15 +27,13 @@ - (void)tearDown { - (void)testCreate { id project = OCMClassMock([FlutterDartProject class]); - FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" - project:project]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:project]; XCTAssertNotNil(engine); } - (void)testSendMessageBeforeRun { id project = OCMClassMock([FlutterDartProject class]); - FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" - project:project]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:project]; XCTAssertNotNil(engine); XCTAssertThrows([engine.binaryMessenger sendOnChannel:@"foo" @@ -46,8 +43,7 @@ - (void)testSendMessageBeforeRun { - (void)testSetMessageHandlerBeforeRun { id project = OCMClassMock([FlutterDartProject class]); - FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" - project:project]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:project]; XCTAssertNotNil(engine); XCTAssertThrows([engine.binaryMessenger setMessageHandlerOnChannel:@"foo"