remove EipFragmentServiceConnection, fixes memory leaks

2 files changed, 14 insertions, 87 deletions

diff --git a/app/src/main/java/se/leap/bitmaskclient/base/fragments/EipFragment.java b/app/src/main/java/se/leap/bitmaskclient/base/fragments/EipFragment.java
index 02feafd3..c5ed7d12 100644
--- a/app/src/main/java/se/leap/bitmaskclient/base/fragments/EipFragment.java
+++ b/app/src/main/java/se/leap/bitmaskclient/base/fragments/EipFragment.java
@@ -16,7 +16,6 @@
  */
 package se.leap.bitmaskclient.base.fragments;
 
-import static de.blinkt.openvpn.core.ConnectionStatus.LEVEL_NONETWORK;
 import static se.leap.bitmaskclient.R.string.vpn_certificate_user_message;
 import static se.leap.bitmaskclient.base.models.Constants.ASK_TO_CANCEL_VPN;
 import static se.leap.bitmaskclient.base.models.Constants.EIP_ACTION_START;
@@ -36,16 +35,12 @@ import static se.leap.bitmaskclient.providersetup.ProviderAPI.UPDATE_INVALID_VPN
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.USER_MESSAGE;
 
 import android.app.Activity;
-import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
-import android.content.ServiceConnection;
 import android.content.SharedPreferences;
 import android.graphics.drawable.Animatable;
-import android.graphics.drawable.Animatable2;
 import android.graphics.drawable.Drawable;
 import android.os.Bundle;
-import android.os.IBinder;
 import android.text.Spannable;
 import android.text.SpannableString;
 import android.text.TextUtils;
@@ -65,17 +60,13 @@ import androidx.core.content.ContextCompat;
 import androidx.fragment.app.DialogFragment;
 import androidx.fragment.app.Fragment;
 import androidx.fragment.app.FragmentTransaction;
-import androidx.vectordrawable.graphics.drawable.Animatable2Compat;
 import androidx.vectordrawable.graphics.drawable.Animatable2Compat.AnimationCallback;
 import androidx.vectordrawable.graphics.drawable.AnimatedVectorDrawableCompat;
 
 import java.util.Observable;
 import java.util.Observer;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 import de.blinkt.openvpn.core.ConnectionStatus;
-import de.blinkt.openvpn.core.IOpenVPNServiceInternal;
-import de.blinkt.openvpn.core.OpenVPNService;
 import de.blinkt.openvpn.core.VpnStatus;
 import de.blinkt.openvpn.core.connection.Connection;
 import se.leap.bitmaskclient.R;
