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 intoflutter:masterfrom
gaaclarke:cleanup-gl-pop-vc
Oct 29, 2019
Merged

Made it so we clean up gl resources when view controllers get deleted.#13396
gaaclarke merged 3 commits intoflutter:masterfrom
gaaclarke:cleanup-gl-pop-vc

Conversation

@gaaclarke
Copy link
Member

Relevant issue: flutter/flutter#38736

@gaaclarke gaaclarke requested review from dnfield and gw280 October 28, 2019 22:19
PlatformViewIOS platform_view = [self iosPlatformView];
if (platform_view) {
fml::WeakPtr<FlutterViewController> null_view_controller;
platform_view->SetOwnerViewController(null_view_controller);
Copy link
Contributor

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

Choose a reason for hiding this comment

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

done

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 added a commit to flutter/flutter that referenced this pull request Oct 30, 2019
[email protected]:flutter/engine.git/compare/36303c61b63f...5051bef

git log 36303c6..5051bef --no-merges --oneline
2019-10-30 [email protected] Roll src/third_party/skia ebdae1144978..7da048b5e8f1 (1 commits) (flutter/engine#13438)
2019-10-30 [email protected] Roll fuchsia/sdk/core/mac-amd64 from DfLN8... to p7XYM... (flutter/engine#13437)
2019-10-30 [email protected] Roll src/third_party/dart eb4ab61349..d3a5b82355 (4 commits)
2019-10-30 [email protected] Roll fuchsia/sdk/core/linux-amd64 from goLxs... to nhX22... (flutter/engine#13435)
2019-10-30 [email protected] Roll src/third_party/skia c3a51a5e47ba..ebdae1144978 (1 commits) (flutter/engine#13434)
2019-10-30 [email protected] Roll src/third_party/skia 1ae54bc22fc2..c3a51a5e47ba (1 commits) (flutter/engine#13433)
2019-10-30 [email protected] Roll src/third_party/dart ae5a86d790..eb4ab61349 (7 commits)
2019-10-30 [email protected] Set the install name at link time for darwin dylibs (flutter/engine#13428)
2019-10-30 [email protected] Add isRunningInRobolectricTest back (flutter/engine#13424)
2019-10-30 [email protected] Roll src/third_party/skia 76d22d1ee8cb..1ae54bc22fc2 (26 commits) (flutter/engine#13430)
2019-10-30 [email protected] Roll src/third_party/dart ebd059030b..ae5a86d790 (3 commits)
2019-10-30 [email protected] Roll fuchsia/clang/linux-amd64 from Vghc_... to WxGHg... (flutter/engine#13420)
2019-10-30 [email protected] Roll fuchsia/sdk/core/mac-amd64 from mc3jR... to DfLN8... (flutter/engine#13413)
2019-10-30 [email protected] Roll src/third_party/dart 780af0fcfc..ebd059030b (17 commits)
2019-10-30 [email protected] Make sure root surface transformations survive resetting the matrix directly in Flow. (flutter/engine#13405)
2019-10-30 [email protected] [fuchsia] [packaging] Create a script to upload debug symbols to CIPD (flutter/engine#13422)
2019-10-29 [email protected] Revert "fix fml_unittes is not run during presubmit (#13395)" (flutter/engine#13425)
2019-10-29 [email protected] Made it so we clean up gl resources when view controllers get deleted. (flutter/engine#13396)
2019-10-29 [email protected] Added back in empty lifecycle events so we don't break people that used to call `super`. (flutter/engine#13421)
2019-10-29 [email protected] fix fml_unittes is not run during presubmit (flutter/engine#13395)
2019-10-29 [email protected] 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 [email protected] 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
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
[email protected]:flutter/engine.git/compare/36303c61b63f...5051bef

git log 36303c6..5051bef --no-merges --oneline
2019-10-30 [email protected] Roll src/third_party/skia ebdae1144978..7da048b5e8f1 (1 commits) (flutter/engine#13438)
2019-10-30 [email protected] Roll fuchsia/sdk/core/mac-amd64 from DfLN8... to p7XYM... (flutter/engine#13437)
2019-10-30 [email protected] Roll src/third_party/dart eb4ab61349..d3a5b82355 (4 commits)
2019-10-30 [email protected] Roll fuchsia/sdk/core/linux-amd64 from goLxs... to nhX22... (flutter/engine#13435)
2019-10-30 [email protected] Roll src/third_party/skia c3a51a5e47ba..ebdae1144978 (1 commits) (flutter/engine#13434)
2019-10-30 [email protected] Roll src/third_party/skia 1ae54bc22fc2..c3a51a5e47ba (1 commits) (flutter/engine#13433)
2019-10-30 [email protected] Roll src/third_party/dart ae5a86d790..eb4ab61349 (7 commits)
2019-10-30 [email protected] Set the install name at link time for darwin dylibs (flutter/engine#13428)
2019-10-30 [email protected] Add isRunningInRobolectricTest back (flutter/engine#13424)
2019-10-30 [email protected] Roll src/third_party/skia 76d22d1ee8cb..1ae54bc22fc2 (26 commits) (flutter/engine#13430)
2019-10-30 [email protected] Roll src/third_party/dart ebd059030b..ae5a86d790 (3 commits)
2019-10-30 [email protected] Roll fuchsia/clang/linux-amd64 from Vghc_... to WxGHg... (flutter/engine#13420)
2019-10-30 [email protected] Roll fuchsia/sdk/core/mac-amd64 from mc3jR... to DfLN8... (flutter/engine#13413)
2019-10-30 [email protected] Roll src/third_party/dart 780af0fcfc..ebd059030b (17 commits)
2019-10-30 [email protected] Make sure root surface transformations survive resetting the matrix directly in Flow. (flutter/engine#13405)
2019-10-30 [email protected] [fuchsia] [packaging] Create a script to upload debug symbols to CIPD (flutter/engine#13422)
2019-10-29 [email protected] Revert "fix fml_unittes is not run during presubmit (flutter#13395)" (flutter/engine#13425)
2019-10-29 [email protected] Made it so we clean up gl resources when view controllers get deleted. (flutter/engine#13396)
2019-10-29 [email protected] Added back in empty lifecycle events so we don't break people that used to call `super`. (flutter/engine#13421)
2019-10-29 [email protected] fix fml_unittes is not run during presubmit (flutter/engine#13395)
2019-10-29 [email protected] 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 [email protected] 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants