Skip to content
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.

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

Fixes FlutterEngine internal retain cycle #18595

Merged
merged 3 commits into from Jun 17, 2020

Conversation

@zhongwuzw
Copy link
Contributor

@zhongwuzw zhongwuzw commented May 25, 2020

No description provided.

@auto-assign auto-assign bot requested a review from cbracken May 25, 2020
Copy link
Contributor

@gaaclarke gaaclarke left a comment

LGTM, thanks for the contribution!

Copy link
Contributor

@gaaclarke gaaclarke left a comment

Please make the aforementioned performance changes.

}];

auto platformViewsController = _platformViewsController.get();

This comment has been minimized.

@gaaclarke

gaaclarke Jun 16, 2020
Contributor

This one should still use the weakptr trick you were using previously. The reason is that platformViewsController is a C++ object which means it won't get retained when it is captured by the block. In practice this looks like it would be safe today but I think we should err on the side of caution.

This comment has been minimized.

@zhongwuzw

zhongwuzw Jun 17, 2020
Author Contributor

@gaaclarke Done.

Copy link
Contributor

@gaaclarke gaaclarke left a comment

Nice! Thanks for the quick responses =)

@gaaclarke gaaclarke force-pushed the zhongwuzw:fix_engine_leaks branch from 9be9c9e to b511285 Jun 17, 2020
@gaaclarke gaaclarke merged commit d7d9e8b into flutter:master Jun 17, 2020
20 of 21 checks passed
20 of 21 checks passed
@fluttergithubbot
Mac iOS Engine Flutter LUCI Build: Mac iOS Engine
Details
@fluttergithubbot
Linux Android AOT Engine Flutter LUCI Build: Linux Android AOT Engine
Details
@fluttergithubbot
Linux Android Debug Engine Flutter LUCI Build: Linux Android Debug Engine
Details
@fluttergithubbot
Linux Fuchsia Flutter LUCI Build: Linux Fuchsia
Details
@fluttergithubbot
Linux Host Engine Flutter LUCI Build: Linux Host Engine
Details
@fluttergithubbot
Linux Web Engine Flutter LUCI Build: Linux Web Engine
Details
@fluttergithubbot
Mac Android AOT Engine Flutter LUCI Build: Mac Android AOT Engine
Details
@fluttergithubbot
Mac Android Debug Engine Flutter LUCI Build: Mac Android Debug Engine
Details
@fluttergithubbot
Mac Host Engine Flutter LUCI Build: Mac Host Engine
Details
@fluttergithubbot
Mac Web Engine Flutter LUCI Build: Mac Web Engine
Details
@wip
WIP Ready for review
Details
@fluttergithubbot
Windows Android AOT Engine Flutter LUCI Build: Windows Android AOT Engine
Details
@fluttergithubbot
Windows Host Engine Flutter LUCI Build: Windows Host Engine
Details
@fluttergithubbot
Windows Web Engine Flutter LUCI Build: Windows Web Engine
Details
@cirrus-ci
build_and_test_linux_release Task Summary
Details
@cirrus-ci
build_and_test_linux_unopt_debug Task Summary
Details
@googlebot
cla/google All necessary CLAs are signed
@cirrus-ci
format_and_dart_test Task Summary
Details
@cirrus-ci
lint_test Task Summary
Details
@fluttergithubbot
luci-engine
Details
@cirrus-ci
web_engine_analysis Task Summary
Details
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants