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

Revert "[Impeller] Enable on-by-default on Android." #54321

Closed
wants to merge 1 commit into from

Conversation

reidbaker
Copy link
Contributor

Reverts #54156

Potential fix forward #54296
Pull request to stop testing the crash path flutter/packages#7294
Discord thread. https://discord.com/channels/608014603317936148/1268607913971417240

@chinmaygarde
Copy link
Member

Per the thread, we are also working on unblocking the roller directly.

@reidbaker
Copy link
Contributor Author

Holding this pr until the discord thread makes a decision.

@chinmaygarde
Copy link
Member

xref flutter/packages#7294

@chinmaygarde
Copy link
Member

Stepping away to grab a bite, but the plan as or right now is to to unblock the autoroller by disabling Impeller in the test in flutter/packages#7294, fixing the actual issue in #54296 to follow the iOS style platform view deletion scheme. This is a fallback in case there are still blockers.

@chinmaygarde
Copy link
Member

I don't think we need this anymore. Please close it when convenient.

@reidbaker
Copy link
Contributor Author

@chinmaygarde we have unblocked the roller by modifying test code to not trigger the crashing path which still means there is a real issue. I am comfortable closing this revert IF we can land the fix this week (lets say by thursday) if we cant, either because the fix is too complicated or not important enough then I would lean towards landing this pr until the fix for the race condition lands.

How does that sound to you?

@chinmaygarde
Copy link
Member

chinmaygarde commented Aug 5, 2024

Jonah is OOO this week so I can't promise a timeline to land the proposed fix. Though, I can take over getting it across the finish line if needed.

Rest sounds good.

I figured I should also spend some understanding why the race is triggered more often in Impeller.

@reidbaker
Copy link
Contributor Author

Ok sounds good, lets sync back up on thursday and see if the revert should land.

@reidbaker reidbaker closed this Aug 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants