Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Started asserting the FlutterEngine is running before communicating over channels. #12469

Merged
gaaclarke merged 5 commits into from
Sep 27, 2019
Merged

Started asserting the FlutterEngine is running before communicating over channels. #12469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3464a50
Started asserting the FlutterEngine is running before communicating
gaaclarke Sep 26, 2019
5ef69cb
Added unit tests.
gaaclarke Sep 27, 2019
f8dd376
use NSParameterAssert
gaaclarke Sep 27, 2019
060de29
Turned arc on for FlutterEngineTest.
gaaclarke Sep 27, 2019
df43744
format code
gaaclarke Sep 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
NSParameterAssert(channel);
NSAssert(_shell && _shell->IsSetup(),
@"Sending a message before the FlutterEngine has been run.");
Copy link
Member

@jmagman jmagman Sep 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we tell them how to run it?

Copy link
Member Author

@gaaclarke gaaclarke Sep 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was careful to use the word "run" to match the verb in our run methods. It's a bit complicated, should I say something like "See run* methods"?

Copy link
Member

@jmagman jmagman Sep 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh it's okay, it would make sense to list it if there was maybe only one way to run it. What you have is fine.

fml::RefPtr<flutter::PlatformMessageResponseDarwin> response =
(callback == nil) ? nullptr
: fml::MakeRefCounted<flutter::PlatformMessageResponseDarwin>(
Expand All @@ -551,8 +553,9 @@ - (void)sendOnChannel:(NSString*)channel

- (void)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:(FlutterBinaryMessageHandler)handler {
NSAssert(channel, @"The channel must not be null");
FML_DCHECK(_shell && _shell->IsSetup());
NSParameterAssert(channel);
NSAssert(_shell && _shell->IsSetup(),
@"Setting a message handler before the FlutterEngine has been run.");
self.iosPlatformView->GetPlatformMessageRouter().SetMessageHandler(channel.UTF8String, handler);
}

Expand Down
32 changes: 30 additions & 2 deletions shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@
#import <XCTest/XCTest.h>
#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!
Copy link
Member

@jmagman jmagman Sep 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity what non-clang compilers are compiling these test files?

Copy link
Member Author

@gaaclarke gaaclarke Sep 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None today that I know of. If we accidentally used GCC it should probably be an error. I'm just being cautious. Ideally this would be a macro too, I just haven't gotten around to make it one. It's not straightforward to get a macro of macros, I'd have to spend some time looking things up and fiddling with it.

#endif

@interface FlutteEngineTest : XCTestCase
@end

Expand All @@ -19,9 +27,29 @@ - (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];
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];
XCTAssertNotNil(engine);
XCTAssertThrows([engine.binaryMessenger
setMessageHandlerOnChannel:@"foo"
binaryMessageHandler:^(NSData* _Nullable message, FlutterBinaryReply _Nonnull reply){

}]);
}

@end
2 changes: 1 addition & 1 deletion testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down