@@ -129,10 +120,6 @@ public class EipFragment extends Fragment implements Observer {
     //------------------------
     AlertDialog alertDialog;
 
-    private IOpenVPNServiceInternal mService;
-    // We use this service connection to detect if openvpn is running without network
-    private EipFragmentServiceConnection openVpnConnection;
-
     @Override
     public void onAttach(Context context) {
         super.onAttach(context);
@@ -166,7 +153,6 @@ public class EipFragment extends Fragment implements Observer {
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
-        openVpnConnection = new EipFragmentServiceConnection();
         eipStatus = EipStatus.getInstance();
         providerObservable = ProviderObservable.getInstance();
         torStatusObservable = TorStatusObservable.getInstance();
@@ -233,16 +219,14 @@ public class EipFragment extends Fragment implements Observer {
     @Override
     public void onResume() {
         super.onResume();
-        if (!eipStatus.isDisconnected()) {
-            openVpnConnection.bindService();
-        }
+        Log.d(TAG, "onResume");
         handleNewState();
     }
 
     @Override
     public void onPause() {
         super.onPause();
-        openVpnConnection.unbindService();
+        Log.d(TAG, "onPause");
     }
 
     @Override
@@ -270,6 +254,8 @@ public class EipFragment extends Fragment implements Observer {
     @Override
     public void onDestroyView() {
         super.onDestroyView();
+        Log.d(TAG, "onDestroyView");
+
         Activity activity = getActivity();
         if (activity != null) {
             ((MainActivity) activity).setDefaultActivityBarColor();
@@ -290,7 +276,7 @@ public class EipFragment extends Fragment implements Observer {
     }
 
     void handleIcon() {
-        if (isOpenVpnRunningWithoutNetwork() || eipStatus.isConnected() || eipStatus.isConnecting() || eipStatus.isUpdatingVpnCert())
+        if (eipStatus.isVPNRunningWithoutNetwork() || eipStatus.isConnected() || eipStatus.isConnecting() || eipStatus.isUpdatingVpnCert())
             handleSwitchOff();
         else
             handleSwitchOn();
@@ -326,7 +312,7 @@ public class EipFragment extends Fragment implements Observer {
     }
 
     private void handleSwitchOff() {
-        if (isOpenVpnRunningWithoutNetwork() || eipStatus.isConnecting() || eipStatus.isUpdatingVpnCert()) {
+        if (eipStatus.isVPNRunningWithoutNetwork() || eipStatus.isConnecting() || eipStatus.isUpdatingVpnCert()) {
             askPendingStartCancellation();
         } else if (eipStatus.isConnected()) {
             askToStopEIP();
@@ -420,13 +406,6 @@ public class EipFragment extends Fragment implements Observer {
             eipStatus = (EipStatus) observable;
             handleNewStateOnMain();
 
-            if (eipStatus.isConnecting()) {
-                openVpnConnection.bindService();
-            }
-            if ("NOPROCESS".equals(EipStatus.getInstance().getState())) {
-                //assure that the Service is shutdown completely if openvpn was stopped
-                openVpnConnection.unbindService();
-            }
         } else if (observable instanceof ProviderObservable) {
             provider = ((ProviderObservable) observable).getCurrentProvider();
         } else if (observable instanceof TorStatusObservable && EipStatus.getInstance().isUpdatingVpnCert()) {
@@ -511,7 +490,7 @@ public class EipFragment extends Fragment implements Observer {
             background.setImageResource(R.drawable.bg_connected);
             animateState(R.drawable.state_connected);
             setActivityBarColor(R.color.bg_running_top, R.color.bg_running_top_light_transparent);
-        } else if(isOpenVpnRunningWithoutNetwork()) {
+        } else if(eipStatus.isVPNRunningWithoutNetwork()) {
             Log.d(TAG, "eip fragment eipStatus - isOpenVpnRunningWithoutNetwork");
             setMainButtonEnabled(true);
             mainButton.updateState(true, false, true);
@@ -614,19 +593,6 @@ public class EipFragment extends Fragment implements Observer {
         }
     }
 
-    private boolean isOpenVpnRunningWithoutNetwork() {
-        boolean isRunning = false;
-        try {
-            isRunning = eipStatus.getLevel() == LEVEL_NONETWORK &&
-                    mService.isVpnRunning();
-        } catch (Exception e) {
-            //eat me
-            e.printStackTrace();
-        }
-
-        return isRunning;
-    }
-
     private void updateInvalidVpnCertificate() {
         eipStatus.setUpdatingVpnCert(true);
         ProviderAPICommand.execute(getContext(), UPDATE_INVALID_VPN_CERTIFICATE, provider);
@@ -646,52 +612,6 @@ public class EipFragment extends Fragment implements Observer {
         }
     }
 
-    private class EipFragmentServiceConnection implements ServiceConnection {
-        private final AtomicBoolean bind = new AtomicBoolean(false);
-
-        void bindService() {
-            Activity activity = getActivity();
-            if (activity == null) {
-                Log.e(TAG, "activity is null when binding OpenVpn");
-                return;
-            }
-            if (!bind.get()) {
-                activity.runOnUiThread(() -> {
-                        Intent intent = new Intent(activity, OpenVPNService.class);
-                        intent.setAction(OpenVPNService.START_SERVICE);
-
-                        activity.bindService(intent, EipFragmentServiceConnection.this, Context.BIND_AUTO_CREATE);
-                        bind.set(true);
-                });
-            }
-        }
-
-        void unbindService() {
-            Activity activity = getActivity();
-            if (activity == null) {
-                return;
-            }
-            if (bind.get()) {
-                activity.runOnUiThread(() -> {
-                    activity.unbindService(EipFragmentServiceConnection.this);
-                    bind.set(false);
-                });
-            }
-        }
-
-        @Override
-        public void onServiceConnected(ComponentName className,
-                                       IBinder service) {
-            mService = IOpenVPNServiceInternal.Stub.asInterface(service);
-            handleNewState();
-        }
-
-        @Override
-        public void onServiceDisconnected(ComponentName arg0) {
-            mService = null;
-        }
-    }
-
     public void showDonationReminderDialog() {
         try {
             FragmentTransaction fragmentTransaction = new FragmentManagerEnhanced(
diff --git a/app/src/main/java/se/leap/bitmaskclient/eip/EipStatus.java b/app/src/main/java/se/leap/bitmaskclient/eip/EipStatus.java
index c9cf6041..9244f531 100644
--- a/app/src/main/java/se/leap/bitmaskclient/eip/EipStatus.java
+++ b/app/src/main/java/se/leap/bitmaskclient/eip/EipStatus.java
@@ -16,6 +16,8 @@
  */
 package se.leap.bitmaskclient.eip;
 
+import static de.blinkt.openvpn.core.ConnectionStatus.LEVEL_NONETWORK;
+
 import android.content.Context;
 import android.os.AsyncTask;
 import androidx.annotation.VisibleForTesting;
@@ -92,6 +94,11 @@ public class EipStatus extends Observable implements VpnStatus.StateListener {
         return "RECONNECTING".equals(currentStatus.getState());
     }
 
+    public boolean isVPNRunningWithoutNetwork() {
+        return currentStatus.getLevel() == LEVEL_NONETWORK &&
+                !"NO_PROCESS".equals(currentStatus.getState());
+    }
+
     private void setEipLevel(ConnectionStatus level) {
         switch (level) {
             case LEVEL_CONNECTED: