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

Conversation

@stuartmorgan-g
Copy link
Contributor

This makes two changes:

  • Adds a way to register a callback for when a FlutterDesktopPluginRegistrarRef is destroyed, and implements the logic to call it in the Windows and Linux embeddings.
  • Adds a class to the C++ wrapper that handles making a singleton owning PluginRegistrar wrappers, and destroying them when the underlying reference goes away, to avoid needing that boilerplate code in every plugin's source.

Fixes flutter/flutter#53496

@auto-assign auto-assign bot requested a review from gw280 April 2, 2020 21:57
@stuartmorgan-g stuartmorgan-g merged commit faf44fe into flutter:master Apr 6, 2020
@stuartmorgan-g stuartmorgan-g deleted the cpp-plugin-registrar-destruction-callback branch April 6, 2020 16:55
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 6, 2020
tvolkert pushed a commit to flutter/flutter that referenced this pull request Apr 6, 2020
* 0945635 Roll src/third_party/skia cc8a76f3c763..04513752fd6e (22 commits) (flutter/engine#17529)

* faf44fe Improve C++ plugin lifetime handling (flutter/engine#17489)

* 4b69cf2 [tools][fuchsia] Do not tar debug symbol CIPD uploads (flutter/engine#17506)

* 19a7fbf Fixed nullability in plugin header and overridden type mismatch error. (flutter/engine#17532)

* ab434c5 Revert "[tools][fuchsia] Do not tar debug symbol CIPD uploads (#17506)" (flutter/engine#17537)
@cbracken
Copy link
Member

cbracken commented Apr 7, 2020

@stuartmorgan The windows bot appears to be failing flakily ever since this landed.

[ RUN      ] PluginRegistrarTest.ManagerRemovesOnDestruction
c:\b\s\w\ir\cache\builder\src\flutter\shell\platform\common\cpp\client_wrapper\plugin_registrar_unittests.cc(149): error: Expected: (manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle)) != (first_wrapper), actual: 000002400A90E3D0 vs 000002400A90E3D0

@cbracken
Copy link
Member

cbracken commented Apr 7, 2020

Revert is out in #17563.

cbracken added a commit that referenced this pull request Apr 7, 2020
Seems to have triggered flaky failures on the Windows bot since landing.
Example failure:

    [ RUN      ] PluginRegistrarTest.ManagerRemovesOnDestruction
    c:\b\s\w\ir\cache\builder\src\flutter\shell\platform\common\cpp\client_wrapper\plugin_registrar_unittests.cc(149): error: Expected: (manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle)) != (first_wrapper), actual: 000002400A90E3D0 vs 000002400A90E3D0

This reverts commit faf44fe.
stuartmorgan-g added a commit to stuartmorgan-g/engine that referenced this pull request Apr 7, 2020
Relands flutter#17489 with a fix for the unit test flake.
stuartmorgan-g added a commit that referenced this pull request Apr 7, 2020
Relands #17489 with a fix for the unit test flake.

The previous unit test relied on the new instance not being created at the same memory address, which isn't guaranteed.
goderbauer pushed a commit to goderbauer/engine that referenced this pull request Apr 16, 2020
This makes two changes:
- Adds a way to register a callback for when a FlutterDesktopPluginRegistrarRef is destroyed, and implements the logic to call it in the Windows and Linux embeddings.
- Adds a class to the C++ wrapper that handles making a singleton owning PluginRegistrar wrappers, and destroying them when the underlying reference goes away, to avoid needing that boilerplate code in every plugin's source.

Fixes flutter/flutter#53496
goderbauer pushed a commit to goderbauer/engine that referenced this pull request Apr 16, 2020
…r#17563)

Seems to have triggered flaky failures on the Windows bot since landing.
Example failure:

    [ RUN      ] PluginRegistrarTest.ManagerRemovesOnDestruction
    c:\b\s\w\ir\cache\builder\src\flutter\shell\platform\common\cpp\client_wrapper\plugin_registrar_unittests.cc(149): error: Expected: (manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle)) != (first_wrapper), actual: 000002400A90E3D0 vs 000002400A90E3D0

This reverts commit faf44fe.
goderbauer pushed a commit to goderbauer/engine that referenced this pull request Apr 16, 2020
Relands flutter#17489 with a fix for the unit test flake.

The previous unit test relied on the new instance not being created at the same memory address, which isn't guaranteed.
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.

C++ plugin registrar should provide a teardown hook

4 participants