Skip to content

Commit 833ef21

Browse files
sjuddglide-copybara-robot
sjudd
authored and
glide-copybara-robot
committedOct 28, 2021
Rollforward using a non-deprecated API for connectivity monitoring
PiperOrigin-RevId: 406191957
·
v5.0.0-rc01compose-alpha6
1 parent b43ea98 commit 833ef21

File tree

3 files changed

+354
-93
lines changed

3 files changed

+354
-93
lines changed
 

‎library/src/main/java/com/bumptech/glide/manager/SingletonConnectivityReceiver.java

+197-71
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,21 @@
66
import android.content.Intent;
77
import android.content.IntentFilter;
88
import android.net.ConnectivityManager;
9+
import android.net.ConnectivityManager.NetworkCallback;
10+
import android.net.Network;
911
import android.net.NetworkInfo;
12+
import android.os.Build;
13+
import android.os.Build.VERSION_CODES;
1014
import android.util.Log;
1115
import androidx.annotation.GuardedBy;
1216
import androidx.annotation.NonNull;
17+
import androidx.annotation.RequiresApi;
1318
import androidx.annotation.VisibleForTesting;
1419
import com.bumptech.glide.manager.ConnectivityMonitor.ConnectivityListener;
15-
import com.bumptech.glide.util.Preconditions;
20+
import com.bumptech.glide.util.GlideSuppliers;
21+
import com.bumptech.glide.util.GlideSuppliers.GlideSupplier;
1622
import com.bumptech.glide.util.Synthetic;
23+
import com.bumptech.glide.util.Util;
1724
import java.util.ArrayList;
1825
import java.util.HashSet;
1926
import java.util.List;
@@ -22,41 +29,9 @@
2229
/** Uses {@link android.net.ConnectivityManager} to identify connectivity changes. */
2330
final class SingletonConnectivityReceiver {
2431
private static volatile SingletonConnectivityReceiver instance;
25-
@Synthetic static final String TAG = "ConnectivityMonitor";
26-
// Only accessed on the main thread.
27-
@Synthetic boolean isConnected;
28-
29-
private final BroadcastReceiver connectivityReceiver =
30-
new BroadcastReceiver() {
31-
@Override
32-
public void onReceive(@NonNull Context context, Intent intent) {
33-
List<ConnectivityListener> listenersToNotify = null;
34-
boolean wasConnected = isConnected;
35-
isConnected = isConnected(context);
36-
if (wasConnected != isConnected) {
37-
if (Log.isLoggable(TAG, Log.DEBUG)) {
38-
Log.d(TAG, "connectivity changed, isConnected: " + isConnected);
39-
}
40-
41-
synchronized (SingletonConnectivityReceiver.this) {
42-
listenersToNotify = new ArrayList<>(listeners);
43-
}
44-
}
45-
// Make sure that we do not hold our lock while calling our listener. Otherwise we could
46-
// deadlock where our listener acquires its lock, then tries to acquire ours elsewhere and
47-
// then here we acquire our lock and try to acquire theirs.
48-
// The consequence of this is that we may notify a listener after it has been
49-
// unregistered in a few specific (unlikely) scenarios. That appears to be safe and is
50-
// documented in the unregister method.
51-
if (listenersToNotify != null) {
52-
for (ConnectivityListener listener : listenersToNotify) {
53-
listener.onConnectivityChanged(isConnected);
54-
}
55-
}
56-
}
57-
};
32+
private static final String TAG = "ConnectivityMonitor";
5833

59-
private final Context context;
34+
private final FrameworkConnectivityMonitor frameworkConnectivityMonitor;
6035

6136
@GuardedBy("this")
6237
@Synthetic
@@ -69,7 +44,7 @@ static SingletonConnectivityReceiver get(@NonNull Context context) {
6944
if (instance == null) {
7045
synchronized (SingletonConnectivityReceiver.class) {
7146
if (instance == null) {
72-
instance = new SingletonConnectivityReceiver(context);
47+
instance = new SingletonConnectivityReceiver(context.getApplicationContext());
7348
}
7449
}
7550
}
@@ -81,8 +56,34 @@ static void reset() {
8156
instance = null;
8257
}
8358

84-
private SingletonConnectivityReceiver(@NonNull Context context) {
85-
this.context = context.getApplicationContext();
59+
private SingletonConnectivityReceiver(final @NonNull Context context) {
60+
GlideSupplier<ConnectivityManager> connectivityManager =
61+
GlideSuppliers.memorize(
62+
new GlideSupplier<ConnectivityManager>() {
63+
@Override
64+
public ConnectivityManager get() {
65+
return (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
66+
}
67+
});
68+
ConnectivityListener connectivityListener =
69+
new ConnectivityListener() {
70+
@Override
71+
public void onConnectivityChanged(boolean isConnected) {
72+
List<ConnectivityListener> toNotify;
73+
synchronized (SingletonConnectivityReceiver.this) {
74+
toNotify = new ArrayList<>(listeners);
75+
}
76+
for (ConnectivityListener listener : toNotify) {
77+
listener.onConnectivityChanged(isConnected);
78+
}
79+
}
80+
};
81+
82+
frameworkConnectivityMonitor =
83+
Build.VERSION.SDK_INT >= Build.VERSION_CODES.N
84+
? new FrameworkConnectivityMonitorPostApi24(connectivityManager, connectivityListener)
85+
: new FrameworkConnectivityMonitorPreApi24(
86+
context, connectivityManager, connectivityListener);
8687
}
8788

8889
synchronized void register(ConnectivityListener listener) {
@@ -106,18 +107,7 @@ private void maybeRegisterReceiver() {
106107
if (isRegistered || listeners.isEmpty()) {
107108
return;
108109
}
109-
isConnected = isConnected(context);
110-
try {
111-
// See #1405
112-
context.registerReceiver(
113-
connectivityReceiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
114-
isRegistered = true;
115-
} catch (SecurityException e) {
116-
// See #1417, registering the receiver can throw SecurityException.
117-
if (Log.isLoggable(TAG, Log.WARN)) {
118-
Log.w(TAG, "Failed to register", e);
119-
}
120-
}
110+
isRegistered = frameworkConnectivityMonitor.register();
121111
}
122112

123113
@GuardedBy("this")
@@ -126,31 +116,167 @@ private void maybeUnregisterReceiver() {
126116
return;
127117
}
128118

129-
context.unregisterReceiver(connectivityReceiver);
119+
frameworkConnectivityMonitor.unregister();
130120
isRegistered = false;
131121
}
132122

133-
@SuppressWarnings("WeakerAccess")
134-
@Synthetic
135-
// Permissions are checked in the factory instead.
136-
@SuppressLint("MissingPermission")
137-
boolean isConnected(@NonNull Context context) {
138-
ConnectivityManager connectivityManager =
139-
Preconditions.checkNotNull(
140-
(ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE));
141-
NetworkInfo networkInfo;
142-
try {
143-
networkInfo = connectivityManager.getActiveNetworkInfo();
144-
} catch (RuntimeException e) {
145-
// #1405 shows that this throws a SecurityException.
146-
// b/70869360 shows that this throws NullPointerException on APIs 22, 23, and 24.
147-
// b/70869360 also shows that this throws RuntimeException on API 24 and 25.
148-
if (Log.isLoggable(TAG, Log.WARN)) {
149-
Log.w(TAG, "Failed to determine connectivity status when connectivity changed", e);
123+
private interface FrameworkConnectivityMonitor {
124+
boolean register();
125+
126+
void unregister();
127+
}
128+
129+
@RequiresApi(VERSION_CODES.N)
130+
private static final class FrameworkConnectivityMonitorPostApi24
131+
implements FrameworkConnectivityMonitor {
132+
133+
@Synthetic boolean isConnected;
134+
@Synthetic final ConnectivityListener listener;
135+
private final GlideSupplier<ConnectivityManager> connectivityManager;
136+
private final NetworkCallback networkCallback =
137+
new NetworkCallback() {
138+
@Override
139+
public void onAvailable(@NonNull Network network) {
140+
postOnConnectivityChange(true);
141+
}
142+
143+
@Override
144+
public void onLost(@NonNull Network network) {
145+
postOnConnectivityChange(false);
146+
}
147+
148+
private void postOnConnectivityChange(final boolean newState) {
149+
// We could use registerDefaultNetworkCallback with a Handler, but that's only available
150+
// on API 26, instead of API 24. We can mimic the same behavior here manually by
151+
// posting to the UI thread. All calls have to be posted to make sure that we retain the
152+
// original order. Otherwise a call on a background thread, followed by a call on the UI
153+
// thread could result in the first call running second.
154+
Util.postOnUiThread(
155+
new Runnable() {
156+
@Override
157+
public void run() {
158+
onConnectivityChange(newState);
159+
}
160+
});
161+
}
162+
163+
@Synthetic
164+
void onConnectivityChange(boolean newState) {
165+
// See b/201425456.
166+
Util.assertMainThread();
167+
168+
boolean wasConnected = isConnected;
169+
isConnected = newState;
170+
if (wasConnected != newState) {
171+
listener.onConnectivityChanged(newState);
172+
}
173+
}
174+
};
175+
176+
FrameworkConnectivityMonitorPostApi24(
177+
GlideSupplier<ConnectivityManager> connectivityManager, ConnectivityListener listener) {
178+
this.connectivityManager = connectivityManager;
179+
this.listener = listener;
180+
}
181+
182+
// Permissions are checked in the factory instead.
183+
@SuppressLint("MissingPermission")
184+
@Override
185+
public boolean register() {
186+
isConnected = connectivityManager.get().getActiveNetwork() != null;
187+
try {
188+
connectivityManager.get().registerDefaultNetworkCallback(networkCallback);
189+
return true;
190+
// See b/201664814, b/204226444: At least TooManyRequestsException is not public and
191+
// doesn't extend from any subclass :/.
192+
} catch (RuntimeException e) {
193+
if (Log.isLoggable(TAG, Log.WARN)) {
194+
Log.w(TAG, "Failed to register callback", e);
195+
}
196+
return false;
197+
}
198+
}
199+
200+
@Override
201+
public void unregister() {
202+
connectivityManager.get().unregisterNetworkCallback(networkCallback);
203+
}
204+
}
205+
206+
private static final class FrameworkConnectivityMonitorPreApi24
207+
implements FrameworkConnectivityMonitor {
208+
private final Context context;
209+
@Synthetic final ConnectivityListener listener;
210+
private final GlideSupplier<ConnectivityManager> connectivityManager;
211+
@Synthetic boolean isConnected;
212+
213+
private final BroadcastReceiver connectivityReceiver =
214+
new BroadcastReceiver() {
215+
@Override
216+
public void onReceive(@NonNull Context context, Intent intent) {
217+
boolean wasConnected = isConnected;
218+
isConnected = isConnected();
219+
if (wasConnected != isConnected) {
220+
if (Log.isLoggable(TAG, Log.DEBUG)) {
221+
Log.d(TAG, "connectivity changed, isConnected: " + isConnected);
222+
}
223+
224+
listener.onConnectivityChanged(isConnected);
225+
}
226+
}
227+
};
228+
229+
FrameworkConnectivityMonitorPreApi24(
230+
Context context,
231+
GlideSupplier<ConnectivityManager> connectivityManager,
232+
ConnectivityListener listener) {
233+
this.context = context.getApplicationContext();
234+
this.connectivityManager = connectivityManager;
235+
this.listener = listener;
236+
}
237+
238+
@Override
239+
public boolean register() {
240+
// Initialize isConnected so that we notice the first time around when there's a broadcast.
241+
isConnected = isConnected();
242+
try {
243+
// See #1405
244+
context.registerReceiver(
245+
connectivityReceiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
246+
return true;
247+
} catch (SecurityException e) {
248+
// See #1417, registering the receiver can throw SecurityException.
249+
if (Log.isLoggable(TAG, Log.WARN)) {
250+
Log.w(TAG, "Failed to register", e);
251+
}
252+
return false;
253+
}
254+
}
255+
256+
@Override
257+
public void unregister() {
258+
context.unregisterReceiver(connectivityReceiver);
259+
}
260+
261+
@SuppressWarnings("WeakerAccess")
262+
@Synthetic
263+
// Permissions are checked in the factory instead.
264+
@SuppressLint("MissingPermission")
265+
boolean isConnected() {
266+
NetworkInfo networkInfo;
267+
try {
268+
networkInfo = connectivityManager.get().getActiveNetworkInfo();
269+
} catch (RuntimeException e) {
270+
// #1405 shows that this throws a SecurityException.
271+
// b/70869360 shows that this throws NullPointerException on APIs 22, 23, and 24.
272+
// b/70869360 also shows that this throws RuntimeException on API 24 and 25.
273+
if (Log.isLoggable(TAG, Log.WARN)) {
274+
Log.w(TAG, "Failed to determine connectivity status when connectivity changed", e);
275+
}
276+
// Default to true;
277+
return true;
150278
}
151-
// Default to true;
152-
return true;
279+
return networkInfo != null && networkInfo.isConnected();
153280
}
154-
return networkInfo != null && networkInfo.isConnected();
155281
}
156282
}

‎library/src/main/java/com/bumptech/glide/util/GlideSuppliers.java

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package com.bumptech.glide.util;
2+
3+
/** Similar to {@link com.google.common.base.Suppliers}, but named to reduce import confusion. */
4+
public final class GlideSuppliers {
5+
/**
6+
* Produces a non-null instance of {@code T}.
7+
*
8+
* @param <T> The data type
9+
*/
10+
public interface GlideSupplier<T> {
11+
T get();
12+
}
13+
14+
private GlideSuppliers() {}
15+
16+
public static <T> GlideSupplier<T> memorize(final GlideSupplier<T> supplier) {
17+
return new GlideSupplier<T>() {
18+
private volatile T instance;
19+
20+
@Override
21+
public T get() {
22+
if (instance == null) {
23+
synchronized (this) {
24+
if (instance == null) {
25+
instance = Preconditions.checkNotNull(supplier.get());
26+
}
27+
}
28+
}
29+
return instance;
30+
}
31+
};
32+
}
33+
}

‎library/test/src/test/java/com/bumptech/glide/manager/DefaultConnectivityMonitorTest.java

+124-22
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@
99
import static org.robolectric.annotation.LooperMode.Mode.LEGACY;
1010

1111
import android.app.Application;
12-
import android.content.BroadcastReceiver;
1312
import android.content.Context;
1413
import android.content.Intent;
1514
import android.net.ConnectivityManager;
15+
import android.net.ConnectivityManager.NetworkCallback;
16+
import android.net.Network;
1617
import android.net.NetworkInfo;
18+
import android.os.Build;
1719
import androidx.test.core.app.ApplicationProvider;
1820
import com.bumptech.glide.manager.DefaultConnectivityMonitorTest.PermissionConnectivityManager;
19-
import java.util.List;
2021
import org.junit.After;
2122
import org.junit.Before;
2223
import org.junit.Test;
@@ -30,11 +31,14 @@
3031
import org.robolectric.annotation.LooperMode;
3132
import org.robolectric.shadow.api.Shadow;
3233
import org.robolectric.shadows.ShadowConnectivityManager;
34+
import org.robolectric.shadows.ShadowNetwork;
3335
import org.robolectric.shadows.ShadowNetworkInfo;
3436

3537
@LooperMode(LEGACY)
3638
@RunWith(RobolectricTestRunner.class)
37-
@Config(sdk = 18, shadows = PermissionConnectivityManager.class)
39+
@Config(
40+
sdk = {24},
41+
shadows = PermissionConnectivityManager.class)
3842
public class DefaultConnectivityMonitorTest {
3943
@Mock private ConnectivityMonitor.ConnectivityListener listener;
4044
private DefaultConnectivityMonitor monitor;
@@ -44,7 +48,10 @@ public class DefaultConnectivityMonitorTest {
4448
public void setUp() {
4549
MockitoAnnotations.initMocks(this);
4650
monitor = new DefaultConnectivityMonitor(ApplicationProvider.getApplicationContext(), listener);
47-
harness = new ConnectivityHarness();
51+
harness =
52+
Build.VERSION.SDK_INT >= Build.VERSION_CODES.N
53+
? new ConnectivityHarnessPost24()
54+
: new ConnectivityHarnessPre24();
4855
}
4956

5057
@After
@@ -56,31 +63,31 @@ public void tearDown() {
5663
public void testRegistersReceiverOnStart() {
5764
monitor.onStart();
5865

59-
assertThat(getConnectivityReceivers()).hasSize(1);
66+
assertThat(harness.getRegisteredReceivers()).isEqualTo(1);
6067
}
6168

6269
@Test
6370
public void testDoesNotRegisterTwiceOnStart() {
6471
monitor.onStart();
6572
monitor.onStart();
6673

67-
assertThat(getConnectivityReceivers()).hasSize(1);
74+
assertThat(harness.getRegisteredReceivers()).isEqualTo(1);
6875
}
6976

7077
@Test
7178
public void testUnregistersReceiverOnStop() {
7279
monitor.onStart();
7380
monitor.onStop();
7481

75-
assertThat(getConnectivityReceivers()).isEmpty();
82+
assertThat(harness.getRegisteredReceivers()).isEqualTo(0);
7683
}
7784

7885
@Test
7986
public void testHandlesUnregisteringTwiceInARow() {
8087
monitor.onStop();
8188
monitor.onStop();
8289

83-
assertThat(getConnectivityReceivers()).isEmpty();
90+
assertThat(harness.getRegisteredReceivers()).isEqualTo(0);
8491
}
8592

8693
@Test
@@ -129,74 +136,169 @@ public void testDoesNotNotifyListenerWhenNotRegistered() {
129136

130137
@Test
131138
public void register_withMissingPermission_doesNotThrow() {
132-
harness.shadowConnectivityManager.isNetworkPermissionGranted = false;
139+
harness.setNetworkPermissionGranted(false);
133140

134141
monitor.onStart();
135142
}
136143

137144
@Test
138145
public void onReceive_withMissingPermission_doesNotThrow() {
139146
monitor.onStart();
140-
harness.shadowConnectivityManager.isNetworkPermissionGranted = false;
147+
harness.setNetworkPermissionGranted(false);
141148
harness.broadcast();
142149
}
143150

144151
@Test
145152
public void onReceive_withMissingPermission_previouslyDisconnected_notifiesListenersConnected() {
146153
harness.disconnect();
147154
monitor.onStart();
148-
harness.shadowConnectivityManager.isNetworkPermissionGranted = false;
155+
harness.setNetworkPermissionGranted(false);
149156
harness.broadcast();
150157

151-
verify(listener).onConnectivityChanged(true);
158+
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) {
159+
verify(listener).onConnectivityChanged(true);
160+
} else {
161+
verify(listener, never()).onConnectivityChanged(anyBoolean());
162+
}
152163
}
153164

154165
@Test
155166
public void onReceive_withMissingPermission_previouslyConnected_doesNotNotifyListeners() {
156167
harness.connect();
157168
monitor.onStart();
158-
harness.shadowConnectivityManager.isNetworkPermissionGranted = false;
169+
harness.setNetworkPermissionGranted(false);
159170
harness.broadcast();
160171

161172
verify(listener, never()).onConnectivityChanged(anyBoolean());
162173
}
163174

164-
private List<BroadcastReceiver> getConnectivityReceivers() {
165-
Intent connectivity = new Intent(ConnectivityManager.CONNECTIVITY_ACTION);
166-
return shadowOf((Application) ApplicationProvider.getApplicationContext())
167-
.getReceiversForIntent(connectivity);
175+
private interface ConnectivityHarness {
176+
void connect();
177+
178+
void disconnect();
179+
180+
void broadcast();
181+
182+
void setNetworkPermissionGranted(boolean isGranted);
183+
184+
int getRegisteredReceivers();
168185
}
169186

170-
private static class ConnectivityHarness {
187+
private static final class ConnectivityHarnessPost24 implements ConnectivityHarness {
188+
171189
private final PermissionConnectivityManager shadowConnectivityManager;
172190

173-
public ConnectivityHarness() {
191+
ConnectivityHarnessPost24() {
174192
ConnectivityManager connectivityManager =
175193
(ConnectivityManager)
176194
ApplicationProvider.getApplicationContext()
177195
.getSystemService(Context.CONNECTIVITY_SERVICE);
178196
shadowConnectivityManager = Shadow.extract(connectivityManager);
179197
}
180198

181-
void disconnect() {
199+
@Override
200+
public void connect() {
201+
shadowConnectivityManager.isConnected = true;
202+
}
203+
204+
@Override
205+
public void disconnect() {
206+
shadowConnectivityManager.isConnected = false;
207+
}
208+
209+
@Override
210+
public void broadcast() {
211+
for (NetworkCallback callback : shadowConnectivityManager.getNetworkCallbacks()) {
212+
if (shadowConnectivityManager.isConnected) {
213+
callback.onAvailable(null);
214+
} else {
215+
callback.onLost(null);
216+
}
217+
}
218+
}
219+
220+
@Override
221+
public void setNetworkPermissionGranted(boolean isGranted) {
222+
shadowConnectivityManager.isNetworkPermissionGranted = isGranted;
223+
}
224+
225+
@Override
226+
public int getRegisteredReceivers() {
227+
return shadowConnectivityManager.getNetworkCallbacks().size();
228+
}
229+
}
230+
231+
private static final class ConnectivityHarnessPre24 implements ConnectivityHarness {
232+
private final PermissionConnectivityManager shadowConnectivityManager;
233+
234+
public ConnectivityHarnessPre24() {
235+
ConnectivityManager connectivityManager =
236+
(ConnectivityManager)
237+
ApplicationProvider.getApplicationContext()
238+
.getSystemService(Context.CONNECTIVITY_SERVICE);
239+
shadowConnectivityManager = Shadow.extract(connectivityManager);
240+
}
241+
242+
@Override
243+
public void disconnect() {
182244
shadowConnectivityManager.setActiveNetworkInfo(null);
183245
}
184246

185-
void connect() {
247+
@Override
248+
public void connect() {
186249
NetworkInfo networkInfo =
187250
ShadowNetworkInfo.newInstance(NetworkInfo.DetailedState.CONNECTED, 0, 0, true, true);
188251
shadowConnectivityManager.setActiveNetworkInfo(networkInfo);
189252
}
190253

191-
void broadcast() {
254+
@Override
255+
public void broadcast() {
192256
Intent connected = new Intent(ConnectivityManager.CONNECTIVITY_ACTION);
193257
ApplicationProvider.getApplicationContext().sendBroadcast(connected);
194258
}
259+
260+
@Override
261+
public void setNetworkPermissionGranted(boolean isGranted) {
262+
shadowConnectivityManager.isNetworkPermissionGranted = isGranted;
263+
}
264+
265+
@Override
266+
public int getRegisteredReceivers() {
267+
Intent connectivity = new Intent(ConnectivityManager.CONNECTIVITY_ACTION);
268+
return shadowOf((Application) ApplicationProvider.getApplicationContext())
269+
.getReceiversForIntent(connectivity)
270+
.size();
271+
}
195272
}
196273

197274
@Implements(ConnectivityManager.class)
198275
public static final class PermissionConnectivityManager extends ShadowConnectivityManager {
199276
private boolean isNetworkPermissionGranted = true;
277+
private boolean isConnected;
278+
279+
@Implementation
280+
@Override
281+
public Network getActiveNetwork() {
282+
if (isConnected) {
283+
return ShadowNetwork.newInstance(1);
284+
} else {
285+
return null;
286+
}
287+
}
288+
289+
@Implementation
290+
@Override
291+
protected void registerDefaultNetworkCallback(NetworkCallback networkCallback) {
292+
if (!isNetworkPermissionGranted) {
293+
throw new SecurityException();
294+
}
295+
super.registerDefaultNetworkCallback(networkCallback);
296+
if (isConnected) {
297+
networkCallback.onAvailable(null);
298+
} else {
299+
networkCallback.onLost(null);
300+
}
301+
}
200302

201303
@Implementation
202304
@Override

0 commit comments

Comments
 (0)
Please sign in to comment.