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

Made it so we clean up gl resources when view controllers get deleted. #13396

Merged
gaaclarke merged 3 commits into from
Oct 29, 2019
Merged

Made it so we clean up gl resources when view controllers get deleted. #13396

gaaclarke merged 3 commits into from
Oct 29, 2019

Conversation

@gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Oct 28, 2019

Relevant issue: flutter/flutter#38736

@gaaclarke gaaclarke requested review from dnfield and gw280 October 28, 2019 22:19
dnfield
dnfield reviewed Oct 28, 2019
View reviewed changes
shell/platform/darwin/ios/framework/Source/FlutterEngine.mm Outdated
PlatformViewIOS platform_view = [self iosPlatformView];
if (platform_view) {
fml::WeakPtr<FlutterViewController> null_view_controller;
platform_view->SetOwnerViewController(null_view_controller);
Copy link
Contributor

@dnfield dnfield Oct 28, 2019

Choose a reason for hiding this comment

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

Can this just be platform_view->SetOwnerViewController({});?

Copy link
Member Author

@gaaclarke gaaclarke Oct 29, 2019

Choose a reason for hiding this comment

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

done

dnfield
dnfield approved these changes Oct 28, 2019
View reviewed changes
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM.

@xster
Copy link
Member

xster commented Oct 29, 2019

Oh, pretty funny that the PR got so much smaller :D

Can we add some tests for this?

@dnfield
Copy link
Contributor

dnfield commented Oct 29, 2019

There's still value in doing the refactor but for now this should fix the bug.

Testing this is really hard right now, see discussion on previous pr

@xster
Copy link
Member

xster commented Oct 29, 2019

LGTM

@gaaclarke gaaclarke added the CQ+1 label Oct 29, 2019
@gaaclarke gaaclarke merged commit f8a7ce5 into flutter:master Oct 29, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 30, 2019
@engine-flutter-autoroll
f8a7ce5 Made it so we clean up gl resources when view controllers get…
81d80d8 
… deleted. (flutter/engine#13396)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 30, 2019
Merged
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Oct 30, 2019
@engine-flutter-autoroll
Roll engine 36303c6..5051bef (21 commits) (#43828)
e0094ee 
git@github.com:flutter/engine.git/compare/36303c61b63f...5051bef

git log 36303c6..5051bef --no-merges --oneline
2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia ebdae1144978..7da048b5e8f1 (1 commits) (flutter/engine#13438)
2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from DfLN8... to p7XYM... (flutter/engine#13437)
2019-10-30 bkonyi@google.com Roll src/third_party/dart eb4ab61349..d3a5b82355 (4 commits)
2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from goLxs... to nhX22... (flutter/engine#13435)
2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia c3a51a5e47ba..ebdae1144978 (1 commits) (flutter/engine#13434)
2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 1ae54bc22fc2..c3a51a5e47ba (1 commits) (flutter/engine#13433)
2019-10-30 bkonyi@google.com Roll src/third_party/dart ae5a86d790..eb4ab61349 (7 commits)
2019-10-30 dnfield@google.com Set the install name at link time for darwin dylibs (flutter/engine#13428)
2019-10-30 mehmetf@users.noreply.github.com Add isRunningInRobolectricTest back (flutter/engine#13424)
2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 76d22d1ee8cb..1ae54bc22fc2 (26 commits) (flutter/engine#13430)
2019-10-30 bkonyi@google.com Roll src/third_party/dart ebd059030b..ae5a86d790 (3 commits)
2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/clang/linux-amd64 from Vghc_... to WxGHg... (flutter/engine#13420)
2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from mc3jR... to DfLN8... (flutter/engine#13413)
2019-10-30 bkonyi@google.com Roll src/third_party/dart 780af0fcfc..ebd059030b (17 commits)
2019-10-30 chinmaygarde@google.com Make sure root surface transformations survive resetting the matrix directly in Flow. (flutter/engine#13405)
2019-10-30 iska.kaushik@gmail.com [fuchsia] [packaging] Create a script to upload debug symbols to CIPD (flutter/engine#13422)
2019-10-29 47866232+chunhtai@users.noreply.github.com Revert "fix fml_unittes is not run during presubmit (#13395)" (flutter/engine#13425)
2019-10-29 30870216+gaaclarke@users.noreply.github.com Made it so we clean up gl resources when view controllers get deleted. (flutter/engine#13396)
2019-10-29 30870216+gaaclarke@users.noreply.github.com Added back in empty lifecycle events so we don't break people that used to call `super`. (flutter/engine#13421)
2019-10-29 47866232+chunhtai@users.noreply.github.com fix fml_unittes is not run during presubmit (flutter/engine#13395)
2019-10-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia f22dda960136..76d22d1ee8cb (2 commits) (flutter/engine#13411)


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 cbracken@google.com on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
@gaaclarke gaaclarke mentioned this pull request Nov 26, 2019
Closed
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
@engine-flutter-autoroll @Inconnu08
Roll engine 36303c6..5051bef (21 commits) (flutter#43828)
0f63f1c 
git@github.com:flutter/engine.git/compare/36303c61b63f...5051bef

git log 36303c6..5051bef --no-merges --oneline
2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia ebdae1144978..7da048b5e8f1 (1 commits) (flutter/engine#13438)
2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from DfLN8... to p7XYM... (flutter/engine#13437)
2019-10-30 bkonyi@google.com Roll src/third_party/dart eb4ab61349..d3a5b82355 (4 commits)
2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from goLxs... to nhX22... (flutter/engine#13435)
2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia c3a51a5e47ba..ebdae1144978 (1 commits) (flutter/engine#13434)
2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 1ae54bc22fc2..c3a51a5e47ba (1 commits) (flutter/engine#13433)
2019-10-30 bkonyi@google.com Roll src/third_party/dart ae5a86d790..eb4ab61349 (7 commits)
2019-10-30 dnfield@google.com Set the install name at link time for darwin dylibs (flutter/engine#13428)
2019-10-30 mehmetf@users.noreply.github.com Add isRunningInRobolectricTest back (flutter/engine#13424)
2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 76d22d1ee8cb..1ae54bc22fc2 (26 commits) (flutter/engine#13430)
2019-10-30 bkonyi@google.com Roll src/third_party/dart ebd059030b..ae5a86d790 (3 commits)
2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/clang/linux-amd64 from Vghc_... to WxGHg... (flutter/engine#13420)
2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from mc3jR... to DfLN8... (flutter/engine#13413)
2019-10-30 bkonyi@google.com Roll src/third_party/dart 780af0fcfc..ebd059030b (17 commits)
2019-10-30 chinmaygarde@google.com Make sure root surface transformations survive resetting the matrix directly in Flow. (flutter/engine#13405)
2019-10-30 iska.kaushik@gmail.com [fuchsia] [packaging] Create a script to upload debug symbols to CIPD (flutter/engine#13422)
2019-10-29 47866232+chunhtai@users.noreply.github.com Revert "fix fml_unittes is not run during presubmit (flutter#13395)" (flutter/engine#13425)
2019-10-29 30870216+gaaclarke@users.noreply.github.com Made it so we clean up gl resources when view controllers get deleted. (flutter/engine#13396)
2019-10-29 30870216+gaaclarke@users.noreply.github.com Added back in empty lifecycle events so we don't break people that used to call `super`. (flutter/engine#13421)
2019-10-29 47866232+chunhtai@users.noreply.github.com fix fml_unittes is not run during presubmit (flutter/engine#13395)
2019-10-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia f22dda960136..76d22d1ee8cb (2 commits) (flutter/engine#13411)


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 cbracken@google.com on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
@gaaclarke gaaclarke mentioned this pull request Jan 23, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@gw280 gw280 gw280 approved these changes

@dnfield dnfield dnfield approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@gaaclarke @xster @dnfield @gw280 @googlebot