Skip to content

Commit

Permalink
Remove pause() and isPaused() from Request API
Browse files Browse the repository at this point in the history
Various other changes allowing Requests to restart have removed the
assertion that cleared requests are never restarted. There doesn't seem
to be any need currently to distinguish between a request that's cleared
by a call to Glide.with().clear() and a request that's cleared by Glide.
We can simplify the API and reduce confusion by just calling clear()
directly and removing pause() entirely.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=201439855
  • Loading branch information
sjudd committed Jun 22, 2018
1 parent 386d89e commit 9089752
Show file tree
Hide file tree
Showing 12 changed files with 238 additions and 308 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void pauseRequests() {
isPaused = true;
for (Request request : Util.getSnapshot(requests)) {
if (request.isRunning()) {
request.pause();
request.clear();
pendingRequests.add(request);
}
}
Expand All @@ -109,7 +109,7 @@ public void pauseAllRequests() {
isPaused = true;
for (Request request : Util.getSnapshot(requests)) {
if (request.isRunning() || request.isComplete()) {
request.pause();
request.clear();
pendingRequests.add(request);
}
}
Expand All @@ -121,7 +121,10 @@ public void pauseAllRequests() {
public void resumeRequests() {
isPaused = false;
for (Request request : Util.getSnapshot(requests)) {
if (!request.isComplete() && !request.isCancelled() && !request.isRunning()) {
// We don't need to check for cleared here. Any explicit clear by a user will remove the
// Request from the tracker, so the only way we'd find a cleared request here is if we cleared
// it. As a result it should be safe for us to resume cleared requests.
if (!request.isComplete() && !request.isRunning()) {
request.begin();
}
}
Expand All @@ -147,12 +150,12 @@ public void clearRequests() {
*/
public void restartRequests() {
for (Request request : Util.getSnapshot(requests)) {
if (!request.isComplete() && !request.isCancelled()) {
// Ensure the request will be restarted in onResume.
request.pause();
if (!request.isComplete() && !request.isCleared()) {
request.clear();
if (!isPaused) {
request.begin();
} else {
// Ensure the request will be restarted in onResume.
pendingRequests.add(request);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,6 @@ public void begin() {
}
}

@Override
public void pause() {
if (!primary.isFailed()) {
primary.pause();
}
if (error.isRunning()) {
error.pause();
}
}

@Override
public void clear() {
primary.clear();
Expand All @@ -50,11 +40,6 @@ public void clear() {
}
}

@Override
public boolean isPaused() {
return primary.isFailed() ? error.isPaused() : primary.isPaused();
}

@Override
public boolean isRunning() {
return primary.isFailed() ? error.isRunning() : primary.isRunning();
Expand All @@ -71,8 +56,8 @@ public boolean isResourceSet() {
}

@Override
public boolean isCancelled() {
return primary.isFailed() ? error.isCancelled() : primary.isCancelled();
public boolean isCleared() {
return primary.isFailed() ? error.isCleared() : primary.isCleared();
}

@Override
Expand Down
14 changes: 2 additions & 12 deletions library/src/main/java/com/bumptech/glide/request/Request.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,13 @@ public interface Request {
*/
void begin();

/**
* Identical to {@link #clear()} except that the request may later be restarted.
*/
void pause();

/**
* Prevents any bitmaps being loaded from previous requests, releases any resources held by this
* request, displays the current placeholder if one was provided, and marks the request as having
* been cancelled.
*/
void clear();

/**
* Returns true if this request is paused and may be restarted.
*/
boolean isPaused();

/**
* Returns true if this request is running and has not completed or failed.
*/
Expand All @@ -44,9 +34,9 @@ public interface Request {
boolean isResourceSet();

/**
* Returns true if the request has been cancelled.
* Returns true if the request has been cleared.
*/
boolean isCancelled();
boolean isCleared();

/**
* Returns true if the request has failed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,9 @@ private enum Status {
*/
FAILED,
/**
* Cancelled by the user, may not be restarted.
*/
CANCELLED,
/**
* Cleared by the user with a placeholder set, may not be restarted.
* Cleared by the user with a placeholder set, may be restarted.
*/
CLEARED,
/**
* Temporarily paused by the system, may be restarted.
*/
PAUSED,
}

@Nullable
Expand Down Expand Up @@ -282,11 +274,10 @@ && canNotifyStatusChanged()) {
*
* @see #clear()
*/
void cancel() {
private void cancel() {
assertNotCallingCallbacks();
stateVerifier.throwIfRecycled();
target.removeCallback(this);
status = Status.CANCELLED;
if (loadStatus != null) {
loadStatus.cancel();
loadStatus = null;
Expand Down Expand Up @@ -327,19 +318,8 @@ public void clear() {
if (canNotifyCleared()) {
target.onLoadCleared(getPlaceholderDrawable());
}
// Must be after cancel().
status = Status.CLEARED;
}

@Override
public boolean isPaused() {
return status == Status.PAUSED;
}

@Override
public void pause() {
clear();
status = Status.PAUSED;
status = Status.CLEARED;
}

private void releaseResource(Resource<?> resource) {
Expand All @@ -363,8 +343,8 @@ public boolean isResourceSet() {
}

@Override
public boolean isCancelled() {
return status == Status.CANCELLED || status == Status.CLEARED;
public boolean isCleared() {
return status == Status.CLEARED;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,25 +120,13 @@ public void begin() {
}
}

@Override
public void pause() {
isRunning = false;
full.pause();
thumb.pause();
}

@Override
public void clear() {
isRunning = false;
thumb.clear();
full.clear();
}

@Override
public boolean isPaused() {
return full.isPaused();
}

/**
* Returns true if the full request is still running.
*/
Expand All @@ -161,8 +149,8 @@ public boolean isResourceSet() {
}

@Override
public boolean isCancelled() {
return full.isCancelled();
public boolean isCleared() {
return full.isCleared();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,19 @@ public void onViewDetachedFromWindow(View v) {
@SuppressWarnings("WeakerAccess")
@Synthetic void resumeMyRequest() {
Request request = getRequest();
if (request != null && request.isPaused()) {
if (request != null && request.isCleared()) {
request.begin();
}
}

@SuppressWarnings("WeakerAccess")
@Synthetic void pauseMyRequest() {
Request request = getRequest();
if (request != null && !request.isCancelled() && !request.isPaused()) {
// If the Request were cleared by the developer, it would be null here. The only way it's
// present is if the developer hasn't previously cleared this Target.
if (request != null) {
isClearedByUs = true;
request.pause();
request.clear();
isClearedByUs = false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ public void onResourceReady(@NonNull Drawable resource,
requestManager.clear(target);

assertThat(target.getRequest()).isNull();
assertThat(request.isCancelled()).isTrue();
assertThat(request.isCleared()).isTrue();
}

@Test
Expand Down
Loading

0 comments on commit 9089752

Please sign in to comment.