Skip to content

[CP-stable]Made the view controller weak for the accessibility bridge. #176030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jtmcdole merged 1 commit into from
Sep 26, 2025
Merged

[CP-stable]Made the view controller weak for the accessibility bridge. #176030

jtmcdole merged 1 commit into from
Sep 26, 2025

Conversation

@justinmc
Copy link
Contributor

@justinmc justinmc commented Sep 25, 2025 •
edited
Loading

This pull request is a manual cherry pick of #172871 after I had trouble updating the automatic cherry pick pull request (#175595).

Issue Link:

What is the link to the issue this cherry-pick is addressing?

#172105

Changelog Description:

Explain this cherry pick in one line that is accessible to most Flutter developers. See best practices for examples

Fixes issue in iOS add-to-app where Flutter view may hang after multiple transitions.

Impact Description:

What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch)

App may hang after transitioning between multiple pages on iOS.

Workaround:

Is there a workaround for this issue?

No

Risk:

What is the risk level of this cherry-pick?

  • Low
  • Medium
  • High

Test Coverage:

Are you confident that your fix is well-tested by automated tests?

  • Yes
  • No

Validation Steps:

What are the steps to validate that this fix works?

Using https://github.com/flutter/samples/tree/main/add_to_app/fullscreen

  • Launch a Flutter screen
  • Dismiss or pop the Flutter screen to return to native
  • Repeat the process several times (open Flutter, close, then open again)
  • Flutter screen should not freeze

@gaaclarke @justinmc
Made the view controller weak for the accessibility bridge. (flutter#…
a7f3d31 
…172871)

fixes: flutter#172105

The migration to ARC accidentally changes the semantics for this ivar.
[It was previously a weak
ptr](https://github.com/flutter/flutter/blob/303823993244ab930458f3767d86255f76e332d8/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h#L91),
this restores that.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
@justinmc justinmc requested review from a team and jtmcdole as code owners September 25, 2025 16:29
@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems platform-android Android applications specifically platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. framework flutter/packages/flutter repository. See also f: labels. engine flutter/engine related. See also e: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) platform-web Web applications specifically platform-linux Building on or for Linux specifically a: desktop Running on desktop f: integration_test The flutter/packages/integration_test plugin e: impeller Impeller rendering backend issues and features requests team-android Owned by Android platform team team-ios Owned by iOS platform team labels Sep 25, 2025
@justinmc justinmc changed the base branch from master to flutter-3.35-candidate.0 September 25, 2025 16:30
@flutter-dashboard
Copy link

flutter-dashboard bot commented Sep 25, 2025

This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter.

Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed.

@justinmc justinmc requested a review from jmagman September 25, 2025 16:31
gemini-code-assist[bot]
gemini-code-assist bot reviewed Sep 25, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a cherry-pick that bundles a large number of fixes. The primary fix, as indicated by the title, addresses a memory leak in the iOS accessibility bridge by using a weak reference to the view controller. This resolves an issue where Flutter views could hang in add-to-app scenarios.

Beyond this, the PR introduces a significant refactoring of the iOS device launching mechanism to support LLDB-based debugging on newer iOS and Xcode versions, with a fallback to the existing Xcode automation. It also includes numerous other improvements and bug fixes across the engine and tooling, such as correcting a buffer flush issue in Impeller, fixing a race condition in the Vulkan backend, improving the web click debouncer for Safari, and enhancing various build and migration scripts for robustness. My review includes a couple of minor suggestions in a C++ test file to improve adherence to the style guide.

reidbaker
reidbaker approved these changes Sep 25, 2025
View reviewed changes
Copy link
Contributor

@reidbaker reidbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

Reminder to format your changelog entry following the best practices https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md

@justinmc justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 25, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Sep 25, 2025

autosubmit label was removed for flutter/flutter/176030, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 25, 2025
@justinmc justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 26, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 26, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Sep 26, 2025

autosubmit label was removed for flutter/flutter/176030, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

@justinmc justinmc mentioned this pull request Sep 26, 2025
Closed
4 tasks
@jtmcdole jtmcdole merged commit d3d45dc into flutter:flutter-3.35-candidate.0 Sep 26, 2025
162 of 163 checks passed
@justinmc justinmc deleted the cp-stable-45d112f1272295ef39984cf1b09035d203479c78 branch September 26, 2025 16:45
This was referenced Sep 26, 2025
Merged
Merged
auto-submit bot pushed a commit that referenced this pull request Sep 26, 2025
@justinmc
Engine version for 3.35.5 (#176105)
d2fcae8 
Sets the engine version to the the most recent engine change (#176030).
auto-submit bot pushed a commit that referenced this pull request Sep 26, 2025
@justinmc
Changelog entry for 3.35.5 (#176103)
ac4e799 
Changelog update for stable hotfix. These are the cherry picks:

#176030
#176048
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 29, 2025
@engine-flutter-autoroll
d3d45dcf2 [CP-stable]Made the view controller weak for the accessibil…
022dfe1 
…ity bridge. (flutter/flutter#176030)
@reidbaker reidbaker mentioned this pull request Oct 1, 2025
Merged
9 tasks
@fabiocdssilva69-prog fabiocdssilva69-prog mentioned this pull request Oct 6, 2025
Closed
9 tasks
@jmagman jmagman mentioned this pull request Oct 17, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmagman jmagman jmagman approved these changes

@reidbaker reidbaker reidbaker approved these changes

@jtmcdole jtmcdole Awaiting requested review from jtmcdole

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: desktop Running on desktop a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels. f: integration_test The flutter/packages/integration_test plugin framework flutter/packages/flutter repository. See also f: labels. platform-android Android applications specifically platform-ios iOS applications specifically platform-linux Building on or for Linux specifically platform-web Web applications specifically team-android Owned by Android platform team team-ios Owned by iOS platform team tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@justinmc @jmagman @reidbaker @jtmcdole @gaaclarke