Unregister Receiver #652
Unregister Receiver #652
Conversation
|
@mhutti1 Could you please take a look into this? |
| super.getActivity().unbindService(mConnection.downloadServiceInterface); | ||
| mBound = false; | ||
| } | ||
| if (isReceiverRegistered){ |
There was a problem hiding this comment.
Ok. just a question. The logic of unregistering should be done in onStop right?
There was a problem hiding this comment.
I have changed it. Can you review it again?
There was a problem hiding this comment.
@srv-twry take a look at it
|
@srv-twry I have changed the code and adjusted the spaces. can you take a look into it? |
|
@hjoshi123 Yes, looking at it again but please don't ping multiple times for it. You did it in the review comments as well. |
| @@ -45,6 +46,7 @@ | |||
| import javax.inject.Inject; | |||
|
|
|||
| import static org.kiwix.kiwixmobile.utils.StyleUtils.dialogStyle; | |||
| import static org.kiwix.kiwixmobile.zim_manager.library_view.LibraryFragment.DownloadServiceConnection.*; | |||
There was a problem hiding this comment.
I am not sure we need these imports. You have just removed a method. Why add these ? May be you were trying out things but make sure to optimize imports before sending the PR.
There was a problem hiding this comment.
@srv-twry I wanted to ask about that. Could we like write a gradle script or something so that we don't need to remove those manually?
There was a problem hiding this comment.
Android studio has a shortcut ctrl + alt + o. I haven't seen anyone writing the gradle script for it.
| public void onDetach() { | ||
| if (mBound && super.getActivity() != null) { | ||
| super.getActivity().unbindService(mConnection.downloadServiceInterface); | ||
| mBound = false; |
There was a problem hiding this comment.
onDetach gets called everytime the fragment is recreated inside the Activity. If you're unbinding the service in the onDetach then you don't need to do it onStop i.e. it should only be in one method.
There was a problem hiding this comment.
Oh Yeah. Sorry for that mistake. Lemme correct it.
|
@srv-twry Give it a look again. :) |
|
@hjoshi123 Please update the branch. |
|
@kelson42 Can you give a look into it and merge this? |
|
@mhutti1 Can you please review that PR as downloading related crash are massive and quite annoying to leave in 2.4? |
Fixes #600
Changes: Added unregisterReceiver() in LibraryFragment to stop the crash.