From fbb8302bc0f512415de8dbd54123ecb84912743b Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 10 Nov 2023 20:14:33 -0800 Subject: [PATCH 1/8] weak_nsobject remove unnecessary interface --- ci/licenses_golden/excluded_files | 1 + ci/licenses_golden/licenses_flutter | 4 + fml/BUILD.gn | 7 +- fml/platform/darwin/weak_nsobject.h | 287 ++++++++++++++++++ fml/platform/darwin/weak_nsobject.mm | 60 ++++ .../darwin/weak_nsobject_unittests.mm | 92 ++++++ .../ios/framework/Source/FlutterEngine.mm | 15 +- .../framework/Source/FlutterPlatformPlugin.h | 4 +- .../framework/Source/FlutterPlatformPlugin.mm | 4 +- .../Source/FlutterPlatformPluginTest.mm | 49 +-- 10 files changed, 487 insertions(+), 36 deletions(-) create mode 100644 fml/platform/darwin/weak_nsobject.h create mode 100644 fml/platform/darwin/weak_nsobject.mm create mode 100644 fml/platform/darwin/weak_nsobject_unittests.mm diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index e518702b32318..fc9d63570de08 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -104,6 +104,7 @@ ../../../flutter/fml/platform/darwin/scoped_nsobject_arc_unittests.mm ../../../flutter/fml/platform/darwin/scoped_nsobject_unittests.mm ../../../flutter/fml/platform/darwin/string_range_sanitization_unittests.mm +../../../flutter/fml/platform/darwin/weak_nsobject_unittests.mm ../../../flutter/fml/platform/win/file_win_unittests.cc ../../../flutter/fml/platform/win/wstring_conversion_unittests.cc ../../../flutter/fml/raster_thread_merger_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 27569036ed116..a5e0ae304931b 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2799,6 +2799,8 @@ ORIGIN: ../../../flutter/fml/platform/darwin/scoped_policy.h + ../../../flutter/ ORIGIN: ../../../flutter/fml/platform/darwin/scoped_typeref.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/platform/darwin/string_range_sanitization.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/platform/darwin/string_range_sanitization.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/fml/platform/darwin/weak_nsobject.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/fml/platform/darwin/weak_nsobject.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/platform/fuchsia/message_loop_fuchsia.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/platform/fuchsia/message_loop_fuchsia.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/platform/fuchsia/paths_fuchsia.cc + ../../../flutter/LICENSE @@ -5563,6 +5565,8 @@ FILE: ../../../flutter/fml/platform/darwin/scoped_policy.h FILE: ../../../flutter/fml/platform/darwin/scoped_typeref.h FILE: ../../../flutter/fml/platform/darwin/string_range_sanitization.h FILE: ../../../flutter/fml/platform/darwin/string_range_sanitization.mm +FILE: ../../../flutter/fml/platform/darwin/weak_nsobject.h +FILE: ../../../flutter/fml/platform/darwin/weak_nsobject.mm FILE: ../../../flutter/fml/platform/fuchsia/message_loop_fuchsia.cc FILE: ../../../flutter/fml/platform/fuchsia/message_loop_fuchsia.h FILE: ../../../flutter/fml/platform/fuchsia/paths_fuchsia.cc diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 984b79ae82720..d07a421104391 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -169,6 +169,8 @@ source_set("fml") { "platform/darwin/scoped_typeref.h", "platform/darwin/string_range_sanitization.h", "platform/darwin/string_range_sanitization.mm", + "platform/darwin/weak_nsobject.h", + "platform/darwin/weak_nsobject.mm", ] frameworks = [ "Foundation.framework" ] @@ -369,7 +371,10 @@ if (enable_unittests) { } if (is_mac || is_ios) { - sources += [ "platform/darwin/scoped_nsobject_unittests.mm" ] + sources += [ + "platform/darwin/scoped_nsobject_unittests.mm", + "platform/darwin/weak_nsobject_unittests.mm", + ] } if (is_win) { diff --git a/fml/platform/darwin/weak_nsobject.h b/fml/platform/darwin/weak_nsobject.h new file mode 100644 index 0000000000000..89c12e4a7ac92 --- /dev/null +++ b/fml/platform/darwin/weak_nsobject.h @@ -0,0 +1,287 @@ +// 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_FML_PLATFORM_DARWIN_WEAK_NSOBJECT_H_ +#define FLUTTER_FML_PLATFORM_DARWIN_WEAK_NSOBJECT_H_ + +#import +#import + +#include +#include "flutter/fml/compiler_specific.h" +#include "flutter/fml/logging.h" +#include "flutter/fml/memory/ref_counted.h" +#include "flutter/fml/memory/ref_ptr.h" +#include "flutter/fml/memory/thread_checker.h" + +namespace debug { +struct DebugThreadChecker { + FML_DECLARE_THREAD_CHECKER(checker); +}; +} // namespace debug + +// WeakNSObject<> is patterned after scoped_nsobject<>, but instead of +// maintaining ownership of an NSObject subclass object, it will nil itself out +// when the object is deallocated. +// +// WeakNSProtocol<> has the same behavior as WeakNSObject, but can be used +// with protocols. +// +// Example usage (fml::WeakNSObject): +// WeakNSObjectFactory factory([[Foo alloc] init]); +// WeakNSObject weak_foo; // No pointer +// weak_foo = factory.GetWeakNSObject() // Now a weak reference is kept. +// [weak_foo description]; // Returns [foo description]. +// foo.reset(); // The reference is released. +// [weak_foo description]; // Returns nil, as weak_foo is pointing to nil. +// +// +// Implementation wise a WeakNSObject keeps a reference to a refcounted +// WeakContainer. There is one unique instance of a WeakContainer per watched +// NSObject, this relationship is maintained via the ObjectiveC associated +// object API, indirectly via an ObjectiveC CRBWeakNSProtocolSentinel class. +// +// Threading restrictions: +// - Several WeakNSObject pointing to the same underlying object must all be +// created and dereferenced on the same thread; +// - thread safety is enforced by the implementation, except: +// - it is allowed to destroy a WeakNSObject on any thread; +// - the implementation assumes that the tracked object will be released on the +// same thread that the WeakNSObject is created on. +// +// fml specifics: +// WeakNSObjects can only originate from a |WeakNSObjectFactory| (see below), though WeakNSObjects +// are copyable and movable. +// +// WeakNSObjects are not in general thread-safe. They may only be *used* on +// a single thread, namely the same thread as the "originating" +// |WeakNSObjectFactory| (which can invalidate the WeakNSObjects that it +// generates). +namespace fml { + +// Forward declaration, so |WeakNSObject| can friend it. +template +class WeakNSObjectFactory; + +// WeakContainer keeps a weak pointer to an object and clears it when it +// receives nullify() from the object's sentinel. +class WeakContainer : public fml::RefCountedThreadSafe { + public: + explicit WeakContainer(id object, std::shared_ptr checker = nullptr) + : checker_(checker), object_(object) {} + + id object() { + CheckThreadSafety(); + return object_; + } + + void nullify() { + CheckThreadSafety(); + object_ = nil; + } + + private: + friend fml::RefCountedThreadSafe; + ~WeakContainer() {} + + std::shared_ptr checker_; + id object_; + + void CheckThreadSafety() const { + if (!checker_) { + return; + } + FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_->checker); + } +}; + +} // namespace fml + +// Sentinel for observing the object contained in the weak pointer. The object +// will be deleted when the weak object is deleted and will notify its +// container. +@interface CRBWeakNSProtocolSentinel : NSObject +// Return the only associated container for this object. There can be only one. +// Will return null if object is nil . ++ (fml::RefPtr)containerForObject:(id)object + threadChecker: + (std::shared_ptr)checker; +@end + +namespace fml { + +// Base class for all WeakNSObject derivatives. +template +class WeakNSProtocol { + public: + WeakNSProtocol() = default; + + WeakNSProtocol(const WeakNSProtocol& that) { + // A WeakNSProtocol object can be copied on one thread and used on + // another. + container_ = that.container_; + } + + ~WeakNSProtocol() = default; + + void reset() { + CheckThreadSafety(); + container_ = [CRBWeakNSProtocolSentinel containerForObject:nil threadChecker:checker_]; + } + + NST get() const { + CheckThreadSafety(); + if (!container_.get()) { + return nil; + } + return container_->object(); + } + + WeakNSProtocol& operator=(const WeakNSProtocol& that) { + // A WeakNSProtocol object can be copied on one thread and used on + // another. + container_ = that.container_; + return *this; + } + + bool operator==(NST that) const { + CheckThreadSafety(); + return get() == that; + } + + bool operator!=(NST that) const { + CheckThreadSafety(); + return get() != that; + } + + operator NST() const { + CheckThreadSafety(); + return get(); + } + + protected: + friend class WeakNSObjectFactory; + + explicit WeakNSProtocol(std::shared_ptr checker, NST object = nil) + : checker_(checker) { + container_ = [CRBWeakNSProtocolSentinel containerForObject:object threadChecker:checker]; + } + + // Refecounted reference to the container tracking the ObjectiveC object this + // class encapsulates. + RefPtr container_; + std::shared_ptr checker_; + + void CheckThreadSafety() const { + if (!checker_) { + return; + } + FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_->checker); + } +}; + +// Free functions +template +bool operator==(NST p1, const WeakNSProtocol& p2) { + return p1 == p2.get(); +} + +template +bool operator!=(NST p1, const WeakNSProtocol& p2) { + return p1 != p2.get(); +} + +template +class WeakNSObject : public WeakNSProtocol { + public: + WeakNSObject() = default; + WeakNSObject(const WeakNSObject& that) : WeakNSProtocol(that) {} + + WeakNSObject& operator=(const WeakNSObject& that) { + WeakNSProtocol::operator=(that); + return *this; + } + + private: + friend class WeakNSObjectFactory; + + explicit WeakNSObject(std::shared_ptr checker, NST* object = nil) + : WeakNSProtocol(checker, object) {} +}; + +// Specialization to make WeakNSObject work. +template <> +class WeakNSObject : public WeakNSProtocol { + public: + WeakNSObject() = default; + WeakNSObject(const WeakNSObject& that) : WeakNSProtocol(that) {} + + WeakNSObject& operator=(const WeakNSObject& that) { + WeakNSProtocol::operator=(that); + return *this; + } + + private: + friend class WeakNSObjectFactory; + + explicit WeakNSObject(std::shared_ptr checker, id object = nil) + : WeakNSProtocol(checker, object) {} +}; + +// Class that produces (valid) |WeakNSObject|s. Typically, this is used as a +// member variable of |NST| (preferably the last one -- see below), and |NST|'s +// methods control how WeakNSObjects to it are vended. This class is not +// thread-safe, and should only be created, destroyed and used on a single +// thread. +// +// Example: +// +// ```objc +// @implementation Controller { +// std::unique_ptr> _weakFactory; +// } +// +// - (instancetype)init { +// self = [super init]; +// _weakFactory = std::make_unique>(self) +// } + +// - (fml::WeakNSObject) { +// return _weakFactory->GetWeakNSObject() +// } +// +// @end +// ``` +template +class WeakNSObjectFactory { + public: + explicit WeakNSObjectFactory(NST* object) : object_(object) { + FML_DCHECK(object_); + checker_ = std::make_shared(); + } + + ~WeakNSObjectFactory() { CheckThreadSafety(); } + + // Gets a new weak pointer, which will be valid until this object is + // destroyed. + WeakNSObject GetWeakNSObject() const { return WeakNSObject(checker_, object_); } + + private: + NST* object_; + + void CheckThreadSafety() const { + if (!checker_) { + return; + } + FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_->checker); + } + + std::shared_ptr checker_; + + FML_DISALLOW_COPY_AND_ASSIGN(WeakNSObjectFactory); +}; + +} // namespace fml + +#endif // FLUTTER_FML_PLATFORM_DARWIN_WEAK_NSOBJECT_H_ diff --git a/fml/platform/darwin/weak_nsobject.mm b/fml/platform/darwin/weak_nsobject.mm new file mode 100644 index 0000000000000..f902ad9f07cb8 --- /dev/null +++ b/fml/platform/darwin/weak_nsobject.mm @@ -0,0 +1,60 @@ +// 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. + +#include "flutter/fml/platform/darwin/weak_nsobject.h" +#include "flutter/fml/platform/darwin/scoped_nsautorelease_pool.h" +#include "flutter/fml/platform/darwin/scoped_nsobject.h" + +namespace { +// The key needed by objc_setAssociatedObject. +char sentinelObserverKey_; +} // namespace + +@interface CRBWeakNSProtocolSentinel () +// Container to notify on dealloc. +@property(readonly, assign) fml::RefPtr container; +// Designed initializer. +- (id)initWithContainer:(fml::RefPtr)container; +@end + +@implementation CRBWeakNSProtocolSentinel + +@synthesize container = container_; + ++ (fml::RefPtr)containerForObject:(id)object + threadChecker: + (std::shared_ptr)checker { + if (object == nil) + return nullptr; + // The autoreleasePool is needed here as the call to objc_getAssociatedObject + // returns an autoreleased object which is better released sooner than later. + fml::ScopedNSAutoreleasePool pool; + CRBWeakNSProtocolSentinel* sentinel = objc_getAssociatedObject(object, &sentinelObserverKey_); + if (!sentinel) { + fml::scoped_nsobject newSentinel([[CRBWeakNSProtocolSentinel alloc] + initWithContainer:AdoptRef(new fml::WeakContainer(object, checker))]); + sentinel = newSentinel; + objc_setAssociatedObject(object, &sentinelObserverKey_, sentinel, OBJC_ASSOCIATION_RETAIN); + // The retain count is 2. One retain is due to the alloc, the other to the + // association with the weak object. + FML_DCHECK(2u == [sentinel retainCount]); + } + return [sentinel container]; +} + +- (id)initWithContainer:(fml::RefPtr)container { + FML_DCHECK(container.get()); + self = [super init]; + if (self) { + container_ = container; + } + return self; +} + +- (void)dealloc { + self.container->nullify(); + [super dealloc]; +} + +@end diff --git a/fml/platform/darwin/weak_nsobject_unittests.mm b/fml/platform/darwin/weak_nsobject_unittests.mm new file mode 100644 index 0000000000000..e78d0dc0f865f --- /dev/null +++ b/fml/platform/darwin/weak_nsobject_unittests.mm @@ -0,0 +1,92 @@ +// 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. + +#include "flutter/fml/message_loop.h" +#include "flutter/fml/platform/darwin/scoped_nsobject.h" +#include "flutter/fml/platform/darwin/weak_nsobject.h" +#include "flutter/fml/task_runner.h" +#include "flutter/fml/thread.h" +#include "gtest/gtest.h" + +namespace fml { +namespace { + +TEST(WeakNSObjectTest, WeakNSObject) { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + WeakNSObject w1 = factory.GetWeakNSObject(); + EXPECT_TRUE(w1); + p1.reset(); + EXPECT_FALSE(w1); +} + +TEST(WeakNSObjectTest, MultipleWeakNSObject) { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + WeakNSObject w1 = factory.GetWeakNSObject(); + WeakNSObject w2(w1); + EXPECT_TRUE(w1); + EXPECT_TRUE(w2); + EXPECT_TRUE(w1.get() == w2.get()); + p1.reset(); + EXPECT_FALSE(w1); + EXPECT_FALSE(w2); +} + +TEST(WeakNSObjectTest, WeakNSObjectDies) { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + { + WeakNSObject w1 = factory.GetWeakNSObject(); + EXPECT_TRUE(w1); + } +} + +TEST(WeakNSObjectTest, WeakNSObjectReset) { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + WeakNSObject w1 = factory.GetWeakNSObject(); + EXPECT_TRUE(w1); + w1.reset(); + EXPECT_FALSE(w1); + EXPECT_TRUE(p1); + EXPECT_TRUE([p1 description]); +} + +TEST(WeakNSObjectTest, WeakNSObjectEmpty) { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObject w1; + EXPECT_FALSE(w1); + WeakNSObjectFactory factory(p1.get()); + w1 = factory.GetWeakNSObject(); + EXPECT_TRUE(w1); + p1.reset(); + EXPECT_FALSE(w1); +} + +TEST(WeakNSObjectTest, WeakNSObjectCopy) { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + WeakNSObject w1 = factory.GetWeakNSObject(); + WeakNSObject w2(w1); + EXPECT_TRUE(w1); + EXPECT_TRUE(w2); + p1.reset(); + EXPECT_FALSE(w1); + EXPECT_FALSE(w2); +} + +TEST(WeakNSObjectTest, WeakNSObjectAssignment) { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + WeakNSObject w1 = factory.GetWeakNSObject(); + WeakNSObject w2 = w1; + EXPECT_TRUE(w1); + EXPECT_TRUE(w2); + p1.reset(); + EXPECT_FALSE(w1); + EXPECT_FALSE(w2); +} +} // namespace +} // namespace fml diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 619b55695b347..148d83a9cbecf 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -11,6 +11,7 @@ #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" @@ -116,7 +117,7 @@ @implementation FlutterEngine { std::shared_ptr _threadHost; std::unique_ptr _shell; NSString* _labelPrefix; - std::unique_ptr> _weakFactory; + std::unique_ptr> _weakFactory; fml::WeakPtr _viewController; fml::scoped_nsobject _publisher; @@ -189,7 +190,7 @@ - (instancetype)initWithName:(NSString*)labelPrefix _allowHeadlessExecution = allowHeadlessExecution; _labelPrefix = [labelPrefix copy]; - _weakFactory = std::make_unique>(self); + _weakFactory = std::make_unique>(self); if (project == nil) { _dartProject.reset([[FlutterDartProject alloc] init]); @@ -327,8 +328,8 @@ - (void)dealloc { return *_shell; } -- (fml::WeakPtr)getWeakPtr { - return _weakFactory->GetWeakPtr(); +- (fml::WeakNSObject)getWeakNSObject { + return _weakFactory->GetWeakNSObject(); } - (void)updateViewportMetrics:(flutter::ViewportMetrics)viewportMetrics { @@ -584,7 +585,7 @@ - (void)startProfiler { - (void)setUpChannels { // This will be invoked once the shell is done setting up and the isolate ID // for the UI isolate is available. - fml::WeakPtr weakSelf = [self getWeakPtr]; + fml::WeakNSObject weakSelf = [self getWeakNSObject]; [_binaryMessenger setMessageHandlerOnChannel:@"flutter/isolate" binaryMessageHandler:^(NSData* message, FlutterBinaryReply reply) { if (weakSelf) { @@ -674,7 +675,7 @@ - (void)setUpChannels { [[FlutterUndoManagerPlugin alloc] initWithDelegate:self]; _undoManagerPlugin.reset(undoManagerPlugin); - _platformPlugin.reset([[FlutterPlatformPlugin alloc] initWithEngine:[self getWeakPtr]]); + _platformPlugin.reset([[FlutterPlatformPlugin alloc] initWithEngine:[self getWeakNSObject]]); _restorationPlugin.reset([[FlutterRestorationPlugin alloc] initWithChannel:_restorationChannel.get() @@ -719,7 +720,7 @@ - (void)maybeSetupPlatformViewChannels { [platformPlugin handleMethodCall:call result:result]; }]; - fml::WeakPtr weakSelf = [self getWeakPtr]; + fml::WeakNSObject weakSelf = [self getWeakNSObject]; [_platformViewsChannel.get() setMethodCallHandler:^(FlutterMethodCall* call, FlutterResult result) { if (weakSelf) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.h index 41dd485dc2c41..5c0498f979ac3 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.h @@ -5,14 +5,14 @@ #ifndef SHELL_PLATFORM_IOS_FRAMEWORK_SOURCE_FLUTTERPLATFORMPLUGIN_H_ #define SHELL_PLATFORM_IOS_FRAMEWORK_SOURCE_FLUTTERPLATFORMPLUGIN_H_ -#include "flutter/fml/memory/weak_ptr.h" +#include "flutter/fml/platform/darwin/weak_nsobject.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h" #import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterEngine.h" @interface FlutterPlatformPlugin : NSObject - (instancetype)init NS_UNAVAILABLE; + (instancetype)new NS_UNAVAILABLE; -- (instancetype)initWithEngine:(fml::WeakPtr)engine NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithEngine:(fml::WeakNSObject)engine NS_DESIGNATED_INITIALIZER; - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm index 722571bda0a03..c1d7eaea3296e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm @@ -72,12 +72,12 @@ @interface FlutterPlatformPlugin () @end @implementation FlutterPlatformPlugin { - fml::WeakPtr _engine; + fml::WeakNSObject _engine; // Used to detect whether this device has live text input ability or not. UITextField* _textField; } -- (instancetype)initWithEngine:(fml::WeakPtr)engine { +- (instancetype)initWithEngine:(fml::WeakNSObject)engine { FML_DCHECK(engine) << "engine must be set"; self = [super init]; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm index 73cc460b351f3..706c5aab0cd8e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm @@ -5,6 +5,7 @@ #import #import +#import "flutter/fml/platform/darwin/weak_nsobject.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" #import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h" @@ -34,15 +35,15 @@ - (void)testSearchWebInvokedWithEscapedTerm { OCMStub([mockApplication sharedApplication]).andReturn(mockApplication); FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); [engine runWithEntrypoint:nil]; XCTestExpectation* invokeExpectation = [self expectationWithDescription:@"Web search launched with escaped search term"]; FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakPtr()] autorelease]; + [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"SearchWeb.invoke" @@ -68,15 +69,15 @@ - (void)testSearchWebInvokedWithNonEscapedTerm { OCMStub([mockApplication sharedApplication]).andReturn(mockApplication); FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); [engine runWithEntrypoint:nil]; XCTestExpectation* invokeExpectation = [self expectationWithDescription:@"Web search launched with non escaped search term"]; FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakPtr()] autorelease]; + [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"SearchWeb.invoke" @@ -100,8 +101,8 @@ - (void)testSearchWebInvokedWithNonEscapedTerm { - (void)testLookUpCallInitiated { FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; [engine runWithEntrypoint:nil]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); XCTestExpectation* presentExpectation = [self expectationWithDescription:@"Look Up view controller presented"]; @@ -111,7 +112,7 @@ - (void)testLookUpCallInitiated { FlutterViewController* mockEngineViewController = OCMPartialMock(engineViewController); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakPtr()] autorelease]; + [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"LookUp.invoke" @@ -130,8 +131,8 @@ - (void)testLookUpCallInitiated { - (void)testShareScreenInvoked { FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; [engine runWithEntrypoint:nil]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); XCTestExpectation* presentExpectation = [self expectationWithDescription:@"Share view controller presented"]; @@ -145,7 +146,7 @@ - (void)testShareScreenInvoked { completion:nil]); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakPtr()] autorelease]; + [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"Share.invoke" @@ -164,10 +165,10 @@ - (void)testShareScreenInvoked { - (void)testClipboardHasCorrectStrings { [UIPasteboard generalPasteboard].string = nil; FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakPtr()] autorelease]; + [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; XCTestExpectation* setStringExpectation = [self expectationWithDescription:@"setString"]; FlutterResult resultSet = ^(id result) { @@ -203,10 +204,10 @@ - (void)testClipboardHasCorrectStrings { - (void)testClipboardSetDataToNullDoNotCrash { [UIPasteboard generalPasteboard].string = nil; FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakPtr()] autorelease]; + [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; XCTestExpectation* setStringExpectation = [self expectationWithDescription:@"setData"]; FlutterResult resultSet = ^(id result) { @@ -237,10 +238,10 @@ - (void)testPopSystemNavigator { initWithRootViewController:flutterViewController] autorelease]; UITabBarController* tabBarController = [[[UITabBarController alloc] init] autorelease]; tabBarController.viewControllers = @[ navigationController ]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakPtr()] autorelease]; + [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; id navigationControllerMock = OCMPartialMock(navigationController); OCMStub([navigationControllerMock popViewControllerAnimated:YES]); @@ -261,12 +262,12 @@ - (void)testPopSystemNavigator { - (void)testWhetherDeviceHasLiveTextInputInvokeCorrectly { FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); XCTestExpectation* invokeExpectation = [self expectationWithDescription:@"isLiveTextInputAvailableInvoke"]; FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakPtr()] autorelease]; + [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"LiveText.isLiveTextInputAvailable" From 8f4671c2bb1f94c94e7c59013ad0601e484ef4d1 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Sat, 11 Nov 2023 06:11:26 -0800 Subject: [PATCH 2/8] fix clang-tidy warning ignore clang-tidy in test --- fml/platform/darwin/weak_nsobject.mm | 3 ++- fml/platform/darwin/weak_nsobject_unittests.mm | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/fml/platform/darwin/weak_nsobject.mm b/fml/platform/darwin/weak_nsobject.mm index f902ad9f07cb8..4398edef8e2fd 100644 --- a/fml/platform/darwin/weak_nsobject.mm +++ b/fml/platform/darwin/weak_nsobject.mm @@ -25,8 +25,9 @@ @implementation CRBWeakNSProtocolSentinel + (fml::RefPtr)containerForObject:(id)object threadChecker: (std::shared_ptr)checker { - if (object == nil) + if (object == nil) { return nullptr; + } // The autoreleasePool is needed here as the call to objc_getAssociatedObject // returns an autoreleased object which is better released sooner than later. fml::ScopedNSAutoreleasePool pool; diff --git a/fml/platform/darwin/weak_nsobject_unittests.mm b/fml/platform/darwin/weak_nsobject_unittests.mm index e78d0dc0f865f..0b93cdf449ec7 100644 --- a/fml/platform/darwin/weak_nsobject_unittests.mm +++ b/fml/platform/darwin/weak_nsobject_unittests.mm @@ -25,6 +25,7 @@ scoped_nsobject p1([[NSObject alloc] init]); WeakNSObjectFactory factory(p1.get()); WeakNSObject w1 = factory.GetWeakNSObject(); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) WeakNSObject w2(w1); EXPECT_TRUE(w1); EXPECT_TRUE(w2); @@ -69,6 +70,7 @@ scoped_nsobject p1([[NSObject alloc] init]); WeakNSObjectFactory factory(p1.get()); WeakNSObject w1 = factory.GetWeakNSObject(); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) WeakNSObject w2(w1); EXPECT_TRUE(w1); EXPECT_TRUE(w2); @@ -81,6 +83,7 @@ scoped_nsobject p1([[NSObject alloc] init]); WeakNSObjectFactory factory(p1.get()); WeakNSObject w1 = factory.GetWeakNSObject(); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) WeakNSObject w2 = w1; EXPECT_TRUE(w1); EXPECT_TRUE(w2); From 663ecd2c4cb5352699b8f16096426519991afc71 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 13 Nov 2023 10:58:21 -0800 Subject: [PATCH 3/8] migrate to weak_nsobject --- fml/platform/darwin/weak_nsobject.h | 65 +++++++------------ fml/platform/darwin/weak_nsobject.mm | 3 +- .../ios/framework/Source/FlutterEngine.mm | 6 +- .../framework/Source/FlutterKeyboardManager.h | 1 - .../Source/FlutterKeyboardManager.mm | 12 ++-- .../Source/FlutterPlatformPluginTest.mm | 8 +-- .../framework/Source/FlutterViewController.mm | 16 ++--- .../Source/FlutterViewController_Internal.h | 5 +- .../Source/accessibility_bridge_test.mm | 10 +-- shell/platform/darwin/ios/platform_view_ios.h | 8 +-- .../platform/darwin/ios/platform_view_ios.mm | 4 +- 11 files changed, 59 insertions(+), 79 deletions(-) diff --git a/fml/platform/darwin/weak_nsobject.h b/fml/platform/darwin/weak_nsobject.h index 89c12e4a7ac92..d64d820900c73 100644 --- a/fml/platform/darwin/weak_nsobject.h +++ b/fml/platform/darwin/weak_nsobject.h @@ -58,6 +58,11 @@ struct DebugThreadChecker { // a single thread, namely the same thread as the "originating" // |WeakNSObjectFactory| (which can invalidate the WeakNSObjects that it // generates). +// +// However, WeakNSObject may be passed to other threads, reset on other +// threads, or destroyed on other threads. They may also be reassigned on +// other threads (in which case they should then only be used on the thread +// corresponding to the new "originating" |WeakNSObjectFactory|). namespace fml { // Forward declaration, so |WeakNSObject| can friend it. @@ -68,7 +73,7 @@ class WeakNSObjectFactory; // receives nullify() from the object's sentinel. class WeakContainer : public fml::RefCountedThreadSafe { public: - explicit WeakContainer(id object, std::shared_ptr checker = nullptr) + explicit WeakContainer(id object, debug::DebugThreadChecker checker) : checker_(checker), object_(object) {} id object() { @@ -76,24 +81,16 @@ class WeakContainer : public fml::RefCountedThreadSafe { return object_; } - void nullify() { - CheckThreadSafety(); - object_ = nil; - } + void nullify() { object_ = nil; } private: friend fml::RefCountedThreadSafe; ~WeakContainer() {} - std::shared_ptr checker_; + debug::DebugThreadChecker checker_; id object_; - void CheckThreadSafety() const { - if (!checker_) { - return; - } - FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_->checker); - } + void CheckThreadSafety() const { FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); } }; } // namespace fml @@ -105,8 +102,7 @@ class WeakContainer : public fml::RefCountedThreadSafe { // Return the only associated container for this object. There can be only one. // Will return null if object is nil . + (fml::RefPtr)containerForObject:(id)object - threadChecker: - (std::shared_ptr)checker; + threadChecker:(debug::DebugThreadChecker)checker; @end namespace fml { @@ -117,16 +113,14 @@ class WeakNSProtocol { public: WeakNSProtocol() = default; - WeakNSProtocol(const WeakNSProtocol& that) { - // A WeakNSProtocol object can be copied on one thread and used on - // another. - container_ = that.container_; - } + // A WeakNSProtocol object can be copied on one thread and used on + // another. + WeakNSProtocol(const WeakNSProtocol& that) + : container_(that.container_), checker_(that.checker_) {} ~WeakNSProtocol() = default; void reset() { - CheckThreadSafety(); container_ = [CRBWeakNSProtocolSentinel containerForObject:nil threadChecker:checker_]; } @@ -142,6 +136,7 @@ class WeakNSProtocol { // A WeakNSProtocol object can be copied on one thread and used on // another. container_ = that.container_; + checker_ = that.checker_; return *this; } @@ -163,22 +158,16 @@ class WeakNSProtocol { protected: friend class WeakNSObjectFactory; - explicit WeakNSProtocol(std::shared_ptr checker, NST object = nil) - : checker_(checker) { + explicit WeakNSProtocol(debug::DebugThreadChecker checker, NST object = nil) : checker_(checker) { container_ = [CRBWeakNSProtocolSentinel containerForObject:object threadChecker:checker]; } // Refecounted reference to the container tracking the ObjectiveC object this // class encapsulates. RefPtr container_; - std::shared_ptr checker_; + debug::DebugThreadChecker checker_; - void CheckThreadSafety() const { - if (!checker_) { - return; - } - FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_->checker); - } + void CheckThreadSafety() const { FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); } }; // Free functions @@ -206,7 +195,7 @@ class WeakNSObject : public WeakNSProtocol { private: friend class WeakNSObjectFactory; - explicit WeakNSObject(std::shared_ptr checker, NST* object = nil) + explicit WeakNSObject(debug::DebugThreadChecker checker, NST* object = nil) : WeakNSProtocol(checker, object) {} }; @@ -225,7 +214,7 @@ class WeakNSObject : public WeakNSProtocol { private: friend class WeakNSObjectFactory; - explicit WeakNSObject(std::shared_ptr checker, id object = nil) + explicit WeakNSObject(debug::DebugThreadChecker checker, id object = nil) : WeakNSProtocol(checker, object) {} }; @@ -256,10 +245,7 @@ class WeakNSObject : public WeakNSProtocol { template class WeakNSObjectFactory { public: - explicit WeakNSObjectFactory(NST* object) : object_(object) { - FML_DCHECK(object_); - checker_ = std::make_shared(); - } + explicit WeakNSObjectFactory(NST* object) : object_(object) { FML_DCHECK(object_); } ~WeakNSObjectFactory() { CheckThreadSafety(); } @@ -270,14 +256,9 @@ class WeakNSObjectFactory { private: NST* object_; - void CheckThreadSafety() const { - if (!checker_) { - return; - } - FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_->checker); - } + void CheckThreadSafety() const { FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); } - std::shared_ptr checker_; + debug::DebugThreadChecker checker_; FML_DISALLOW_COPY_AND_ASSIGN(WeakNSObjectFactory); }; diff --git a/fml/platform/darwin/weak_nsobject.mm b/fml/platform/darwin/weak_nsobject.mm index 4398edef8e2fd..8661e69f67555 100644 --- a/fml/platform/darwin/weak_nsobject.mm +++ b/fml/platform/darwin/weak_nsobject.mm @@ -23,8 +23,7 @@ @implementation CRBWeakNSProtocolSentinel @synthesize container = container_; + (fml::RefPtr)containerForObject:(id)object - threadChecker: - (std::shared_ptr)checker { + threadChecker:(debug::DebugThreadChecker)checker { if (object == nil) { return nullptr; } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 148d83a9cbecf..8d16c6d0d20de 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -119,7 +119,7 @@ @implementation FlutterEngine { NSString* _labelPrefix; std::unique_ptr> _weakFactory; - fml::WeakPtr _viewController; + fml::WeakNSObject _viewController; fml::scoped_nsobject _publisher; std::shared_ptr _platformViewsController; @@ -424,8 +424,8 @@ - (void)ensureSemanticsEnabled { - (void)setViewController:(FlutterViewController*)viewController { FML_DCHECK(self.iosPlatformView); - _viewController = - viewController ? [viewController getWeakPtr] : fml::WeakPtr(); + _viewController = viewController ? [viewController getWeakNSObject] + : fml::WeakNSObject(); self.iosPlatformView->SetOwnerViewController(_viewController); [self maybeSetupPlatformViewChannels]; [self updateDisplays]; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.h b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.h index ebdcae1d2d2a7..abfde38ef4217 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.h @@ -10,7 +10,6 @@ #import #import -#include "flutter/fml/memory/weak_ptr.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterKeyPrimaryResponder.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterKeySecondaryResponder.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterUIPressProxy.h" diff --git a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm index db6aeca077bac..524dc6f43f791 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm @@ -3,8 +3,8 @@ // found in the LICENSE file. #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.h" -#include "flutter/fml/memory/weak_ptr.h" #include "flutter/fml/platform/darwin/message_loop_darwin.h" +#include "flutter/fml/platform/darwin/weak_nsobject.h" static constexpr CFTimeInterval kDistantFuture = 1.0e10; @@ -29,7 +29,7 @@ - (void)dispatchToSecondaryResponders:(nonnull FlutterUIPressProxy*)press @end @implementation FlutterKeyboardManager { - std::unique_ptr> _weakFactory; + std::unique_ptr> _weakFactory; } - (nonnull instancetype)init { @@ -37,7 +37,7 @@ - (nonnull instancetype)init { if (self != nil) { _primaryResponders = [[NSMutableArray alloc] init]; _secondaryResponders = [[NSMutableArray alloc] init]; - _weakFactory = std::make_unique>(self); + _weakFactory = std::make_unique>(self); } return self; } @@ -64,8 +64,8 @@ - (void)dealloc { [super dealloc]; } -- (fml::WeakPtr)getWeakPtr { - return _weakFactory->GetWeakPtr(); +- (fml::WeakNSObject)getWeakNSObject { + return _weakFactory->GetWeakNSObject(); } - (void)handlePress:(nonnull FlutterUIPressProxy*)press @@ -89,7 +89,7 @@ - (void)handlePress:(nonnull FlutterUIPressProxy*)press // encounter. NSAssert([_primaryResponders count] >= 0, @"At least one primary responder must be added."); - __block auto weakSelf = [self getWeakPtr]; + __block auto weakSelf = [self getWeakNSObject]; __block NSUInteger unreplied = [self.primaryResponders count]; __block BOOL anyHandled = false; FlutterAsyncKeyCallback replyCallback = ^(BOOL handled) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm index 706c5aab0cd8e..65226839d365a 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm @@ -290,8 +290,8 @@ - (void)testViewControllerBasedStatusBarHiddenUpdate { [engine runWithEntrypoint:nil]; FlutterViewController* flutterViewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); XCTAssertFalse(flutterViewController.prefersStatusBarHidden); // Update to hidden. @@ -331,8 +331,8 @@ - (void)testViewControllerBasedStatusBarHiddenUpdate { [engine runWithEntrypoint:nil]; FlutterViewController* flutterViewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; - std::unique_ptr> _weakFactory = - std::make_unique>(engine); + std::unique_ptr> _weakFactory = + std::make_unique>(engine); XCTAssertFalse(flutterViewController.prefersStatusBarHidden); // Update to hidden. diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 9ebc028edf5ad..0a9307cac9b21 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -112,7 +112,7 @@ - (void)deregisterNotifications; @end @implementation FlutterViewController { - std::unique_ptr> _weakFactory; + std::unique_ptr> _weakFactory; fml::scoped_nsobject _engine; // We keep a separate reference to this and create it ahead of time because we want to be able to @@ -171,7 +171,7 @@ - (instancetype)initWithEngine:(FlutterEngine*)engine _flutterView.reset([[FlutterView alloc] initWithDelegate:_engine opaque:self.isViewOpaque enableWideGamut:engine.project.isWideGamutEnabled]); - _weakFactory = std::make_unique>(self); + _weakFactory = std::make_unique>(self); _ongoingTouches.reset([[NSMutableSet alloc] init]); [self performCommonViewControllerInitialization]; @@ -232,7 +232,7 @@ - (void)sharedSetupWithProject:(nullable FlutterDartProject*)project project = [[[FlutterDartProject alloc] init] autorelease]; } FlutterView.forceSoftwareRendering = project.settings.enable_software_rendering; - _weakFactory = std::make_unique>(self); + _weakFactory = std::make_unique>(self); auto engine = fml::scoped_nsobject{[[FlutterEngine alloc] initWithName:@"io.flutter" project:project @@ -286,8 +286,8 @@ - (FlutterEngine*)engine { return _engine.get(); } -- (fml::WeakPtr)getWeakPtr { - return _weakFactory->GetWeakPtr(); +- (fml::WeakNSObject)getWeakNSObject { + return _weakFactory->GetWeakNSObject(); } - (void)setUpNotificationCenterObservers { @@ -617,7 +617,7 @@ - (void)installFirstFrameCallback { } // Start on the platform thread. - weakPlatformView->SetNextFrameCallback([weakSelf = [self getWeakPtr], + weakPlatformView->SetNextFrameCallback([weakSelf = [self getWeakNSObject], platformTaskRunner = [_engine.get() platformTaskRunner], rasterTaskRunner = [_engine.get() rasterTaskRunner]]() { FML_DCHECK(rasterTaskRunner->RunsTasksOnCurrentThread()); @@ -800,7 +800,7 @@ - (void)viewDidLoad { - (void)addInternalPlugins { self.keyboardManager = [[[FlutterKeyboardManager alloc] init] autorelease]; - fml::WeakPtr weakSelf = [self getWeakPtr]; + fml::WeakNSObject weakSelf = [self getWeakNSObject]; FlutterSendKeyEvent sendEvent = ^(const FlutterKeyEvent& event, FlutterKeyEventCallback callback, void* userData) { if (weakSelf) { @@ -1702,7 +1702,7 @@ - (void)startKeyBoardAnimation:(NSTimeInterval)duration { // Invalidate old vsync client if old animation is not completed. [self invalidateKeyboardAnimationVSyncClient]; - fml::WeakPtr weakSelf = [self getWeakPtr]; + fml::WeakNSObject weakSelf = [self getWeakNSObject]; FlutterKeyboardAnimationCallback keyboardAnimationCallback = ^( fml::TimePoint keyboardAnimationTargetTime) { if (!weakSelf) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h index 657c189dc1628..66ed0d1afeea2 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h @@ -5,7 +5,8 @@ #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/memory/weak_ptr.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" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterKeySecondaryResponder.h" @@ -51,7 +52,7 @@ typedef void (^FlutterKeyboardAnimationCallback)(fml::TimePoint); */ @property(nonatomic, assign, readwrite) BOOL prefersStatusBarHidden; -- (fml::WeakPtr)getWeakPtr; +- (fml::WeakNSObject)getWeakNSObject; - (std::shared_ptr&)platformViewsController; - (FlutterRestorationPlugin*)restorationPlugin; // Send touches to the Flutter Engine while forcing the change type to be cancelled. diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm index ac44da5370b9d..5b75abf8a4b6b 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm @@ -1979,8 +1979,8 @@ - (void)testAccessibilityMessageAfterDeletion { fml::AutoResetWaitableEvent latch; thread_task_runner->PostTask([&] { auto weakFactory = - std::make_unique>(flutterViewController); - platform_view->SetOwnerViewController(weakFactory->GetWeakPtr()); + std::make_unique>(flutterViewController); + platform_view->SetOwnerViewController(weakFactory->GetWeakNSObject()); auto bridge = std::make_unique(/*view=*/nil, /*platform_view=*/platform_view.get(), @@ -2067,9 +2067,9 @@ - (void)testPlatformViewDestructorDoesNotCallSemanticsAPIs { std::make_shared(); OCMStub([mockFlutterViewController platformViewsController]) .andReturn(flutterPlatformViewsController.get()); - auto weakFactory = - std::make_unique>(mockFlutterViewController); - platform_view->SetOwnerViewController(weakFactory->GetWeakPtr()); + auto weakFactory = std::make_unique>( + mockFlutterViewController); + platform_view->SetOwnerViewController(weakFactory->GetWeakNSObject()); platform_view->SetSemanticsEnabled(true); XCTAssertNotEqual(test_delegate.set_semantics_enabled_calls, 0); diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h index fa48ce8ee309b..5002c00fefbd6 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/shell/platform/darwin/ios/platform_view_ios.h @@ -9,8 +9,8 @@ #include "flutter/fml/closure.h" #include "flutter/fml/macros.h" -#include "flutter/fml/memory/weak_ptr.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" @@ -59,14 +59,14 @@ class PlatformViewIOS final : public PlatformView { * Returns the `FlutterViewController` currently attached to the `FlutterEngine` owning * this PlatformViewIOS. */ - fml::WeakPtr GetOwnerViewController() const; + fml::WeakNSObject 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::WeakPtr& owner_controller); + void SetOwnerViewController(const fml::WeakNSObject& owner_controller); /** * Called one time per `FlutterViewController` when the `FlutterViewController`'s @@ -133,7 +133,7 @@ class PlatformViewIOS final : public PlatformView { std::function set_semantics_enabled_; }; - fml::WeakPtr owner_controller_; + fml::WeakNSObject 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_; diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index 66636fe084680..d4de687a2f2cc 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -76,12 +76,12 @@ new PlatformMessageHandlerIos(task_runners.GetPlatformTaskRunner())) {} platform_message_handler_->HandlePlatformMessage(std::move(message)); } -fml::WeakPtr PlatformViewIOS::GetOwnerViewController() const { +fml::WeakNSObject PlatformViewIOS::GetOwnerViewController() const { return owner_controller_; } void PlatformViewIOS::SetOwnerViewController( - const fml::WeakPtr& owner_controller) { + const fml::WeakNSObject& owner_controller) { FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); std::lock_guard guard(ios_surface_mutex_); if (ios_surface_ || !owner_controller) { From a872879aa6ffa87fea1bf2098d4f8d2c302977d8 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 13 Nov 2023 11:55:48 -0800 Subject: [PATCH 4/8] make wean_nsobject support arc https://codereview.chromium.org/2943263003/patch/1/10002 --- ci/licenses_golden/excluded_files | 1 + fml/BUILD.gn | 5 +- fml/platform/darwin/weak_nsobject.h | 7 +- fml/platform/darwin/weak_nsobject.mm | 9 ++ .../darwin/weak_nsobject_arc_unittests.mm | 121 ++++++++++++++++++ 5 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 fml/platform/darwin/weak_nsobject_arc_unittests.mm diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index fc9d63570de08..148ff2d6aedc6 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -104,6 +104,7 @@ ../../../flutter/fml/platform/darwin/scoped_nsobject_arc_unittests.mm ../../../flutter/fml/platform/darwin/scoped_nsobject_unittests.mm ../../../flutter/fml/platform/darwin/string_range_sanitization_unittests.mm +../../../flutter/fml/platform/darwin/weak_nsobject_arc_unittests.mm ../../../flutter/fml/platform/darwin/weak_nsobject_unittests.mm ../../../flutter/fml/platform/win/file_win_unittests.cc ../../../flutter/fml/platform/win/wstring_conversion_unittests.cc diff --git a/fml/BUILD.gn b/fml/BUILD.gn index d07a421104391..063c0aee65068 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -404,7 +404,10 @@ if (enable_unittests) { testonly = true if (is_mac || is_ios) { cflags_objcc = flutter_cflags_objc_arc - sources = [ "platform/darwin/scoped_nsobject_arc_unittests.mm" ] + sources = [ + "platform/darwin/scoped_nsobject_arc_unittests.mm", + "platform/darwin/weak_nsobject_arc_unittests.mm", + ] } deps = [ diff --git a/fml/platform/darwin/weak_nsobject.h b/fml/platform/darwin/weak_nsobject.h index d64d820900c73..cef84d170c2b7 100644 --- a/fml/platform/darwin/weak_nsobject.h +++ b/fml/platform/darwin/weak_nsobject.h @@ -73,8 +73,7 @@ class WeakNSObjectFactory; // receives nullify() from the object's sentinel. class WeakContainer : public fml::RefCountedThreadSafe { public: - explicit WeakContainer(id object, debug::DebugThreadChecker checker) - : checker_(checker), object_(object) {} + explicit WeakContainer(id object, debug::DebugThreadChecker checker); id object() { CheckThreadSafety(); @@ -85,10 +84,10 @@ class WeakContainer : public fml::RefCountedThreadSafe { private: friend fml::RefCountedThreadSafe; - ~WeakContainer() {} + ~WeakContainer(); debug::DebugThreadChecker checker_; - id object_; + __unsafe_unretained id object_; void CheckThreadSafety() const { FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); } }; diff --git a/fml/platform/darwin/weak_nsobject.mm b/fml/platform/darwin/weak_nsobject.mm index 8661e69f67555..565c7a25a0b3c 100644 --- a/fml/platform/darwin/weak_nsobject.mm +++ b/fml/platform/darwin/weak_nsobject.mm @@ -11,6 +11,15 @@ char sentinelObserverKey_; } // namespace +namespace fml { + +WeakContainer::WeakContainer(id object, debug::DebugThreadChecker checker) + : checker_(checker), object_(object) {} + +WeakContainer::~WeakContainer() {} + +} // namespace fml + @interface CRBWeakNSProtocolSentinel () // Container to notify on dealloc. @property(readonly, assign) fml::RefPtr container; diff --git a/fml/platform/darwin/weak_nsobject_arc_unittests.mm b/fml/platform/darwin/weak_nsobject_arc_unittests.mm new file mode 100644 index 0000000000000..7e09b746d5d89 --- /dev/null +++ b/fml/platform/darwin/weak_nsobject_arc_unittests.mm @@ -0,0 +1,121 @@ +// 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. + +#include "flutter/fml/message_loop.h" +#include "flutter/fml/platform/darwin/scoped_nsobject.h" +#include "flutter/fml/platform/darwin/weak_nsobject.h" +#include "flutter/fml/task_runner.h" +#include "flutter/fml/thread.h" +#include "gtest/gtest.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace fml { +namespace { + +TEST(WeakNSObjectTestARC, WeakNSObject) { + scoped_nsobject p1; + WeakNSObject w1; + @autoreleasepool { + p1.reset(([[NSObject alloc] init])); + WeakNSObjectFactory factory(p1.get()); + w1 = factory.GetWeakNSObject(); + EXPECT_TRUE(w1); + p1.reset(); + } + EXPECT_FALSE(w1); +} + +TEST(WeakNSObjectTestARC, MultipleWeakNSObject) { + scoped_nsobject p1; + WeakNSObject w1; + WeakNSObject w2; + @autoreleasepool { + p1.reset([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + w1 = factory.GetWeakNSObject(); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) + w2 = w1; + EXPECT_TRUE(w1); + EXPECT_TRUE(w2); + EXPECT_TRUE(w1.get() == w2.get()); + p1.reset(); + } + EXPECT_FALSE(w1); + EXPECT_FALSE(w2); +} + +TEST(WeakNSObjectTestARC, WeakNSObjectDies) { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + { + WeakNSObject w1 = factory.GetWeakNSObject(); + EXPECT_TRUE(w1); + } +} + +TEST(WeakNSObjectTestARC, WeakNSObjectReset) { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + WeakNSObject w1 = factory.GetWeakNSObject(); + EXPECT_TRUE(w1); + w1.reset(); + EXPECT_FALSE(w1); + EXPECT_TRUE(p1); + EXPECT_TRUE([p1 description]); +} + +TEST(WeakNSObjectTestARC, WeakNSObjectEmpty) { + scoped_nsobject p1; + WeakNSObject w1; + @autoreleasepool { + scoped_nsobject p1([[NSObject alloc] init]); + EXPECT_FALSE(w1); + WeakNSObjectFactory factory(p1.get()); + w1 = factory.GetWeakNSObject(); + EXPECT_TRUE(w1); + p1.reset(); + } + EXPECT_FALSE(w1); +} + +TEST(WeakNSObjectTestARC, WeakNSObjectCopy) { + scoped_nsobject p1; + WeakNSObject w1; + WeakNSObject w2; + @autoreleasepool { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + w1 = factory.GetWeakNSObject(); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) + w2 = w1; + EXPECT_TRUE(w1); + EXPECT_TRUE(w2); + p1.reset(); + } + EXPECT_FALSE(w1); + EXPECT_FALSE(w2); +} + +TEST(WeakNSObjectTestARC, WeakNSObjectAssignment) { + scoped_nsobject p1; + WeakNSObject w1; + WeakNSObject w2; + @autoreleasepool { + scoped_nsobject p1([[NSObject alloc] init]); + WeakNSObjectFactory factory(p1.get()); + w1 = factory.GetWeakNSObject(); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) + w2 = w1; + EXPECT_TRUE(w1); + EXPECT_TRUE(w2); + p1.reset(); + } + EXPECT_FALSE(w1); + EXPECT_FALSE(w2); +} +} // namespace +} // namespace fml From 7124bf9cef6e9b420277b831db93afa9dc974d60 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 13 Nov 2023 19:48:13 -0800 Subject: [PATCH 5/8] fix weakFactory in arc, migrate a test containing weakNSObject to arc fix clang-tidy fix warning --- fml/platform/darwin/weak_nsobject.h | 37 +++++++----- fml/platform/darwin/weak_nsobject.mm | 2 +- shell/platform/darwin/ios/BUILD.gn | 2 +- .../Source/FlutterPlatformPluginTest.mm | 57 ++++++++++--------- 4 files changed, 54 insertions(+), 44 deletions(-) diff --git a/fml/platform/darwin/weak_nsobject.h b/fml/platform/darwin/weak_nsobject.h index cef84d170c2b7..c636c6dc895ba 100644 --- a/fml/platform/darwin/weak_nsobject.h +++ b/fml/platform/darwin/weak_nsobject.h @@ -86,10 +86,15 @@ class WeakContainer : public fml::RefCountedThreadSafe { friend fml::RefCountedThreadSafe; ~WeakContainer(); - debug::DebugThreadChecker checker_; __unsafe_unretained id object_; void CheckThreadSafety() const { FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); } + +// checker_ is unused in non-unopt mode. +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-private-field" + debug::DebugThreadChecker checker_; +#pragma clang diagnostic pop }; } // namespace fml @@ -157,16 +162,15 @@ class WeakNSProtocol { protected: friend class WeakNSObjectFactory; - explicit WeakNSProtocol(debug::DebugThreadChecker checker, NST object = nil) : checker_(checker) { - container_ = [CRBWeakNSProtocolSentinel containerForObject:object threadChecker:checker]; - } + explicit WeakNSProtocol(RefPtr container, debug::DebugThreadChecker checker) + : container_(container), checker_(checker) {} // Refecounted reference to the container tracking the ObjectiveC object this // class encapsulates. RefPtr container_; - debug::DebugThreadChecker checker_; - void CheckThreadSafety() const { FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); } + + debug::DebugThreadChecker checker_; }; // Free functions @@ -194,8 +198,8 @@ class WeakNSObject : public WeakNSProtocol { private: friend class WeakNSObjectFactory; - explicit WeakNSObject(debug::DebugThreadChecker checker, NST* object = nil) - : WeakNSProtocol(checker, object) {} + explicit WeakNSObject(RefPtr container, debug::DebugThreadChecker checker) + : WeakNSProtocol(container, checker) {} }; // Specialization to make WeakNSObject work. @@ -213,8 +217,8 @@ class WeakNSObject : public WeakNSProtocol { private: friend class WeakNSObjectFactory; - explicit WeakNSObject(debug::DebugThreadChecker checker, id object = nil) - : WeakNSProtocol(checker, object) {} + explicit WeakNSObject(RefPtr container, debug::DebugThreadChecker checker) + : WeakNSProtocol(container, checker) {} }; // Class that produces (valid) |WeakNSObject|s. Typically, this is used as a @@ -244,16 +248,21 @@ class WeakNSObject : public WeakNSProtocol { template class WeakNSObjectFactory { public: - explicit WeakNSObjectFactory(NST* object) : object_(object) { FML_DCHECK(object_); } + explicit WeakNSObjectFactory(NST* object) { + FML_DCHECK(object); + container_ = [CRBWeakNSProtocolSentinel containerForObject:object threadChecker:checker_]; + } ~WeakNSObjectFactory() { CheckThreadSafety(); } // Gets a new weak pointer, which will be valid until this object is - // destroyed. - WeakNSObject GetWeakNSObject() const { return WeakNSObject(checker_, object_); } + // destroyed.e + WeakNSObject GetWeakNSObject() const { return WeakNSObject(container_, checker_); } private: - NST* object_; + // Refecounted reference to the container tracking the ObjectiveC object this + // class encapsulates. + RefPtr container_; void CheckThreadSafety() const { FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); } diff --git a/fml/platform/darwin/weak_nsobject.mm b/fml/platform/darwin/weak_nsobject.mm index 565c7a25a0b3c..e57f25d53c813 100644 --- a/fml/platform/darwin/weak_nsobject.mm +++ b/fml/platform/darwin/weak_nsobject.mm @@ -14,7 +14,7 @@ namespace fml { WeakContainer::WeakContainer(id object, debug::DebugThreadChecker checker) - : checker_(checker), object_(object) {} + : object_(object), checker_(checker) {} WeakContainer::~WeakContainer() {} diff --git a/shell/platform/darwin/ios/BUILD.gn b/shell/platform/darwin/ios/BUILD.gn index 4f23609c3fa28..93bbe44130748 100644 --- a/shell/platform/darwin/ios/BUILD.gn +++ b/shell/platform/darwin/ios/BUILD.gn @@ -236,7 +236,6 @@ source_set("ios_test_flutter_mrc") { sources = [ "framework/Source/FlutterEnginePlatformViewTest.mm", "framework/Source/FlutterEngineTest_mrc.mm", - "framework/Source/FlutterPlatformPluginTest.mm", "framework/Source/FlutterPlatformViewsTest.mm", "framework/Source/FlutterTouchInterceptingView_Test.h", "framework/Source/FlutterViewControllerTest_mrc.mm", @@ -304,6 +303,7 @@ shared_library("ios_test_flutter") { "framework/Source/FlutterFakeKeyEvents.h", "framework/Source/FlutterFakeKeyEvents.mm", "framework/Source/FlutterKeyboardManagerTest.mm", + "framework/Source/FlutterPlatformPluginTest.mm", "framework/Source/FlutterPluginAppLifeCycleDelegateTest.mm", "framework/Source/FlutterRestorationPluginTest.mm", "framework/Source/FlutterSpellCheckPluginTest.mm", diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm index 65226839d365a..894e1f30cd5e1 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm @@ -13,6 +13,8 @@ #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.h" #import "flutter/shell/platform/darwin/ios/platform_view_ios.h" +FLUTTER_ASSERT_ARC + @interface FlutterPlatformPluginTest : XCTestCase @end @@ -34,7 +36,7 @@ - (void)testSearchWebInvokedWithEscapedTerm { id mockApplication = OCMClassMock([UIApplication class]); OCMStub([mockApplication sharedApplication]).andReturn(mockApplication); - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; std::unique_ptr> _weakFactory = std::make_unique>(engine); [engine runWithEntrypoint:nil]; @@ -43,7 +45,7 @@ - (void)testSearchWebInvokedWithEscapedTerm { [self expectationWithDescription:@"Web search launched with escaped search term"]; FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; + [[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"SearchWeb.invoke" @@ -68,7 +70,7 @@ - (void)testSearchWebInvokedWithNonEscapedTerm { id mockApplication = OCMClassMock([UIApplication class]); OCMStub([mockApplication sharedApplication]).andReturn(mockApplication); - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; std::unique_ptr> _weakFactory = std::make_unique>(engine); [engine runWithEntrypoint:nil]; @@ -77,7 +79,7 @@ - (void)testSearchWebInvokedWithNonEscapedTerm { [self expectationWithDescription:@"Web search launched with non escaped search term"]; FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; + [[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"SearchWeb.invoke" @@ -99,7 +101,7 @@ - (void)testSearchWebInvokedWithNonEscapedTerm { } - (void)testLookUpCallInitiated { - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; [engine runWithEntrypoint:nil]; std::unique_ptr> _weakFactory = std::make_unique>(engine); @@ -107,12 +109,13 @@ - (void)testLookUpCallInitiated { XCTestExpectation* presentExpectation = [self expectationWithDescription:@"Look Up view controller presented"]; - FlutterViewController* engineViewController = - [[[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil] autorelease]; + FlutterViewController* engineViewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; FlutterViewController* mockEngineViewController = OCMPartialMock(engineViewController); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; + [[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"LookUp.invoke" @@ -129,7 +132,7 @@ - (void)testLookUpCallInitiated { } - (void)testShareScreenInvoked { - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; [engine runWithEntrypoint:nil]; std::unique_ptr> _weakFactory = std::make_unique>(engine); @@ -137,8 +140,9 @@ - (void)testShareScreenInvoked { XCTestExpectation* presentExpectation = [self expectationWithDescription:@"Share view controller presented"]; - FlutterViewController* engineViewController = - [[[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil] autorelease]; + FlutterViewController* engineViewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; FlutterViewController* mockEngineViewController = OCMPartialMock(engineViewController); OCMStub([mockEngineViewController presentViewController:[OCMArg isKindOfClass:[UIActivityViewController class]] @@ -146,7 +150,7 @@ - (void)testShareScreenInvoked { completion:nil]); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; + [[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"Share.invoke" @@ -164,11 +168,11 @@ - (void)testShareScreenInvoked { - (void)testClipboardHasCorrectStrings { [UIPasteboard generalPasteboard].string = nil; - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; std::unique_ptr> _weakFactory = std::make_unique>(engine); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; + [[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()]; XCTestExpectation* setStringExpectation = [self expectationWithDescription:@"setString"]; FlutterResult resultSet = ^(id result) { @@ -203,11 +207,11 @@ - (void)testClipboardHasCorrectStrings { - (void)testClipboardSetDataToNullDoNotCrash { [UIPasteboard generalPasteboard].string = nil; - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; std::unique_ptr> _weakFactory = std::make_unique>(engine); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; + [[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()]; XCTestExpectation* setStringExpectation = [self expectationWithDescription:@"setData"]; FlutterResult resultSet = ^(id result) { @@ -230,18 +234,18 @@ - (void)testClipboardSetDataToNullDoNotCrash { } - (void)testPopSystemNavigator { - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; [engine runWithEntrypoint:nil]; FlutterViewController* flutterViewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; - UINavigationController* navigationController = [[[UINavigationController alloc] - initWithRootViewController:flutterViewController] autorelease]; - UITabBarController* tabBarController = [[[UITabBarController alloc] init] autorelease]; + UINavigationController* navigationController = + [[UINavigationController alloc] initWithRootViewController:flutterViewController]; + UITabBarController* tabBarController = [[UITabBarController alloc] init]; tabBarController.viewControllers = @[ navigationController ]; std::unique_ptr> _weakFactory = std::make_unique>(engine); FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; + [[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()]; id navigationControllerMock = OCMPartialMock(navigationController); OCMStub([navigationControllerMock popViewControllerAnimated:YES]); @@ -257,17 +261,16 @@ - (void)testPopSystemNavigator { OCMVerify([navigationControllerMock popViewControllerAnimated:YES]); [flutterViewController deregisterNotifications]; - [flutterViewController release]; } - (void)testWhetherDeviceHasLiveTextInputInvokeCorrectly { - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; std::unique_ptr> _weakFactory = std::make_unique>(engine); XCTestExpectation* invokeExpectation = [self expectationWithDescription:@"isLiveTextInputAvailableInvoke"]; FlutterPlatformPlugin* plugin = - [[[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()] autorelease]; + [[FlutterPlatformPlugin alloc] initWithEngine:_weakFactory->GetWeakNSObject()]; FlutterPlatformPlugin* mockPlugin = OCMPartialMock(plugin); FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"LiveText.isLiveTextInputAvailable" @@ -286,7 +289,7 @@ - (void)testViewControllerBasedStatusBarHiddenUpdate { .andReturn(@YES); { // Enabling system UI overlays to update status bar. - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; [engine runWithEntrypoint:nil]; FlutterViewController* flutterViewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; @@ -323,11 +326,10 @@ - (void)testViewControllerBasedStatusBarHiddenUpdate { XCTAssertFalse(flutterViewController.prefersStatusBarHidden); [flutterViewController deregisterNotifications]; - [flutterViewController release]; } { // Enable system UI mode to update status bar. - FlutterEngine* engine = [[[FlutterEngine alloc] initWithName:@"test" project:nil] autorelease]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; [engine runWithEntrypoint:nil]; FlutterViewController* flutterViewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; @@ -364,7 +366,6 @@ - (void)testViewControllerBasedStatusBarHiddenUpdate { XCTAssertFalse(flutterViewController.prefersStatusBarHidden); [flutterViewController deregisterNotifications]; - [flutterViewController release]; } [bundleMock stopMocking]; } From 31712910574d21582177063c2687665819f31da8 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 15 Nov 2023 11:46:13 -0800 Subject: [PATCH 6/8] review --- fml/platform/darwin/weak_nsobject.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/platform/darwin/weak_nsobject.mm b/fml/platform/darwin/weak_nsobject.mm index e57f25d53c813..202756289a675 100644 --- a/fml/platform/darwin/weak_nsobject.mm +++ b/fml/platform/darwin/weak_nsobject.mm @@ -62,7 +62,7 @@ - (id)initWithContainer:(fml::RefPtr)container { } - (void)dealloc { - self.container->nullify(); + _container->nullify(); [super dealloc]; } From 97e54266f417eed19883852e2d080cfe44b492ba Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 15 Nov 2023 13:08:45 -0800 Subject: [PATCH 7/8] remove synthesize and use _container directly --- fml/platform/darwin/weak_nsobject.mm | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fml/platform/darwin/weak_nsobject.mm b/fml/platform/darwin/weak_nsobject.mm index 202756289a675..c8844a5f8a406 100644 --- a/fml/platform/darwin/weak_nsobject.mm +++ b/fml/platform/darwin/weak_nsobject.mm @@ -29,8 +29,6 @@ - (id)initWithContainer:(fml::RefPtr)container; @implementation CRBWeakNSProtocolSentinel -@synthesize container = container_; - + (fml::RefPtr)containerForObject:(id)object threadChecker:(debug::DebugThreadChecker)checker { if (object == nil) { @@ -53,10 +51,10 @@ @implementation CRBWeakNSProtocolSentinel } - (id)initWithContainer:(fml::RefPtr)container { - FML_DCHECK(container.get()); + FML_DCHECK(_container.get()); self = [super init]; if (self) { - container_ = container; + _container = container; } return self; } From 80ffbcddef63f6783bca925d6e5d9d0cd37b473f Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 15 Nov 2023 15:01:08 -0800 Subject: [PATCH 8/8] fix typo --- fml/platform/darwin/weak_nsobject.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/platform/darwin/weak_nsobject.mm b/fml/platform/darwin/weak_nsobject.mm index c8844a5f8a406..4ac709755a58f 100644 --- a/fml/platform/darwin/weak_nsobject.mm +++ b/fml/platform/darwin/weak_nsobject.mm @@ -51,7 +51,7 @@ @implementation CRBWeakNSProtocolSentinel } - (id)initWithContainer:(fml::RefPtr)container { - FML_DCHECK(_container.get()); + FML_DCHECK(container.get()); self = [super init]; if (self) { _container = container;