-
Notifications
You must be signed in to change notification settings - Fork 6k
Avoid crash caused by flutterImageView be null #34395
Conversation
On rare occasions, we had a crash where the flutterImageView was empty, and according to the stack,it's due to the flutterImageView NullPointerException. I guess is the renderer. addIsDisplayingFlutterUiListener register the callback. But by the time the flutter rendering is done, the platform thread called the detachFromFlutterEngine for some reason (and was faster) and left the flutterImageView empty. You can evaluate the need for a merger
|
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 (don't just cc him here, he won't see it! He's on Discord!). 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. |
|
cc @Hixie |
|
From Triage: Seems like a sound fix. cc @dnfield for guidance on testing. |
|
Change is good, but needs a test. Suggested test would be to mock the Without this change, that test will result in an NPE, with it it should succeed. |
|
Thanks for your reply. I will supplement the test according to the suggestions later @dnfield |
|
LGTM once tests are passing. |
|
LGTM |
On rare occasions, we had a crash where the flutterImageView was empty, and according to the stack,it's due to the flutterImageView NullPointerException. I guess is the renderer. addIsDisplayingFlutterUiListener register the callback. But by the time the flutter rendering is done, the platform thread called the detachFromFlutterEngine for some reason (and was faster) and left the flutterImageView empty.
You can evaluate the need for a merger
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.