iOS: Eliminate ScopedBlock, ScopedTypeRef #56410

cbracken · 2024-11-06T20:18:49Z

Eliminates the two remaining uses of ScopedBlock and with it, ScopedTypeRef which was only used by ScopedBlock. ARC automatically generates the necessary block copy/retain/release calls, including moving blocks to the heap during a retain so manual _Block_copy/_Block_release calls are no longer required.

No test changes since no semantic changes.

Issue: flutter/flutter#137801

Pre-launch Checklist

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

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

flutter-dashboard · 2024-11-06T20:18:52Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

cbracken · 2024-11-06T21:02:41Z

shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h

  ~PlatformMessageResponseDarwin() override;

-  fml::ScopedBlock<PlatformMessageResponseCallback> callback_;
+  PlatformMessageResponseCallback callback_;


For reference, the typedef is here:

engine/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h

Line 17 in eec1ba4

typedef void (^PlatformMessageResponseCallback)(NSData*);

cbracken · 2024-11-06T21:03:38Z

shell/platform/darwin/ios/platform_message_handler_ios.h

  struct HandlerInfo {
    NSObject<FlutterTaskQueueDispatch>* task_queue;
-    fml::ScopedBlock<FlutterBinaryMessageHandler> handler;
+    FlutterBinaryMessageHandler handler;


Typedef is here:

engine/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h

Line 30 in eec1ba4

typedef void (^FlutterBinaryMessageHandler)(NSData* _Nullable message, FlutterBinaryReply reply);

stuartmorgan-g

LGTM

stuartmorgan-g · 2024-11-06T21:08:19Z

test-exempt: code refactor with no semantic change

auto-submit · 2024-11-06T21:23:24Z

auto label is removed for flutter/engine/56410, due to - The status or check suite Linux local_engine_builds has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2024-11-06T23:18:16Z

auto label is removed for flutter/engine/56410, due to - The status or check suite Mac mac_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

The status or check suite Mac mac_clang_tidy has failed. Please fix the issues identified (or deflake) before re-applying this label.

Eliminates the two remaining uses of ScopedBlock and with it, ScopedTypeRef which were only used by ScopedBlock. ARC automatically generates the necessary block copy/retain/release calls, including moving blocks to the heap during a retain so manual _Block_copy/_Block_release calls are no longer required. Though now unused, ScopedPolicy remains since I intend to use it in the CFRef constructor and Reset methods. Issue: flutter/flutter#137801

…utter/engine#56410)

…158354) flutter/engine@bcb281c...8e19915 2024-11-07 skia-flutter-autoroll@skia.org Roll Skia from e9a7546ef3d9 to 2ed9606702f1 (1 revision) (flutter/engine#56434) 2024-11-07 jason-simmons@users.noreply.github.com [Impeller] Use Matrix::MakeTranslateScale to compute the source-to-destination mapping in DrawImageRect (flutter/engine#56433) 2024-11-07 chris@bracken.jp iOS: Eliminate ScopedBlock, ScopedTypeRef, ScopedPolicy (flutter/engine#56410) 2024-11-07 jacksongardner@google.com [skwasm] Make sure to include the transfer list when using postMessage. (flutter/engine#56431) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jsimmons@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

The last usage of this was in fml::ScopedBlock which was deleted in #56410. No test changes since this just deletes dead code. Issue: flutter/flutter#137801 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

…ne#56410) Eliminates the two remaining uses of ScopedBlock and with it, ScopedTypeRef which was only used by ScopedBlock. ARC automatically generates the necessary block copy/retain/release calls, including moving blocks to the heap during a retain so manual _Block_copy/_Block_release calls are no longer required. No test changes since no semantic changes. Issue: flutter#137801 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

The last usage of this was in fml::ScopedBlock which was deleted in flutter/engine#56410. No test changes since this just deletes dead code. Issue: flutter#137801 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

cbracken requested review from hellohuanlin, jmagman and stuartmorgan-g November 6, 2024 20:18

github-actions bot added the platform-ios label Nov 6, 2024

cbracken commented Nov 6, 2024

View reviewed changes

stuartmorgan-g approved these changes Nov 6, 2024

View reviewed changes

cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2024

hellohuanlin approved these changes Nov 6, 2024

View reviewed changes

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2024

cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2024

cbracken changed the title ~~iOS: Eliminate ScopedBlock, ScopedTypeRef~~ iOS: Eliminate ScopedBlock, ScopedTypeRef, ScopedPolicy Nov 6, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2024

cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 7, 2024

auto-submit bot merged commit d132ada into flutter:main Nov 7, 2024
32 checks passed

engine-flutter-autoroll mentioned this pull request Nov 7, 2024

Roll Flutter Engine from 371c86fb6b49 to d132adad9869 (3 revisions) flutter/flutter#158344

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 7, 2024

d132adad9 iOS: Eliminate ScopedBlock, ScopedTypeRef, ScopedPolicy (fl…

39007bd

…utter/engine#56410)

cbracken deleted the bye-bye-scoped-block branch November 7, 2024 21:35

engine-flutter-autoroll mentioned this pull request Nov 7, 2024

Roll Flutter Engine from bcb281cde579 to d132adad9869 (2 revisions) flutter/flutter#158351

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 7, 2024

d132adad9 iOS: Eliminate ScopedBlock, ScopedTypeRef, ScopedPolicy (fl…

a13ec56

…utter/engine#56410)

engine-flutter-autoroll mentioned this pull request Nov 7, 2024

Roll Flutter Engine from bcb281cde579 to 8e19915c19fc (4 revisions) flutter/flutter#158354

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 7, 2024

d132adad9 iOS: Eliminate ScopedBlock, ScopedTypeRef, ScopedPolicy (fl…

c7dec89

…utter/engine#56410)

cbracken mentioned this pull request Nov 8, 2024

fml: delete unused scoped_policy #56462

Merged

8 tasks

cbracken changed the title ~~iOS: Eliminate ScopedBlock, ScopedTypeRef, ScopedPolicy~~ iOS: Eliminate ScopedBlock, ScopedTypeRef Nov 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

iOS: Eliminate ScopedBlock, ScopedTypeRef #56410

iOS: Eliminate ScopedBlock, ScopedTypeRef #56410

Uh oh!

cbracken commented Nov 6, 2024

Uh oh!

flutter-dashboard bot commented Nov 6, 2024

Uh oh!

cbracken Nov 6, 2024 •

edited

Loading

Uh oh!

cbracken Nov 6, 2024

Uh oh!

stuartmorgan-g left a comment

Uh oh!

stuartmorgan-g commented Nov 6, 2024

Uh oh!

auto-submit bot commented Nov 6, 2024

Uh oh!

auto-submit bot commented Nov 6, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

iOS: Eliminate ScopedBlock, ScopedTypeRef #56410

iOS: Eliminate ScopedBlock, ScopedTypeRef #56410

Uh oh!

Conversation

cbracken commented Nov 6, 2024

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Nov 6, 2024

Uh oh!

cbracken Nov 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbracken Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Nov 6, 2024

Uh oh!

auto-submit bot commented Nov 6, 2024

Uh oh!

auto-submit bot commented Nov 6, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cbracken Nov 6, 2024 •

edited

Loading