Skip to content

Commit

Permalink
Made FlutterTaskQueue visible for Swift. (flutter#47348)
Browse files Browse the repository at this point in the history
fixes flutter/flutter#118832

This makes the `FlutterTaskQueue` protocol visible, but keeps the
`dispatch:` selector private. Under the covers there are runtime checks
to make sure `dispatch:` is available since we can't have compile time
checks anymore.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [ ] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
  • Loading branch information
gaaclarke authored Oct 27, 2023
1 parent 0bba9ee commit 1e66c0a
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 22 deletions.
2 changes: 2 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -3614,6 +3614,7 @@ ORIGIN: ../../../flutter/shell/platform/darwin/graphics/FlutterDarwinContextMeta
ORIGIN: ../../../flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalSkia.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/flutter_task_queue_dispatch.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Headers/Flutter.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Headers/FlutterAppDelegate.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Headers/FlutterCallbackCache.h + ../../../flutter/LICENSE
Expand Down Expand Up @@ -6412,6 +6413,7 @@ FILE: ../../../flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalS
FILE: ../../../flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalSkia.mm
FILE: ../../../flutter/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.h
FILE: ../../../flutter/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm
FILE: ../../../flutter/shell/platform/darwin/ios/flutter_task_queue_dispatch.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Headers/Flutter.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Headers/FlutterAppDelegate.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Headers/FlutterCallbackCache.h
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ typedef void (^FlutterBinaryMessageHandler)(NSData* _Nullable message, FlutterBi

typedef int64_t FlutterBinaryMessengerConnection;

@protocol FlutterTaskQueue;
@protocol FlutterTaskQueue <NSObject>
@end

/**
* A facility for communicating with the Flutter side using asynchronous message
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
#import "FlutterBinaryMessenger.h"
#import "FlutterCodecs.h"

@protocol FlutterTaskQueue;

NS_ASSUME_NONNULL_BEGIN
/**
* A message reply callback.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"
#import "flutter/shell/platform/darwin/common/framework/Source/FlutterTestUtils.h"
#import "flutter/shell/platform/darwin/ios/flutter_task_queue_dispatch.h"
#import "flutter/testing/testing.h"
#include "gtest/gtest.h"

FLUTTER_ASSERT_ARC

@protocol FlutterTaskQueue <NSObject>
@end

@interface FlutterBinaryMessengerRelayTest : NSObject
@end

Expand Down Expand Up @@ -56,7 +54,7 @@ - (void)testSetMessageHandlerWithTaskQueue {
FlutterBinaryMessengerRelay* relay =
[[FlutterBinaryMessengerRelay alloc] initWithParent:messenger];
NSString* channel = @"foobar";
NSObject<FlutterTaskQueue>* taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueue));
NSObject<FlutterTaskQueue>* taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueueDispatch));
FlutterBinaryMessageHandler handler = ^(NSData* _Nullable, FlutterBinaryReply _Nonnull) {
};
[relay setMessageHandlerOnChannel:channel binaryMessageHandler:handler taskQueue:taskQueue];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@
// found in the LICENSE file.

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h"
#import "flutter/shell/platform/darwin/ios/flutter_task_queue_dispatch.h"

#import <OCMock/OCMock.h>
#import <XCTest/XCTest.h>

FLUTTER_ASSERT_ARC

@protocol FlutterTaskQueue <NSObject>
@end

@interface MockBinaryMessenger : NSObject <FlutterBinaryMessenger>
@property(nonatomic, copy) NSString* channel;
@property(nonatomic, strong) NSData* message;
Expand Down Expand Up @@ -243,7 +241,7 @@ - (void)testBasicMessageChannelTaskQueue {
FlutterBinaryMessengerConnection connection = 123;
id binaryMessenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
id codec = OCMProtocolMock(@protocol(FlutterMethodCodec));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueue));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueueDispatch));
FlutterBasicMessageChannel* channel =
[[FlutterBasicMessageChannel alloc] initWithName:channelName
binaryMessenger:binaryMessenger
Expand Down Expand Up @@ -271,7 +269,7 @@ - (void)testBasicMessageChannelInvokeHandlerAfterChannelReleased {
FlutterBinaryMessengerConnection connection = 123;
id binaryMessenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
id codec = OCMProtocolMock(@protocol(FlutterMessageCodec));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueue));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueueDispatch));
FlutterBasicMessageChannel* channel =
[[FlutterBasicMessageChannel alloc] initWithName:channelName
binaryMessenger:binaryMessenger
Expand Down Expand Up @@ -308,7 +306,7 @@ - (void)testMethodChannelInvokeHandlerAfterChannelReleased {
@autoreleasepool {
id binaryMessenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
id codec = OCMProtocolMock(@protocol(FlutterMethodCodec));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueue));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueueDispatch));
FlutterMethodChannel* channel = [[FlutterMethodChannel alloc] initWithName:channelName
binaryMessenger:binaryMessenger
codec:codec
Expand Down Expand Up @@ -340,7 +338,7 @@ - (void)testMethodChannelTaskQueue {
FlutterBinaryMessengerConnection connection = 123;
id binaryMessenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
id codec = OCMProtocolMock(@protocol(FlutterMethodCodec));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueue));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueueDispatch));
FlutterMethodChannel* channel = [[FlutterMethodChannel alloc] initWithName:channelName
binaryMessenger:binaryMessenger
codec:codec
Expand All @@ -365,7 +363,7 @@ - (void)testEventChannelTaskQueue {
FlutterBinaryMessengerConnection connection = 123;
id binaryMessenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
id codec = OCMProtocolMock(@protocol(FlutterMethodCodec));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueue));
id taskQueue = OCMProtocolMock(@protocol(FlutterTaskQueueDispatch));
id handler = OCMProtocolMock(@protocol(FlutterStreamHandler));
FlutterEventChannel* channel = [[FlutterEventChannel alloc] initWithName:channelName
binaryMessenger:binaryMessenger
Expand Down
20 changes: 20 additions & 0 deletions shell/platform/darwin/ios/flutter_task_queue_dispatch.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_SHELL_PLATFORM_DARWIN_IOS_FLUTTER_TASK_QUEUE_DISPATCH_H_
#define FLUTTER_SHELL_PLATFORM_DARWIN_IOS_FLUTTER_TASK_QUEUE_DISPATCH_H_

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h"

/// The private implementation of `FlutterTaskQueue` that has method
/// declarations.
///
/// `FlutterTaskQueue` doesn't have any methods publicly since it is supposed to
/// be an opaque data structure. For Swift integration though `FlutterTaskQueue`
/// is visible publicly with no methods.
@protocol FlutterTaskQueueDispatch <FlutterTaskQueue>
- (void)dispatch:(dispatch_block_t)block;
@end

#endif
7 changes: 2 additions & 5 deletions shell/platform/darwin/ios/platform_message_handler_ios.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
#include "flutter/fml/platform/darwin/scoped_nsobject.h"
#include "flutter/shell/common/platform_message_handler.h"
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h"

@protocol FlutterTaskQueue
- (void)dispatch:(dispatch_block_t)block;
@end
#import "flutter/shell/platform/darwin/ios/flutter_task_queue_dispatch.h"

namespace flutter {

Expand All @@ -39,7 +36,7 @@ class PlatformMessageHandlerIos : public PlatformMessageHandler {
NSObject<FlutterTaskQueue>* task_queue);

struct HandlerInfo {
fml::scoped_nsprotocol<NSObject<FlutterTaskQueue>*> task_queue;
fml::scoped_nsprotocol<NSObject<FlutterTaskQueueDispatch>*> task_queue;
fml::ScopedBlock<FlutterBinaryMessageHandler> handler;
};

Expand Down
9 changes: 7 additions & 2 deletions shell/platform/darwin/ios/platform_message_handler_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

static uint64_t platform_message_counter = 1;

@interface FLTSerialTaskQueue : NSObject <FlutterTaskQueue>
@interface FLTSerialTaskQueue : NSObject <FlutterTaskQueueDispatch>
@property(nonatomic, strong) dispatch_queue_t queue;
@end

Expand Down Expand Up @@ -118,12 +118,17 @@ - (void)dispatch:(dispatch_block_t)block {
FlutterBinaryMessageHandler handler,
NSObject<FlutterTaskQueue>* task_queue) {
FML_CHECK(platform_task_runner_->RunsTasksOnCurrentThread());
// Use `respondsToSelector` instead of `conformsToProtocol` to accomodate
// injecting your own `FlutterTaskQueue`. This is not a supported usage but
// not one worth breaking.
FML_CHECK(!task_queue || [task_queue respondsToSelector:@selector(dispatch:)]);
/// TODO(gaaclarke): This should be migrated to a lockfree datastructure.
std::lock_guard lock(message_handlers_mutex_);
message_handlers_.erase(channel);
if (handler) {
message_handlers_[channel] = {
.task_queue = fml::scoped_nsprotocol([task_queue retain]),
.task_queue = fml::scoped_nsprotocol(
[static_cast<NSObject<FlutterTaskQueueDispatch>*>(task_queue) retain]),
.handler =
fml::ScopedBlock<FlutterBinaryMessageHandler>{handler, fml::OwnershipPolicy::kRetain},
};
Expand Down

0 comments on commit 1e66c0a

Please sign in to comment.