fix some leaks and minor changes#3218
Conversation
| Text(context.l10n.countryRegion, style: Styles.formLabel), | ||
| const SizedBox(height: 6.0), | ||
| Autocomplete<String>( | ||
| focusNode: _countryFocusNode, |
There was a problem hiding this comment.
I don't see how this could be a memory leak since the argument was not provided.
Do you assume that internally, Flutter does not dispose the focus node? While it is possible, I doubt it.
There was a problem hiding this comment.
I'll look into that. That was my understanding but I will check everything out to be 100% sure.
There was a problem hiding this comment.
Bad news first:
That code ran, analyzed, tested, etc. but apparently I didn't remember to go to the profile screen and attempt to change the country. 😩 Right now it will give:
══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════════════════════════════════════════════════════
The following assertion was thrown building Autocomplete<String>(dirty):
'package:flutter/src/widgets/autocomplete.dart': Failed assertion: line 191 pos 15: '(focusNode ==
null) == (textEditingController == null)': is not true.
Either the assertion indicates an error in the framework itself, or we should provide substantially
more information in this error message to help you determine and fix the underlying cause.
In either case, please report this assertion by filing a bug on GitHub:
https://github.com/flutter/flutter/issues/new?template=02_bug.yml
What I read about the old code was right though. I stress tested it and can see it leaking.
I fixed the assertion and did a memory test with the new one. It is showing a delta of 0 now.
| recognizer: TapGestureRecognizer() | ||
| ..onTap = () => launchUrlString(url), | ||
| ), | ||
| return GestureDetector( |
There was a problem hiding this comment.
Please explain this change, I don't see why it is better.
There was a problem hiding this comment.
This was because GestureDetector manages its own disposal and we were not disposing the TapGestureRecognizer. If the clicking behavior is going to be worse then I didn't know that. The display worked fine for me and I clicked on everything fine in my testing.
|
I found [RawAutocomplete] Remove assert that don't seem to be needed from 2023 so I guess I don't need to tell them about that. |
| final _formData = <String, dynamic>{ | ||
| 'flag': null, | ||
| 'location': null, | ||
| 'bio': null, | ||
| 'flair': null, | ||
| 'realName': null, | ||
| 'lastName': null, | ||
| 'fideRating': null, |
There was a problem hiding this comment.
I set a flair with this build, and cleared the flair too. It worked for me.
Now I'm really, really taking a break.
lib/src/view/account/edit_profile_screen.dartfixes a potential memory leak
Linked the
focusNodefromAutocomplete<String>to_countryFocusNodeAdded
_countryFocusNode.dispose()lib/src/view/message/contacts_screen.dartadded
_controller.dispose();added
_focusNode.dispose();Used Riverpod's built-in
ref.debounce()instead of a manual debouncer that we also have to cancel later.lib/src/view/user/player_screen.dartEnsure we really stop watching our friends.
There is a potential race condition if the user pops the
PlayerScreenoff the navigation stack and then the(context.mounted == false)and we don't stop watching friends.lib/src/view/user/search_screen.dartfixed typo in the variable name
_kSaveHistoryDebounceTimeradded
_saveHistoryDebouncer.cancelremoved a
ref.readbecause it is redundant when we are already watchinglib/src/widgets/emoji_picker/emoji_picker.dartadded
_debouncer.cancel();lib/src/widgets/platform_search_bar.dartFixes a potential memory leak. When a parent widget rebuilds and passes a new
TextEditingControllerthe old controller still holds a reference to_onTextChanged.didUpdateWidgetswaps the listener to the new controller.lib/src/view/broadcast/broadcast_game_screen.dartfixes a memory leak in the
BroadcastGameScreenby replacingTapGestureRecognizerwithGestureDetectorTapGestureRecognizerinstances insideTextSpanmust be manually disposed of to prevent memory leaks. They were never being cleaned up.Removed
import 'package:flutter/gestures.dart';since it's no longer used,Made sure the broadcast pgn text still looks fine.