Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow host app to provide a way to clear all resources onStop() #5145

Merged
merged 3 commits into from
Jul 1, 2023
Merged

Allow host app to provide a way to clear all resources onStop() #5145

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b7fa881
* Integrated a way in glide to allow host app to provide a way to cle…
osamaaftab May 18, 2023
5837c55
* Clear targetTracker only in case clearOnStop is true
osamaaftab May 21, 2023
31d9ce3
* Extracted a clearRequests() and re-used it in onStop() and onDestroy()
osamaaftab Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion library/src/main/java/com/bumptech/glide/RequestManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ public void run() {

private boolean pauseAllRequestsOnTrimMemoryModerate;

private boolean clearOnStop;

public RequestManager(
@NonNull Glide glide,
@NonNull Lifecycle lifecycle,
Expand Down Expand Up @@ -203,6 +205,17 @@ public synchronized RequestManager setDefaultRequestOptions(
return this;
}

/**
* Clear all resources when onStop() from {@link LifecycleListener} is called.
*
* @return This request manager.
*/
@NonNull
public synchronized RequestManager clearOnStop() {
clearOnStop = true;
return this;
}

/**
* Adds a default {@link RequestListener} that will be added to every request started with this
* {@link RequestManager}.
Expand Down Expand Up @@ -354,12 +367,19 @@ public synchronized void onStart() {

/**
* Lifecycle callback that unregisters for connectivity events (if the
* android.permission.ACCESS_NETWORK_STATE permission is present) and pauses in progress loads.
* android.permission.ACCESS_NETWORK_STATE permission is present) and pauses in progress loads
* and clears all resources if {@link #clearOnStop()} is called.
*/
@Override
public synchronized void onStop() {
pauseRequests();
targetTracker.onStop();
if (clearOnStop) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you extract a clearRequests() method and re-use it here and in onDestroy?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and I think you could then do:

if (clearOnStop) {
  clearRequests();
} else { 
  pauseRequests();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjudd thanks for the suggestion mate.
Please review, I have implemented.

for (Target<?> target : targetTracker.getAll()) {
clear(target);
}
targetTracker.clear();
}
}

/**
Expand Down