Skip to content

Commit

Permalink
iOS: Migrate platform_view_ios to ARC
Browse files Browse the repository at this point in the history
Migrates PlatformViewIOS from manual reference counting to ARC.
Eliminates use of scoped_nsobject, scoped_nsprotocol, and WeakNSObject.

Since this is the last non-ARC file in `flutter_framework_source`, this
also eliminates the `flutter_framework_source` target, then also renames
the `flutter_framework_source_arc` target to `flutter_framework_source`
since... it's ALL ARC.

No semantic changes, therefore no changes to tests.

Issue: flutter/flutter#137801
  • Loading branch information
cbracken committed Oct 5, 2024
1 parent b08328f commit f0c9427
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 142 deletions.
90 changes: 23 additions & 67 deletions shell/platform/darwin/ios/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ _flutter_framework_headers = [

_flutter_framework_headers_copy_dir = "$_flutter_framework_dir/Headers"

source_set("flutter_framework_source_arc") {
source_set("flutter_framework_source") {
visibility = [ ":*" ]
cflags_objc = flutter_cflags_objc_arc
cflags_objcc = flutter_cflags_objcc_arc
Expand All @@ -48,15 +48,10 @@ source_set("flutter_framework_source_arc") {
if (darwin_extension_safe) {
defines += [ "APPLICATION_EXTENSION_API_ONLY=1" ]
}
allow_circular_includes_from = [ ":flutter_framework_source" ]
deps = [
":flutter_framework_source",
"//flutter/fml",
"//flutter/shell/platform/common:common_cpp_input",
"//flutter/shell/platform/darwin/common:framework_common",
"//flutter/third_party/icu",
public_configs = [
":ios_gpu_configuration_config",
"//flutter:config",
]
public_configs = [ "//flutter:config" ]

sources = [
"framework/Source/FlutterAppDelegate.mm",
Expand Down Expand Up @@ -156,81 +151,46 @@ source_set("flutter_framework_source_arc") {
"ios_surface_software.mm",
"platform_message_handler_ios.h",
"platform_message_handler_ios.mm",
"platform_view_ios.h",
"platform_view_ios.mm",
"rendering_api_selection.h",
"rendering_api_selection.mm",
]
sources += _flutter_framework_headers

frameworks = [
"UIKit.framework",
"AudioToolbox.framework",
"CoreMedia.framework",
"CoreVideo.framework",
"IOSurface.framework",
"QuartzCore.framework",
"UIKit.framework",
]
if (flutter_runtime_mode == "profile" || flutter_runtime_mode == "debug") {
# This is required by the profiler_metrics_ios.mm to get GPU statistics.
# Usage in release builds will cause rejection from the App Store.
frameworks += [ "IOKit.framework" ]
}

deps += [
deps = [
":ios_gpu_configuration",
"//flutter/common:common",
"//flutter/common/graphics",
"//flutter/fml",
"//flutter/lib/ui",
"//flutter/runtime",
"//flutter/shell/common",
"//flutter/shell/platform/common:common_cpp_input",
"//flutter/shell/platform/darwin/common",
"//flutter/shell/platform/darwin/common:framework_common",
"//flutter/shell/platform/darwin/graphics",
"//flutter/shell/platform/embedder:embedder_as_internal_library",
"//flutter/shell/profiling:profiling",
"//flutter/third_party/icu",
"//flutter/third_party/spring_animation",
]
}

source_set("flutter_framework_source") {
visibility = [ ":*" ]
cflags_objc = flutter_cflags_objc
cflags_objcc = flutter_cflags_objcc

deps = []

sources = [
# iOS embedder is migrating to ARC.
# New files are highly encouraged to be in ARC.
# To add new files in ARC, add them to the `flutter_framework_source_arc` target.
"platform_view_ios.h",
"platform_view_ios.mm",
]

sources += _flutter_framework_headers

defines = [ "FLUTTER_FRAMEWORK=1" ]
if (darwin_extension_safe) {
defines += [ "APPLICATION_EXTENSION_API_ONLY=1" ]
}

deps += [
"//flutter/fml",
"//flutter/runtime",
"//flutter/shell/common",
"//flutter/shell/platform/darwin/common",
"//flutter/shell/platform/darwin/common:framework_common",
"//flutter/shell/platform/embedder:embedder_as_internal_library",
"//flutter/shell/profiling:profiling",
]

public_configs = [
":ios_gpu_configuration_config",
"//flutter:config",
]

frameworks = [
"AudioToolbox.framework",
"CoreMedia.framework",
"CoreVideo.framework",
"QuartzCore.framework",
"UIKit.framework",
]
if (flutter_runtime_mode == "profile" || flutter_runtime_mode == "debug") {
# This is required by the profiler_metrics_ios.mm to get GPU statistics.
# Usage in release builds will cause rejection from the App Store.
frameworks += [ "IOKit.framework" ]
}
}

platform_frameworks_path =
rebase_path("$ios_sdk_path/../../Library/Frameworks/")

Expand Down Expand Up @@ -295,7 +255,6 @@ shared_library("ios_test_flutter") {
deps = [
":flutter_framework",
":flutter_framework_source",
":flutter_framework_source_arc",
":ios_gpu_configuration",
"//flutter/common:common",
"//flutter/lib/ui:ui",
Expand Down Expand Up @@ -332,10 +291,7 @@ shared_library("create_flutter_framework_dylib") {

public = _flutter_framework_headers

deps = [
":flutter_framework_source",
":flutter_framework_source_arc",
]
deps = [ ":flutter_framework_source" ]

public_configs = [ "//flutter:config" ]
}
Expand Down
17 changes: 2 additions & 15 deletions shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "flutter/common/constants.h"
#include "flutter/fml/message_loop.h"
#include "flutter/fml/platform/darwin/platform_version.h"
#include "flutter/fml/platform/darwin/weak_nsobject.h"
#include "flutter/fml/trace_event.h"
#include "flutter/runtime/ptrace_check.h"
#include "flutter/shell/common/engine.h"
Expand Down Expand Up @@ -152,10 +151,6 @@ @implementation FlutterEngine {
std::shared_ptr<flutter::ThreadHost> _threadHost;
std::unique_ptr<flutter::Shell> _shell;

// TODO(cbracken): https://github.com/flutter/flutter/issues/155943
// Migrate to @property(nonatomic, weak).
fml::WeakNSObject<FlutterViewController> _viewController;

std::shared_ptr<flutter::PlatformViewsController> _platformViewsController;
flutter::IOSRenderingAPI _renderingApi;
std::shared_ptr<flutter::ProfilerMetricsIOS> _profiler_metrics;
Expand Down Expand Up @@ -407,8 +402,7 @@ - (void)ensureSemanticsEnabled {

- (void)setViewController:(FlutterViewController*)viewController {
FML_DCHECK(self.iosPlatformView);
_viewController = viewController ? [viewController getWeakNSObject]
: fml::WeakNSObject<FlutterViewController>();
_viewController = viewController;
self.iosPlatformView->SetOwnerViewController(_viewController);
[self maybeSetupPlatformViewChannels];
[self updateDisplays];
Expand Down Expand Up @@ -455,7 +449,7 @@ - (void)notifyViewControllerDeallocated {
}
}
[self.textInputPlugin resetViewResponder];
_viewController.reset();
_viewController = nil;
}

- (void)destroyContext {
Expand All @@ -467,13 +461,6 @@ - (void)destroyContext {
_platformViewsController.reset();
}

- (FlutterViewController*)viewController {
if (!_viewController) {
return nil;
}
return _viewController.get();
}

- (std::shared_ptr<flutter::PlatformViewsController>&)platformViewsController {
return _platformViewsController;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,6 @@ - (void)deregisterNotifications;
@end

@implementation FlutterViewController {
// TODO(cbracken): https://github.com/flutter/flutter/issues/137801
// Eliminate once we can use weak pointers in platform_view_ios.h.
std::unique_ptr<fml::WeakNSObjectFactory<FlutterViewController>> _weakFactory;

flutter::ViewportMetrics _viewportMetrics;
MouseState _mouseState;
}
Expand Down Expand Up @@ -184,7 +180,6 @@ - (instancetype)initWithEngine:(FlutterEngine*)engine
_flutterView = [[FlutterView alloc] initWithDelegate:_engine
opaque:self.isViewOpaque
enableWideGamut:engine.project.isWideGamutEnabled];
_weakFactory = std::make_unique<fml::WeakNSObjectFactory<FlutterViewController>>(self);
_ongoingTouches = [[NSMutableSet alloc] init];

[self performCommonViewControllerInitialization];
Expand Down Expand Up @@ -245,7 +240,6 @@ - (void)sharedSetupWithProject:(nullable FlutterDartProject*)project
project = [[FlutterDartProject alloc] init];
}
FlutterView.forceSoftwareRendering = project.settings.enable_software_rendering;
_weakFactory = std::make_unique<fml::WeakNSObjectFactory<FlutterViewController>>(self);
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"io.flutter"
project:project
allowHeadlessExecution:self.engineAllowHeadlessExecution
Expand Down Expand Up @@ -292,10 +286,6 @@ - (void)performCommonViewControllerInitialization {
[self setUpNotificationCenterObservers];
}

- (fml::WeakNSObject<FlutterViewController>)getWeakNSObject {
return _weakFactory->GetWeakNSObject();
}

- (void)setUpNotificationCenterObservers {
NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
[center addObserver:self
Expand Down Expand Up @@ -956,10 +946,6 @@ - (void)deregisterNotifications {
}

- (void)dealloc {
// It will be destroyed and invalidate its weak pointers
// before any other members are destroyed.
_weakFactory.reset();

[self removeInternalPlugins];
[self deregisterNotifications];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#ifndef FLUTTER_SHELL_PLATFORM_DARWIN_IOS_FRAMEWORK_SOURCE_FLUTTERVIEWCONTROLLER_INTERNAL_H_
#define FLUTTER_SHELL_PLATFORM_DARWIN_IOS_FRAMEWORK_SOURCE_FLUTTERVIEWCONTROLLER_INTERNAL_H_

#include "flutter/fml/platform/darwin/weak_nsobject.h"
#include "flutter/fml/time/time_point.h"

#import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h"
Expand Down Expand Up @@ -57,7 +56,6 @@ typedef void (^FlutterKeyboardAnimationCallback)(fml::TimePoint);
*/
@property(nonatomic, assign, readwrite) BOOL prefersStatusBarHidden;

- (fml::WeakNSObject<FlutterViewController>)getWeakNSObject;
- (std::shared_ptr<flutter::PlatformViewsController>&)platformViewsController;
- (FlutterRestorationPlugin*)restorationPlugin;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification,
weak_factory_(this) {
accessibility_channel_.reset([[FlutterBasicMessageChannel alloc]
initWithName:@"flutter/accessibility"
binaryMessenger:platform_view->GetOwnerViewController().get().engine.binaryMessenger
binaryMessenger:platform_view->GetOwnerViewController().engine.binaryMessenger
codec:[FlutterStandardMessageCodec sharedInstance]]);
[accessibility_channel_.get() setMessageHandler:^(id message, FlutterReply reply) {
HandleEvent((NSDictionary*)message);
Expand All @@ -69,7 +69,7 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification,
}

UIView<UITextInput>* AccessibilityBridge::textInputView() {
return [[platform_view_->GetOwnerViewController().get().engine textInputPlugin] textInputView];
return [[platform_view_->GetOwnerViewController().engine textInputPlugin] textInputView];
}

void AccessibilityBridge::AccessibilityObjectDidBecomeFocused(int32_t id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1402,9 +1402,7 @@ - (void)testAccessibilityObjectDidBecomeFocused {
/*is_gpu_disabled_sync_switch=*/std::make_shared<fml::SyncSwitch>());
fml::AutoResetWaitableEvent latch;
thread_task_runner->PostTask([&] {
auto weakFactory =
std::make_unique<fml::WeakNSObjectFactory<FlutterViewController>>(flutterViewController);
platform_view->SetOwnerViewController(weakFactory->GetWeakNSObject());
platform_view->SetOwnerViewController(flutterViewController);
auto bridge =
std::make_unique<flutter::AccessibilityBridge>(/*view=*/nil,
/*platform_view=*/platform_view.get(),
Expand Down Expand Up @@ -2091,9 +2089,7 @@ - (void)testAccessibilityMessageAfterDeletion {
/*is_gpu_disabled_sync_switch=*/std::make_shared<fml::SyncSwitch>());
fml::AutoResetWaitableEvent latch;
thread_task_runner->PostTask([&] {
auto weakFactory =
std::make_unique<fml::WeakNSObjectFactory<FlutterViewController>>(flutterViewController);
platform_view->SetOwnerViewController(weakFactory->GetWeakNSObject());
platform_view->SetOwnerViewController(flutterViewController);
auto bridge =
std::make_unique<flutter::AccessibilityBridge>(/*view=*/nil,
/*platform_view=*/platform_view.get(),
Expand Down Expand Up @@ -2184,9 +2180,7 @@ - (void)testPlatformViewDestructorDoesNotCallSemanticsAPIs {

OCMStub([mockFlutterViewController platformViewsController])
.andReturn(flutterPlatformViewsController.get());
auto weakFactory = std::make_unique<fml::WeakNSObjectFactory<FlutterViewController>>(
mockFlutterViewController);
platform_view->SetOwnerViewController(weakFactory->GetWeakNSObject());
platform_view->SetOwnerViewController(mockFlutterViewController);

platform_view->SetSemanticsEnabled(true);
XCTAssertNotEqual(test_delegate.set_semantics_enabled_calls, 0);
Expand Down
9 changes: 3 additions & 6 deletions shell/platform/darwin/ios/platform_view_ios.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@

#include "flutter/fml/closure.h"
#include "flutter/fml/macros.h"
#include "flutter/fml/platform/darwin/scoped_nsobject.h"
#include "flutter/fml/platform/darwin/weak_nsobject.h"
#include "flutter/shell/common/platform_view.h"
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterTexture.h"
#import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h"
Expand Down Expand Up @@ -59,14 +57,14 @@ class PlatformViewIOS final : public PlatformView {
* Returns the `FlutterViewController` currently attached to the `FlutterEngine` owning
* this PlatformViewIOS.
*/
fml::WeakNSObject<FlutterViewController> GetOwnerViewController() const;
FlutterViewController* GetOwnerViewController() const;

/**
* Updates the `FlutterViewController` currently attached to the `FlutterEngine` owning
* this PlatformViewIOS. This should be updated when the `FlutterEngine`
* is given a new `FlutterViewController`.
*/
void SetOwnerViewController(const fml::WeakNSObject<FlutterViewController>& owner_controller);
void SetOwnerViewController(__weak FlutterViewController* owner_controller);

/**
* Called one time per `FlutterViewController` when the `FlutterViewController`'s
Expand Down Expand Up @@ -133,15 +131,14 @@ class PlatformViewIOS final : public PlatformView {
std::function<void(bool)> set_semantics_enabled_;
};

fml::WeakNSObject<FlutterViewController> owner_controller_;
__weak FlutterViewController* owner_controller_;
// Since the `ios_surface_` is created on the platform thread but
// used on the raster thread we need to protect it with a mutex.
std::mutex ios_surface_mutex_;
std::unique_ptr<IOSSurface> ios_surface_;
std::shared_ptr<IOSContext> ios_context_;
const std::shared_ptr<PlatformViewsController>& platform_views_controller_;
AccessibilityBridgeManager accessibility_bridge_;
fml::scoped_nsprotocol<FlutterTextInputPlugin*> text_input_plugin_;
ScopedObserver dealloc_view_controller_observer_;
std::vector<std::string> platform_resolved_locale_;
std::shared_ptr<PlatformMessageHandlerIos> platform_message_handler_;
Expand Down
Loading

0 comments on commit f0c9427

Please sign in to comment.