Fixed issue where the gpu disable syncswitch was being overridden after init #24958
Conversation
|
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. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Added description to compile engine with backport of pull request flutter/engine#24958
|
@chinmaygarde Should we revert #24503 then merge it with this PR to make it easier to cherry-pick? #24503 is a couple weeks old now, i'm not sure if it cleanly will peel off. |
|
We tested the fix brought with this PR on a release as we experience great problems through flutter/flutter#76068. It seems to have fixed the original issue, leading to different crashes that may or may not be related to the same root cause... Please see flutter/flutter#76068 (comment) on the issue. |
|
Making this easy to cherry pick is fine. I was going to insist on a test but I am drawing a blank about how to test this using any existing test harnesses. |
|
Yea, you'd have to do something like an integration test that swizzles UIApplication to return the background state or something. I know it's annoying to cherry-pickers but it might create unnecessary overhead and maybe more confusion if I revert. I'll just land it and babysit it with the cherrypicks. |
* b2d02f7 Roll Fuchsia Linux SDK from GsRYVri0-... to aRqEDMjwC... (flutter/engine#24990) * 3a1a3e6 Roll Skia from 7854da39b3c1 to 4fb13e026b6b (20 revisions) (flutter/engine#24993) * e3a84f9 Fixed issue where the gpu disable syncswitch was being overridden after init. (flutter/engine#24958) * aa83691 fix selectable text selections are not announced in voice over (flutter/engine#24933) * 04b0451 fixes reference retaining issue in flutter text input plugin (flutter/engine#24768) * 1ea7dc6 Set automatic simulator rotation in scenario test (flutter/engine#24985) * 38977a5 Implement AXPlatformNodeBase::GetInstanceCountForTesting (flutter/engine#24999) * a25b0de Roll Fuchsia Mac SDK from xOxFrRRO6... to pmsuWkRQA... (flutter/engine#24997) * 3270c87 Roll Skia from 4fb13e026b6b to 1aa25c3217b6 (25 revisions) (flutter/engine#25001) * 96d5104 [deferred components] Handle base module loading units (flutter/engine#24983) * 6a3d8c7 Roll Skia from 1aa25c3217b6 to ead52dc068d5 (1 revision) (flutter/engine#25005) * 0f52360 Hardware Keyboard: macOS (flutter/engine#23469)
Based on flutter engine with commit: 40441de - contains backport of flutter#24958 and flutter#24503
| _publisher.reset([[FlutterObservatoryPublisher alloc] | ||
| initWithEnableObservatoryPublication:doesObservatoryPublication]); | ||
| [self maybeSetupPlatformViewChannels]; | ||
| _shell->GetIsGpuDisabledSyncSwitch()->SetSwitch(_isGpuDisabled ? true : false); |
There was a problem hiding this comment.
Because that value is now getting set when the shell is created.
4 participants
This amends the fix from #24503, there was extra code that was undoing the initialization of the syncswitch that we couldn't see because we didn't have reproduction steps. Following the steps in flutter/flutter#64558 (comment) allowed me to reproduce the bug.
fixes flutter/flutter#64558
fixes flutter/flutter#71074
I suspect this may also fix flutter/flutter#76068, but I'm not sure.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.