Skip to content

Commit d350582

Browse files
committed
Merge pull request square#1046 from square/dimitris/leak-fix
Fix potential leak with deferred request creators
2 parents 7a706d2 + cb109c7 commit d350582

File tree

5 files changed

+114
-48
lines changed

5 files changed

+114
-48
lines changed

picasso/src/main/java/com/squareup/picasso/DeferredRequestCreator.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ class DeferredRequestCreator implements ViewTreeObserver.OnPreDrawListener {
6161
}
6262

6363
void cancel() {
64+
creator.clearTag();
6465
callback = null;
6566
ImageView target = this.target.get();
6667
if (target == null) {
@@ -72,4 +73,8 @@ void cancel() {
7273
}
7374
vto.removeOnPreDrawListener(this);
7475
}
76+
77+
Object getTag() {
78+
return creator.getTag();
79+
}
7580
}

picasso/src/main/java/com/squareup/picasso/Picasso.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,14 +227,28 @@ public void cancelRequest(RemoteViews remoteViews, int viewId) {
227227
*/
228228
public void cancelTag(Object tag) {
229229
checkMain();
230+
if (tag == null) {
231+
throw new IllegalArgumentException("Cannot cancel requests with null tag.");
232+
}
233+
230234
List<Action> actions = new ArrayList<Action>(targetToAction.values());
231235
//noinspection ForLoopReplaceableByForEach
232236
for (int i = 0, n = actions.size(); i < n; i++) {
233237
Action action = actions.get(i);
234-
if (action.getTag().equals(tag)) {
238+
if (tag.equals(action.getTag())) {
235239
cancelExistingRequest(action.getTarget());
236240
}
237241
}
242+
243+
List<DeferredRequestCreator> deferredRequestCreators =
244+
new ArrayList<DeferredRequestCreator>(targetToDeferredRequestCreator.values());
245+
//noinspection ForLoopReplaceableByForEach
246+
for (int i = 0, n = deferredRequestCreators.size(); i < n; i++) {
247+
DeferredRequestCreator deferredRequestCreator = deferredRequestCreators.get(i);
248+
if (tag.equals(deferredRequestCreator.getTag())) {
249+
deferredRequestCreator.cancel();
250+
}
251+
}
238252
}
239253

240254
/**

picasso/src/main/java/com/squareup/picasso/RequestCreator.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,17 @@ RequestCreator unfit() {
206206
return this;
207207
}
208208

209+
/** Internal use only. Used by {@link DeferredRequestCreator}. */
210+
RequestCreator clearTag() {
211+
this.tag = null;
212+
return this;
213+
}
214+
215+
/** Internal use only. Used by {@link DeferredRequestCreator}. */
216+
Object getTag() {
217+
return tag;
218+
}
219+
209220
/** Resize the image to the specified dimension size. */
210221
public RequestCreator resizeDimen(int targetWidthResId, int targetHeightResId) {
211222
Resources resources = picasso.context.getResources();

picasso/src/test/java/com/squareup/picasso/DeferredRequestCreatorTest.java

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,26 +46,26 @@ public class DeferredRequestCreatorTest {
4646

4747
@Captor ArgumentCaptor<Action> actionCaptor;
4848

49-
@Before public void setUp() throws Exception {
49+
@Before public void setUp() {
5050
initMocks(this);
5151
}
5252

53-
@Test public void initAttachesLayoutListener() throws Exception {
53+
@Test public void initAttachesLayoutListener() {
5454
ImageView target = mockFitImageViewTarget(true);
5555
ViewTreeObserver observer = target.getViewTreeObserver();
5656
DeferredRequestCreator request = new DeferredRequestCreator(mock(RequestCreator.class), target);
5757
verify(observer).addOnPreDrawListener(request);
5858
}
5959

60-
@Test public void cancelRemovesLayoutListener() throws Exception {
60+
@Test public void cancelRemovesLayoutListener() {
6161
ImageView target = mockFitImageViewTarget(true);
6262
ViewTreeObserver observer = target.getViewTreeObserver();
6363
DeferredRequestCreator request = new DeferredRequestCreator(mock(RequestCreator.class), target);
6464
request.cancel();
6565
verify(observer).removeOnPreDrawListener(request);
6666
}
6767

68-
@Test public void cancelClearsCallback() throws Exception {
68+
@Test public void cancelClearsCallback() {
6969
ImageView target = mockFitImageViewTarget(true);
7070
Callback callback = mockCallback();
7171
DeferredRequestCreator request =
@@ -75,7 +75,16 @@ public class DeferredRequestCreatorTest {
7575
assertThat(request.callback).isNull();
7676
}
7777

78-
@Test public void onLayoutSkipsIfTargetIsNull() throws Exception {
78+
@Test public void cancelClearsTag() {
79+
ImageView target = mockFitImageViewTarget(true);
80+
RequestCreator creator = mock(RequestCreator.class);
81+
when(creator.getTag()).thenReturn("TAG");
82+
DeferredRequestCreator request = new DeferredRequestCreator(creator, target);
83+
request.cancel();
84+
verify(creator).clearTag();
85+
}
86+
87+
@Test public void onLayoutSkipsIfTargetIsNull() {
7988
ImageView target = mockFitImageViewTarget(true);
8089
RequestCreator creator = mock(RequestCreator.class);
8190
DeferredRequestCreator request = new DeferredRequestCreator(creator, target);
@@ -87,7 +96,7 @@ public class DeferredRequestCreatorTest {
8796
verifyNoMoreInteractions(viewTreeObserver);
8897
}
8998

90-
@Test public void onLayoutSkipsIfViewTreeObserverIsDead() throws Exception {
99+
@Test public void onLayoutSkipsIfViewTreeObserverIsDead() {
91100
ImageView target = mockFitImageViewTarget(false);
92101
RequestCreator creator = mock(RequestCreator.class);
93102
DeferredRequestCreator request = new DeferredRequestCreator(creator, target);
@@ -99,7 +108,7 @@ public class DeferredRequestCreatorTest {
99108
verifyZeroInteractions(creator);
100109
}
101110

102-
@Test public void waitsForAnotherLayoutIfWidthOrHeightIsZero() throws Exception {
111+
@Test public void waitsForAnotherLayoutIfWidthOrHeightIsZero() {
103112
ImageView target = mockFitImageViewTarget(true);
104113
when(target.getWidth()).thenReturn(0);
105114
when(target.getHeight()).thenReturn(0);
@@ -110,7 +119,7 @@ public class DeferredRequestCreatorTest {
110119
verifyZeroInteractions(creator);
111120
}
112121

113-
@Test public void cancelSkipsWithNullTarget() throws Exception {
122+
@Test public void cancelSkipsWithNullTarget() {
114123
ImageView target = mockFitImageViewTarget(true);
115124
RequestCreator creator = mock(RequestCreator.class);
116125
DeferredRequestCreator request = new DeferredRequestCreator(creator, target);
@@ -119,15 +128,15 @@ public class DeferredRequestCreatorTest {
119128
verify(target.getViewTreeObserver(), never()).removeOnPreDrawListener(request);
120129
}
121130

122-
@Test public void cancelSkipsIfViewTreeObserverIsDead() throws Exception {
131+
@Test public void cancelSkipsIfViewTreeObserverIsDead() {
123132
ImageView target = mockFitImageViewTarget(false);
124133
RequestCreator creator = mock(RequestCreator.class);
125134
DeferredRequestCreator request = new DeferredRequestCreator(creator, target);
126135
request.cancel();
127136
verify(target.getViewTreeObserver(), never()).removeOnPreDrawListener(request);
128137
}
129138

130-
@Test public void onGlobalLayoutSubmitsRequestAndCleansUp() throws Exception {
139+
@Test public void onGlobalLayoutSubmitsRequestAndCleansUp() {
131140
Picasso picasso = mock(Picasso.class);
132141
when(picasso.transformRequest(any(Request.class))).thenAnswer(TRANSFORM_REQUEST_ANSWER);
133142

0 commit comments

Comments
 (0)