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

[CanvasKit] Dispose the overlay surface when a platform view is disposed #19546

Conversation

harryterkelsen
Copy link
Contributor

Description

When we disposed of platform views previously, we weren't disposing of the associated SkSurface.

Related Issues

Fixes flutter/flutter#50721

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change.

@fluttergithubbot
Copy link
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@auto-assign auto-assign bot requested a review from gaaclarke July 6, 2020 21:14
@@ -369,6 +370,8 @@ class HtmlViewEmbedder {
if (_overlays[viewId] != null) {
final Overlay overlay = _overlays[viewId]!;
overlay.surface.htmlElement?.remove();
overlay.surface.htmlElement = null;
overlay.skSurface?.dispose();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also null it out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't because skSurface is final. It should be garbage collected anyways after overlay

@harryterkelsen harryterkelsen merged commit a15bc1b into flutter:master Jul 7, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 8, 2020
dnfield pushed a commit to flutter/flutter that referenced this pull request Jul 8, 2020
* a53782f Roll Skia from 6130d5079d55 to 0c0d8dd6d637 (3 revisions) (flutter/engine#19570)

* 0541502 kick build (flutter/engine#19575)

* 07d5090 Roll Skia from 0c0d8dd6d637 to cf5e35f72130 (13 revisions) (flutter/engine#19573)

* 35b5aa5 switch const finder to package_config (flutter/engine#19576)

* de0932b Manual roll of Dart 06cb010247...69aba23371 (flutter/engine#19577)

* b919945 include list_libraries.dart as a snapshot for fuchsia (flutter/engine#19567)

* 3fe5edf Roll Skia from cf5e35f72130 to b4d60f807dbd (5 revisions) (flutter/engine#19587)

* a15bc1b [CanvasKit] Dispose the overlay surface when a platform view is disposed (flutter/engine#19546)

* 0dc86cd Only attempt surface creation in viewDidLayoutSubviews if the application is active. (flutter/engine#19592)
srujzs pushed a commit to srujzs/engine that referenced this pull request Jul 9, 2020
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
* a53782f Roll Skia from 6130d5079d55 to 0c0d8dd6d637 (3 revisions) (flutter/engine#19570)

* 0541502 kick build (flutter/engine#19575)

* 07d5090 Roll Skia from 0c0d8dd6d637 to cf5e35f72130 (13 revisions) (flutter/engine#19573)

* 35b5aa5 switch const finder to package_config (flutter/engine#19576)

* de0932b Manual roll of Dart 06cb010247...69aba23371 (flutter/engine#19577)

* b919945 include list_libraries.dart as a snapshot for fuchsia (flutter/engine#19567)

* 3fe5edf Roll Skia from cf5e35f72130 to b4d60f807dbd (5 revisions) (flutter/engine#19587)

* a15bc1b [CanvasKit] Dispose the overlay surface when a platform view is disposed (flutter/engine#19546)

* 0dc86cd Only attempt surface creation in viewDidLayoutSubviews if the application is active. (flutter/engine#19592)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Web] [CanvasKit] WebGL context is not cleaned up / lost
4 participants