-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Clean injected View fields in onDestroyView() #1165
Conversation
|
I think we really should clean up the injected |
|
The background thread does not need to access views directly, it may just call a @UiThread-decorated method that will. I'll try to make the change you propose, but since I don't yet have deep understanding of how does AndroidAnnotations work, this may take a while. |
|
Yes, you are right. To almost properly fix that issue, the developer can add |
|
What would be the safe way then? A new annotation like |
|
Please review this commit carefully, as I may have missed something important. |
|
I think this is good. But i would not merge without @yDelouis taking a look at this. |
|
Wait, I think I did indeed miss something. Will push a commit in few minutes. |
|
Good point, actually i was thinking about, too, but forgot to mention. |
|
By cleaning injected views, we will make some existing app crash. |
|
@yDelouis, i guess you are right. But we have to release 4.0.0 with the new plugin system. When do you have time to continue work on that? |
|
Shall I make another PR only with the first (safe) commit then? |
|
Yes, please. |
|
Here it is: #1171 |
|
Great. Could you rebase this branch (since the first PR got merged)? |
|
Rebased onto wrong branch, should be fine now. |
|
Can you rebase this? |
Clean refs to injected views in onDestroyView()
|
Rebased and merged as of faaaae5. |
If setRetainInstance(true) has been set on the fragment or
addToBackStack() was applied to FragmentTransaction, the fragment won't be
destroyed and will keep the reference to its parent view in contentView_
and could lead to memory leak.
This change implements onDestroyView() which cleans the reference.
However, the developer is still responsible for cleaning up the injected
views, because otherwise a (badly implemented) background task may get a
NullPointerException.