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

Commit cf3473b

Browse files
author
Chris Yang
authored
[ios] making objective-C smart pointers support ARC (#47612)
Moving the implementation from https://codereview.chromium.org/1855483004 into the code base, including: - scoped_nsobject, scoped_nsprotocol, scoped_block will support both mrc and arc - Added parent class scoped_typeref for shared code between scoped_block and scoped_nsobject - moving OwnershipPolicy to its own file The implementation of the smart pointers are almost identical to https://codereview.chromium.org/1855483004 besides some syntax preference differences between chromium and flutter. This PR also migrated [VsyncWaiterIosTest.mm](https://github.com/flutter/engine/pull/47612/files#diff-c98ce1a2aca65c29bbc444523b66921a53ecce5ff39a420b4eda7dbfe8ca1cc7) to ARC with scoped_nsobject fixes flutter/flutter#137802 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 64d5af2 commit cf3473b

18 files changed

+638
-145
lines changed

BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ group("unittests") {
159159
"//flutter/display_list:display_list_rendertests",
160160
"//flutter/display_list:display_list_unittests",
161161
"//flutter/flow:flow_unittests",
162+
"//flutter/fml:fml_arc_unittests",
162163
"//flutter/fml:fml_unittests",
163164
"//flutter/lib/ui:ui_unittests",
164165
"//flutter/runtime:dart_plugin_registrant_unittests",

ci/licenses_golden/excluded_files

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@
101101
../../../flutter/fml/message_loop_unittests.cc
102102
../../../flutter/fml/paths_unittests.cc
103103
../../../flutter/fml/platform/darwin/cf_utils_unittests.mm
104+
../../../flutter/fml/platform/darwin/scoped_nsobject_arc_unittests.mm
105+
../../../flutter/fml/platform/darwin/scoped_nsobject_unittests.mm
104106
../../../flutter/fml/platform/darwin/string_range_sanitization_unittests.mm
105107
../../../flutter/fml/platform/win/file_win_unittests.cc
106108
../../../flutter/fml/platform/win/wstring_conversion_unittests.cc

ci/licenses_golden/licenses_flutter

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2795,6 +2795,8 @@ ORIGIN: ../../../flutter/fml/platform/darwin/scoped_nsautorelease_pool.cc + ../.
27952795
ORIGIN: ../../../flutter/fml/platform/darwin/scoped_nsautorelease_pool.h + ../../../flutter/LICENSE
27962796
ORIGIN: ../../../flutter/fml/platform/darwin/scoped_nsobject.h + ../../../flutter/LICENSE
27972797
ORIGIN: ../../../flutter/fml/platform/darwin/scoped_nsobject.mm + ../../../flutter/LICENSE
2798+
ORIGIN: ../../../flutter/fml/platform/darwin/scoped_policy.h + ../../../flutter/LICENSE
2799+
ORIGIN: ../../../flutter/fml/platform/darwin/scoped_typeref.h + ../../../flutter/LICENSE
27982800
ORIGIN: ../../../flutter/fml/platform/darwin/string_range_sanitization.h + ../../../flutter/LICENSE
27992801
ORIGIN: ../../../flutter/fml/platform/darwin/string_range_sanitization.mm + ../../../flutter/LICENSE
28002802
ORIGIN: ../../../flutter/fml/platform/fuchsia/message_loop_fuchsia.cc + ../../../flutter/LICENSE
@@ -5549,6 +5551,8 @@ FILE: ../../../flutter/fml/platform/darwin/scoped_nsautorelease_pool.cc
55495551
FILE: ../../../flutter/fml/platform/darwin/scoped_nsautorelease_pool.h
55505552
FILE: ../../../flutter/fml/platform/darwin/scoped_nsobject.h
55515553
FILE: ../../../flutter/fml/platform/darwin/scoped_nsobject.mm
5554+
FILE: ../../../flutter/fml/platform/darwin/scoped_policy.h
5555+
FILE: ../../../flutter/fml/platform/darwin/scoped_typeref.h
55525556
FILE: ../../../flutter/fml/platform/darwin/string_range_sanitization.h
55535557
FILE: ../../../flutter/fml/platform/darwin/string_range_sanitization.mm
55545558
FILE: ../../../flutter/fml/platform/fuchsia/message_loop_fuchsia.cc

fml/BUILD.gn

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ source_set("fml") {
165165
"platform/darwin/scoped_nsautorelease_pool.h",
166166
"platform/darwin/scoped_nsobject.h",
167167
"platform/darwin/scoped_nsobject.mm",
168+
"platform/darwin/scoped_policy.h",
169+
"platform/darwin/scoped_typeref.h",
168170
"platform/darwin/string_range_sanitization.h",
169171
"platform/darwin/string_range_sanitization.mm",
170172
]
@@ -366,6 +368,10 @@ if (enable_unittests) {
366368
]
367369
}
368370

371+
if (is_mac || is_ios) {
372+
sources += [ "platform/darwin/scoped_nsobject_unittests.mm" ]
373+
}
374+
369375
if (is_win) {
370376
sources += [
371377
"platform/win/file_win_unittests.cc",
@@ -388,4 +394,18 @@ if (enable_unittests) {
388394
[ "${fuchsia_sdk_path}/arch/${target_cpu}/sysroot/lib/libzircon.so" ]
389395
}
390396
}
397+
398+
executable("fml_arc_unittests") {
399+
testonly = true
400+
if (is_mac || is_ios) {
401+
cflags_objcc = flutter_cflags_objc_arc
402+
sources = [ "platform/darwin/scoped_nsobject_arc_unittests.mm" ]
403+
}
404+
405+
deps = [
406+
":fml_fixtures",
407+
"//flutter/fml",
408+
"//flutter/testing",
409+
]
410+
}
391411
}

fml/platform/darwin/scoped_block.h

Lines changed: 23 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -8,86 +8,40 @@
88
#include <Block.h>
99

1010
#include "flutter/fml/compiler_specific.h"
11+
#include "flutter/fml/platform/darwin/scoped_typeref.h"
1112

12-
namespace fml {
13-
14-
// ScopedBlock<> is patterned after ScopedCFTypeRef<>, but uses Block_copy() and
15-
// Block_release() instead of CFRetain() and CFRelease().
13+
#if defined(__has_feature) && __has_feature(objc_arc)
14+
#define BASE_MAC_BRIDGE_CAST(TYPE, VALUE) (__bridge TYPE)(VALUE)
15+
#else
16+
#define BASE_MAC_BRIDGE_CAST(TYPE, VALUE) VALUE
17+
#endif
1618

17-
enum class OwnershipPolicy {
18-
// The scoped object takes ownership of an object by taking over an existing
19-
// ownership claim.
20-
kAssume,
19+
namespace fml {
2120

22-
// The scoped object will retain the object and any initial ownership is
23-
// not changed.
24-
kRetain,
25-
};
21+
namespace internal {
2622

2723
template <typename B>
28-
class ScopedBlock {
29-
public:
30-
explicit ScopedBlock(B block = nullptr,
31-
OwnershipPolicy policy = OwnershipPolicy::kAssume)
32-
: block_(block) {
33-
if (block_ && policy == OwnershipPolicy::kRetain) {
34-
block_ = Block_copy(block);
35-
}
24+
struct ScopedBlockTraits {
25+
static B InvalidValue() { return nullptr; }
26+
static B Retain(B block) {
27+
return BASE_MAC_BRIDGE_CAST(
28+
B, Block_copy(BASE_MAC_BRIDGE_CAST(const void*, block)));
3629
}
37-
38-
ScopedBlock(const ScopedBlock<B>& that) : block_(that.block_) {
39-
if (block_) {
40-
block_ = Block_copy(block_);
41-
}
42-
}
43-
44-
~ScopedBlock() {
45-
if (block_) {
46-
Block_release(block_);
47-
}
48-
}
49-
50-
ScopedBlock& operator=(const ScopedBlock<B>& that) {
51-
reset(that.get(), OwnershipPolicy::kRetain);
52-
return *this;
53-
}
54-
55-
void reset(B block = nullptr,
56-
OwnershipPolicy policy = OwnershipPolicy::kAssume) {
57-
if (block && policy == OwnershipPolicy::kRetain) {
58-
block = Block_copy(block);
59-
}
60-
if (block_) {
61-
Block_release(block_);
62-
}
63-
block_ = block;
30+
static void Release(B block) {
31+
Block_release(BASE_MAC_BRIDGE_CAST(const void*, block));
6432
}
33+
};
6534

66-
bool operator==(B that) const { return block_ == that; }
67-
68-
bool operator!=(B that) const { return block_ != that; }
69-
70-
// NOLINTNEXTLINE(google-explicit-constructor)
71-
operator B() const { return block_; }
72-
73-
B get() const { return block_; }
74-
75-
void swap(ScopedBlock& that) {
76-
B temp = that.block_;
77-
that.block_ = block_;
78-
block_ = temp;
79-
}
35+
} // namespace internal
8036

81-
[[nodiscard]] B release() {
82-
B temp = block_;
83-
block_ = nullptr;
84-
return temp;
85-
}
37+
// ScopedBlock<> is patterned after ScopedCFTypeRef<>, but uses Block_copy() and
38+
// Block_release() instead of CFRetain() and CFRelease().
8639

87-
private:
88-
B block_;
89-
};
40+
template <typename B>
41+
using ScopedBlock = ScopedTypeRef<B, internal::ScopedBlockTraits<B>>;
9042

9143
} // namespace fml
9244

45+
#undef BASE_MAC_BRIDGE_CAST
46+
9347
#endif // FLUTTER_FML_PLATFORM_DARWIN_SCOPED_BLOCK_H_

0 commit comments

Comments
 (0)