Skip to content

Commit c1d88a8

Browse files
Merge pull request #16465 from nextcloud/jtr/fix-ui-FileDetailsActivitiesFragment
fix(ui): prevent NPE and memory leaks in FileDetailActivitiesFragment
2 parents 4742524 + f971397 commit c1d88a8

1 file changed

Lines changed: 72 additions & 14 deletions

File tree

app/src/main/java/com/owncloud/android/ui/fragment/FileDetailActivitiesFragment.java

Lines changed: 72 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.apache.commons.httpclient.HttpStatus;
5252
import org.greenrobot.eventbus.EventBus;
5353

54+
import java.lang.ref.WeakReference;
5455
import java.util.ArrayList;
5556
import java.util.List;
5657

@@ -93,6 +94,8 @@ public class FileDetailActivitiesFragment extends Fragment implements
9394
private FileOperationsHelper operationsHelper;
9495
private VersionListInterface.CommentCallback callback;
9596

97+
private SubmitCommentTask submitCommentTask;
98+
9699
FileDetailsActivitiesFragmentBinding binding;
97100

98101
@Inject UserAccountManager accountManager;
@@ -152,13 +155,18 @@ public View onCreateView(@NonNull LayoutInflater inflater,
152155

153156
@Override
154157
public void onSuccess() {
155-
binding.commentInputField.getText().clear();
156-
fetchAndSetData(-1);
158+
if (binding != null && getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.RESUMED)) {
159+
binding.commentInputField.getText().clear();
160+
fetchAndSetData(-1);
161+
}
157162
}
158163

159164
@Override
160165
public void onError(int error) {
161-
Snackbar.make(binding.list, error, Snackbar.LENGTH_LONG).show();
166+
View view = getView();
167+
if (view != null && isAdded()) {
168+
Snackbar.make(view, error, Snackbar.LENGTH_LONG).show();
169+
}
162170
}
163171
};
164172

@@ -177,6 +185,10 @@ public void onError(int error) {
177185
}
178186

179187
public void submitComment() {
188+
if (binding == null) {
189+
return;
190+
}
191+
180192
Editable commentField = binding.commentInputField.getText();
181193

182194
if (commentField == null) {
@@ -186,24 +198,47 @@ public void submitComment() {
186198
String trimmedComment = commentField.toString().trim();
187199

188200
if (!trimmedComment.isEmpty() && nextcloudClient != null && isDataFetched) {
189-
new SubmitCommentTask(trimmedComment, file.getLocalId(), callback, nextcloudClient).execute();
201+
// Cancel previous task
202+
if (submitCommentTask != null) {
203+
submitCommentTask.cancel(true);
204+
}
205+
206+
submitCommentTask = new SubmitCommentTask(
207+
trimmedComment,
208+
file.getLocalId(),
209+
callback,
210+
nextcloudClient
211+
);
212+
submitCommentTask.execute();
190213
}
191214
}
192215

193216
private void setLoadingMessage() {
194-
binding.swipeContainingEmpty.setVisibility(View.GONE);
217+
if (binding != null) {
218+
binding.swipeContainingEmpty.setVisibility(View.GONE);
219+
}
195220
}
196221

197222
@VisibleForTesting
198223
public void setLoadingMessageEmpty() {
199-
binding.swipeContainingList.setVisibility(View.GONE);
200-
binding.emptyList.emptyListView.setVisibility(View.GONE);
201-
binding.loadingContent.setVisibility(View.VISIBLE);
224+
if (binding != null) {
225+
binding.swipeContainingList.setVisibility(View.GONE);
226+
binding.emptyList.emptyListView.setVisibility(View.GONE);
227+
binding.loadingContent.setVisibility(View.VISIBLE);
228+
}
202229
}
203230

204231
@Override
205232
public void onDestroyView() {
206233
super.onDestroyView();
234+
235+
// Cancel any pending async operations
236+
if (submitCommentTask != null) {
237+
submitCommentTask.cancel(true);
238+
submitCommentTask = null;
239+
}
240+
241+
callback = null; // Clear callback reference
207242
binding = null;
208243
}
209244

@@ -373,6 +408,10 @@ public void markCommentsAsRead() {
373408
public void populateList(List<Object> activities, boolean clear) {
374409
adapter.setActivityAndVersionItems(activities, nextcloudClient, clear);
375410

411+
if (binding == null) {
412+
return;
413+
}
414+
376415
if (adapter.getItemCount() == 0) {
377416
setEmptyContent(
378417
getString(R.string.activities_no_results_headline),
@@ -396,6 +435,10 @@ public void setErrorContent(String message) {
396435
}
397436

398437
private void setInfoContent(@DrawableRes int icon, String headline, String message) {
438+
if (binding == null) {
439+
return;
440+
}
441+
399442
binding.emptyList.emptyListIcon.setImageDrawable(ResourcesCompat.getDrawable(requireContext().getResources(),
400443
icon,
401444
null));
@@ -414,7 +457,7 @@ private void setInfoContent(@DrawableRes int icon, String headline, String messa
414457

415458
private void hideRefreshLayoutLoader(FragmentActivity activity) {
416459
activity.runOnUiThread(() -> {
417-
if (getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.RESUMED)) {
460+
if (binding != null && getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.RESUMED)) {
418461
binding.swipeContainingList.setRefreshing(false);
419462
binding.swipeContainingEmpty.setRefreshing(false);
420463
binding.emptyList.emptyListView.setVisibility(View.GONE);
@@ -443,7 +486,9 @@ public void onRestoreClicked(FileVersion fileVersion) {
443486

444487
@Override
445488
public void avatarGenerated(Drawable avatarDrawable, Object callContext) {
446-
binding.avatar.setImageDrawable(avatarDrawable);
489+
if (binding != null) {
490+
binding.avatar.setImageDrawable(avatarDrawable);
491+
}
447492
}
448493

449494
@Override
@@ -460,7 +505,7 @@ private static class SubmitCommentTask extends AsyncTask<Void, Void, Boolean> {
460505

461506
private final String message;
462507
private final long fileId;
463-
private final VersionListInterface.CommentCallback callback;
508+
private final WeakReference<VersionListInterface.CommentCallback> callbackRef;
464509
private final NextcloudClient client;
465510

466511
private SubmitCommentTask(String message,
@@ -469,23 +514,36 @@ private SubmitCommentTask(String message,
469514
NextcloudClient client) {
470515
this.message = message;
471516
this.fileId = fileId;
472-
this.callback = callback;
517+
this.callbackRef = new WeakReference<>(callback);
473518
this.client = client;
474519
}
475520

476521
@Override
477522
protected Boolean doInBackground(Void... voids) {
478-
CommentFileOperation commentFileOperation = new CommentFileOperation(message, fileId);
523+
if (isCancelled()) {
524+
return false;
525+
}
479526

527+
CommentFileOperation commentFileOperation = new CommentFileOperation(message, fileId);
480528
RemoteOperationResult<Void> result = commentFileOperation.execute(client);
481-
482529
return result.isSuccess();
483530
}
484531

485532
@Override
486533
protected void onPostExecute(Boolean success) {
487534
super.onPostExecute(success);
488535

536+
// Don't call callback if task was cancelled
537+
if (isCancelled()) {
538+
return;
539+
}
540+
541+
VersionListInterface.CommentCallback callback = callbackRef.get();
542+
if (callback == null) {
543+
// Fragment was destroyed, callback was GC'd
544+
return;
545+
}
546+
489547
if (success) {
490548
callback.onSuccess();
491549
} else {

0 commit comments

Comments
 (0)