CaptivePortalLogin correctly unregisters callbacks
The NetworkCallback registered by the CaptivePortalLogin activity in
onCreate was unregistered in both onDestroy() and done(). In addition
done() can be called concurrently from different places (from the
webview, from the captive portal test probe, from the activity menu),
resulting in incorrectly unregistering the callback more than once.
This patch fixes the lifecycle management of the NetworkCallback
registered by the CaptivePortalLogin activity so that it is unregistered
once only in onDestroy.
In addition the done() method is made robust against multiple calls and
becomes a no-op after the first call. This avoids multiple calls
to CaptivePortal for the same captive portal.
Bug: 62497809
Test: tested manually with captive portal setup in the office
Change-Id: I77fbeb55cf91d3b44e91d2fecb800dae40279652
diff --git a/packages/CaptivePortalLogin/src/com/android/captiveportallogin/CaptivePortalLoginActivity.java b/packages/CaptivePortalLogin/src/com/android/captiveportallogin/CaptivePortalLoginActivity.java
index 71d9fc8..7480ad1 100644
--- a/packages/CaptivePortalLogin/src/com/android/captiveportallogin/CaptivePortalLoginActivity.java
+++ b/packages/CaptivePortalLogin/src/com/android/captiveportallogin/CaptivePortalLoginActivity.java
@@ -58,6 +58,7 @@
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.Random;
+import java.util.concurrent.atomic.AtomicBoolean;
public class CaptivePortalLoginActivity extends Activity {
private static final String TAG = CaptivePortalLoginActivity.class.getSimpleName();
@@ -83,6 +84,8 @@
private ConnectivityManager mCm;
private boolean mLaunchBrowser = false;
private MyWebViewClient mWebViewClient;
+ // Ensures that done() happens once exactly, handling concurrent callers with atomic operations.
+ private final AtomicBoolean isDone = new AtomicBoolean(false);
@Override
protected void onCreate(Bundle savedInstanceState) {
@@ -179,13 +182,13 @@
}
private void done(Result result) {
+ if (isDone.getAndSet(true)) {
+ // isDone was already true: done() already called
+ return;
+ }
if (DBG) {
Log.d(TAG, String.format("Result %s for %s", result.name(), mUrl.toString()));
}
- if (mNetworkCallback != null) {
- mCm.unregisterNetworkCallback(mNetworkCallback);
- mNetworkCallback = null;
- }
logMetricsEvent(result.metricsEvent);
switch (result) {
case DISMISSED:
@@ -245,8 +248,8 @@
public void onDestroy() {
super.onDestroy();
if (mNetworkCallback != null) {
+ // mNetworkCallback is not null if mUrl is not null.
mCm.unregisterNetworkCallback(mNetworkCallback);
- mNetworkCallback = null;
}
if (mLaunchBrowser) {
// Give time for this network to become default. After 500ms just proceed.