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

Conversation

@robert-ancell
Copy link
Contributor

Due to changes in the renderer in 6b857de the engine was created once a widget is realized, not when the widget is created. If a Flutter application changed the default my_application.cc template to show the Flutter widget after plugins are run then these plugins would not be able to access the engine.

Solved by removing the GdkWindow from the renderer constructor and setting in later when the widget is realized. This works because the renderer is not used until the widget is realized.

Fixes flutter/flutter#144873

With the changes in 9463b5d we were now taking two weak references using
different methods. A later change showed reference errors in the tests,
which seem to be solved by removing this change. Instead use a private
shutdown method, which is clearer in any case.
This is for consistency with the other weak references. The newer API is
thread safe.
Due to changes in the renderer in 6b857de the engine was created once
a widget is realized, not when the widget is created. If a Flutter application
changed the default my_application.cc template to show the Flutter widget after
plugins are run then these plugins would not be able to access the engine.

Solved by removing the GdkWindow from the renderer constructor and setting in
later when the widget is realized. This works because the renderer is not
used until the widget is realized.

Fixes flutter/flutter#144873
@robert-ancell
Copy link
Contributor Author

Note this change also includes fixes for the weak reference handling which were causing the tests to crash after this change. This change should not be squashed so these commits are separate from the fix.

@robert-ancell
Copy link
Contributor Author

@spydon please confirm if this fixes the issue for you.

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM stamp from a Japanese personal seal

@cbracken
Copy link
Member

Kicked the failing mac bot. Looked like a Metal related flake unrelated to your change.

Copy link

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Lgtm! Tried it on our the installer and the app center and it works with the plugins.

We still have the (most likely unrelated) issue where the start-up screen is black:
Screencast from 2024-07-01 14-56-54.webm
EDIT: The above only happens in debug mode
Opened flutter/flutter#151098 for that issue

@robert-ancell robert-ancell merged commit 40c087b into flutter:main Jul 2, 2024
@robert-ancell robert-ancell deleted the linux-engine-earlier branch July 2, 2024 05:18
@Merrit
Copy link

Merrit commented Jul 19, 2024

I had thought this would be cherry-picked for a hotfix, but it still seems broken today with Flutter 3.22.3 - is the fix only available in main still?

@robert-ancell
Copy link
Contributor Author

@Merrit it is only availale in main. Since the change wasn't trivial I didn't want to propose it for a hotfix without being sure it is working well. I haven't seen any issues since this applied - @spydon have you seen any problems?

@spydon
Copy link

spydon commented Jul 22, 2024

@Merrit it is only availale in main. Since the change wasn't trivial I didn't want to propose it for a hotfix without being sure it is working well. I haven't seen any issues since this applied - @spydon have you seen any problems?

Everything has worked fine while I've tested it!

@robert-ancell robert-ancell added the cp: stable cherry pick to the stable release candidate branch label Jul 23, 2024
@robert-ancell
Copy link
Contributor Author

Great, proposing for a cherry pick.

@flutteractionsbot
Copy link

Failed to create CP due to merge conflicts.
You will need to create the PR manually. See the cherrypick wiki for more info.

@robert-ancell
Copy link
Contributor Author

This is now in 3.24, I won't propose it for 3.22 unless there is a need.

@Merrit
Copy link

Merrit commented Aug 14, 2024

Excellent! Thank you for all your hard work! 🙇‍♀️

I appreciate you 💙 😁

@alexmercerind
Copy link

Thank you very much... it was very important.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

affects: desktop cp: stable cherry pick to the stable release candidate branch platform-linux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3.21.0-0.0.pre encounters asserts on Linux when showing GTK widgets only after plugin registration

6 participants