From e33adc4f6662168ec5fee6e70f7222357abd6625 Mon Sep 17 00:00:00 2001 From: Mygod Date: Wed, 28 Dec 2016 01:45:23 +0800 Subject: [PATCH] Seriously clean up Shadowsocks MainActivity This activity contains lots of obsolete code and it's time to do a cleanup. --- src/main/res/layout/layout_main.xml | 6 +- .../com/github/shadowsocks/MainActivity.scala | 234 ++++++------------ .../github/shadowsocks/ProfilesFragment.scala | 2 +- .../shadowsocks/ServiceBoundContext.scala | 18 +- .../github/shadowsocks/utils/Constants.scala | 7 +- 5 files changed, 101 insertions(+), 166 deletions(-) diff --git a/src/main/res/layout/layout_main.xml b/src/main/res/layout/layout_main.xml index c9ebbb4b..1ca4a71d 100644 --- a/src/main/res/layout/layout_main.xml +++ b/src/main/res/layout/layout_main.xml @@ -75,11 +75,13 @@ android:layout_alignParentEnd="true" android:layout_marginEnd="16dp" android:layout_marginTop="@dimen/fab_margin_top"> - + app:pressedTranslationZ="6dp" + app:borderWidth="0dp"/> { - s match { - case State.CONNECTING => - fab.setBackgroundTintList(greyTint) - fab.setImageResource(R.drawable.ic_start_busy) - fab.setEnabled(false) - fabProgressCircle.show() - statusText.setText("Connecting...") - case State.CONNECTED => - fab.setBackgroundTintList(greenTint) - if (state == State.CONNECTING) { - fabProgressCircle.beginFinalAnimation() - } else { - fabProgressCircle.postDelayed(hideCircle, 1000) - } - fab.setEnabled(true) - changeSwitch(checked = true) - statusText.setText(if (app.isNatEnabled) "Connected" else "Connected, tap to check connection") - case State.STOPPED => - fab.setBackgroundTintList(greyTint) - fabProgressCircle.postDelayed(hideCircle, 1000) - fab.setEnabled(true) - changeSwitch(checked = false) - if (m != null) { - val snackbar = Snackbar.make(findViewById(R.id.snackbar), - getString(R.string.vpn_error).formatLocal(Locale.ENGLISH, m), Snackbar.LENGTH_LONG) - if (m == getString(R.string.nat_no_root)) addDisableNatToSnackbar(snackbar) - snackbar.show() - Log.e(TAG, "Error to start VPN service: " + m) - } - statusText.setText("Not connected") - case State.STOPPING => - fab.setBackgroundTintList(greyTint) - fab.setImageResource(R.drawable.ic_start_busy) - fab.setEnabled(false) - if (state == State.CONNECTED) fabProgressCircle.show() // ignore for stopped - statusText.setText("Shutting down...") - } - state = s - }) - } - def trafficUpdated(txRate: Long, rxRate: Long, txTotal: Long, rxTotal: Long) { + def stateChanged(s: Int, profileName: String, m: String): Unit = handler.post(() => changeState(s, profileName, m)) + def trafficUpdated(txRate: Long, rxRate: Long, txTotal: Long, rxTotal: Long): Unit = handler.post(() => updateTraffic(txRate, rxRate, txTotal, rxTotal)) - } } + private lazy val greyTint = ContextCompat.getColorStateList(MainActivity.this, R.color.material_primary_500) + private lazy val greenTint = ContextCompat.getColorStateList(MainActivity.this, R.color.material_green_700) + private def hideCircle() = try fabProgressCircle.hide() catch { + case _: NullPointerException => + } + private def changeState(s: Int, profileName: String = null, m: String = null) { + // TODO: localize texts for statusText in this method + s match { + case State.CONNECTING => + fab.setImageResource(R.drawable.ic_start_busy) + fabProgressCircle.show() + statusText.setText("Connecting...") + case State.CONNECTED => + if (state == State.CONNECTING) fabProgressCircle.beginFinalAnimation() + else fabProgressCircle.postDelayed(hideCircle, 1000) + fab.setImageResource(R.drawable.ic_start_connected) + statusText.setText(if (app.isNatEnabled) "Connected" else "Connected, tap to check connection") + case State.STOPPING => + fab.setImageResource(R.drawable.ic_start_busy) + if (state == State.CONNECTED) fabProgressCircle.show() // ignore for stopped + statusText.setText("Shutting down...") + case _ => + fab.setImageResource(R.drawable.ic_start_idle) + fabProgressCircle.postDelayed(hideCircle, 1000) + if (m != null) { + val snackbar = Snackbar.make(findViewById(R.id.snackbar), + getString(R.string.vpn_error).formatLocal(Locale.ENGLISH, m), Snackbar.LENGTH_LONG) + if (m == getString(R.string.nat_no_root)) addDisableNatToSnackbar(snackbar) + snackbar.show() + Log.e(TAG, "Error to start VPN service: " + m) + } + statusText.setText("Not connected") + } + state = s + if (state == State.CONNECTED) fab.setBackgroundTintList(greenTint) else { + fab.setBackgroundTintList(greyTint) + updateTraffic(0, 0, 0, 0) + } + fab.setEnabled(false) + if (state == State.CONNECTED || state == State.STOPPED) + handler.postDelayed(() => fab.setEnabled(state == State.CONNECTED || state == State.STOPPED), 1000) + } def updateTraffic(txRate: Long, rxRate: Long, txTotal: Long, rxTotal: Long) { txText.setText(TrafficMonitor.formatTraffic(txTotal)) rxText.setText(TrafficMonitor.formatTraffic(rxTotal)) @@ -137,67 +141,28 @@ class MainActivity extends Activity with ServiceBoundContext with Drawer.OnDrawe if (child != null) child.onTrafficUpdated(txRate, rxRate, txTotal, rxTotal) } - def attachServiceCallback(): Unit = attachService(callback) - override def onServiceConnected() { - // Update the UI - if (fab != null) fab.setEnabled(true) - Log.d(TAG, "bgService " + bgService.getState) - callback.stateChanged(bgService.getState, null, null) - state = bgService.getState + changeState(bgService.getState) if (Build.VERSION.SDK_INT >= 21 && app.isNatEnabled) { val snackbar = Snackbar.make(findViewById(R.id.snackbar), R.string.nat_deprecated, Snackbar.LENGTH_LONG) addDisableNatToSnackbar(snackbar) snackbar.show() } } + override def onServiceDisconnected(): Unit = changeState(State.IDLE) private def addDisableNatToSnackbar(snackbar: Snackbar) = snackbar.setAction(R.string.switch_to_vpn, (_ => if (state == State.STOPPED) app.editor.putBoolean(Key.isNAT, false)): View.OnClickListener) - override def onServiceDisconnected() { - if (fab != null) fab.setEnabled(false) - } - override def binderDied() { detachService() app.crashRecovery() - attachServiceCallback() + attachService(callback) } - private var testCount: Int = _ - private var statusText: TextView = _ // TODO: localize texts for statusText in this file - private var txText: TextView = _ - private var rxText: TextView = _ - private var txRateText: TextView = _ - private var rxRateText: TextView = _ - private lazy val greyTint = ContextCompat.getColorStateList(this, R.color.material_primary_500) - private lazy val greenTint = ContextCompat.getColorStateList(this, R.color.material_green_700) - - val handler = new Handler() - private val connectedListener: BroadcastReceiver = (_, _) => - statusText.setText(if (app.isNatEnabled) "Connected" else "Connected, tap to check connection") - - private def changeSwitch(checked: Boolean) { - serviceStarted = checked - fab.setImageResource(if (checked) R.drawable.ic_start_connected else R.drawable.ic_start_idle) - if (fab.isEnabled) { - fab.setEnabled(false) - handler.postDelayed(() => fab.setEnabled(true), 1000) - } - } - - def prepareStartService() { - Utils.ThrowableFuture { - if (app.isNatEnabled) serviceLoad() else { - val intent = VpnService.prepare(this) - if (intent != null) { - startActivityForResult(intent, REQUEST_CONNECT) - } else { - handler.post(() => onActivityResult(REQUEST_CONNECT, Activity.RESULT_OK, null)) - } - } - } + override def onActivityResult(requestCode: Int, resultCode: Int, data: Intent): Unit = resultCode match { + case Activity.RESULT_OK => bgService.use(app.profileId) + case _ => Log.e(TAG, "Failed to start VpnService") } override def onCreate(savedInstanceState: Bundle) { @@ -276,7 +241,7 @@ class MainActivity extends Activity with ServiceBoundContext with Drawer.OnDrawe success = false result = getString(R.string.connection_test_error, e.getMessage) } - synchronized(if (serviceStarted && testCount == id && app.isVpnEnabled) handler.post(() => + synchronized(if (state == State.CONNECTED && testCount == id && app.isVpnEnabled) handler.post(() => if (success) statusText.setText(result) else { statusText.setText(R.string.connection_test_fail) Snackbar.make(findViewById(R.id.snackbar), result, Snackbar.LENGTH_LONG).show() @@ -288,19 +253,22 @@ class MainActivity extends Activity with ServiceBoundContext with Drawer.OnDrawe fab = findViewById(R.id.fab).asInstanceOf[FloatingActionButton] fabProgressCircle = findViewById(R.id.fabProgressCircle).asInstanceOf[FABProgressCircle] - fab.setOnClickListener(_ => if (serviceStarted) serviceStop() - else if (bgService != null) prepareStartService() - else changeSwitch(checked = false)) - fab.setOnLongClickListener((v: View) => { - Utils.positionToast(Toast.makeText(this, if (serviceStarted) R.string.stop else R.string.connect, + fab.setOnClickListener(_ => if (state == State.CONNECTED) bgService.use(-1) else Utils.ThrowableFuture { + if (app.isNatEnabled) bgService.use(app.profileId) else { + val intent = VpnService.prepare(this) + if (intent != null) startActivityForResult(intent, REQUEST_CONNECT) + else handler.post(() => onActivityResult(REQUEST_CONNECT, Activity.RESULT_OK, null)) + } + }) + fab.setOnLongClickListener(_ => { + Utils.positionToast(Toast.makeText(this, if (state == State.CONNECTED) R.string.stop else R.string.connect, Toast.LENGTH_SHORT), fab, getWindow, 0, Utils.dpToPx(this, 8)).show() true }) - updateTraffic(0, 0, 0, 0) - handler.post(attachServiceCallback) + changeState(State.IDLE) // reset everything to init state + handler.post(() => attachService(callback)) app.settings.registerOnSharedPreferenceChangeListener(this) - registerReceiver(connectedListener, new IntentFilter(Action.CONNECTED)) val intent = getIntent if (intent != null) handleShareIntent(intent) @@ -342,7 +310,7 @@ class MainActivity extends Activity with ServiceBoundContext with Drawer.OnDrawe def onSharedPreferenceChanged(pref: SharedPreferences, key: String): Unit = key match { case Key.isNAT => handler.post(() => { detachService() - attachServiceCallback() + attachService(callback) }) case _ => } @@ -358,8 +326,11 @@ class MainActivity extends Activity with ServiceBoundContext with Drawer.OnDrawe case DRAWER_PROFILES => displayFragment(profilesFragment) case DRAWER_RECOVERY => app.track("GlobalConfigFragment", "reset") - serviceStop() - val handler = showProgress(R.string.recovering) + if (bgService != null) bgService.use(-1) + val dialog = ProgressDialog.show(this, "", getString(R.string.recovering), true, false) + val handler = new Handler { + override def handleMessage(msg: Message): Unit = if (dialog.isShowing && !isDestroyed) dialog.dismiss() + } Utils.ThrowableFuture { app.copyAssets() handler.sendEmptyMessage(0) @@ -372,29 +343,6 @@ class MainActivity extends Activity with ServiceBoundContext with Drawer.OnDrawe true // unexpected cases will throw exception } - private def showProgress(msg: Int): Handler = { - clearDialog() - progressDialog = ProgressDialog.show(this, "", getString(msg), true, false) - new Handler { - override def handleMessage(msg: Message): Unit = clearDialog() - } - } - - private def clearDialog() { - if (progressDialog != null && progressDialog.isShowing) { - if (!isDestroyed) progressDialog.dismiss() - progressDialog = null - } - } - - private def hideCircle() { - try { - fabProgressCircle.hide() - } catch { - case _: java.lang.NullPointerException => // Ignore - } - } - protected override def onResume() { super.onResume() app.refreshContainerHolder() @@ -418,31 +366,9 @@ class MainActivity extends Activity with ServiceBoundContext with Drawer.OnDrawe override def onDestroy() { super.onDestroy() - unregisterReceiver(connectedListener) app.settings.unregisterOnSharedPreferenceChangeListener(this) detachService() new BackupManager(this).dataChanged() handler.removeCallbacksAndMessages(null) } - - override def onActivityResult(requestCode: Int, resultCode: Int, data: Intent): Unit = resultCode match { - case Activity.RESULT_OK => - serviceLoad() - case _ => - changeSwitch(checked = false) - Log.e(TAG, "Failed to start VpnService") - } - - def serviceStop() { - if (bgService != null) bgService.use(-1) - } - - /** Called when connect button is clicked. */ - def serviceLoad() { - bgService.use(app.profileId) - - if (app.isVpnEnabled) { - changeSwitch(checked = false) - } - } } diff --git a/src/main/scala/com/github/shadowsocks/ProfilesFragment.scala b/src/main/scala/com/github/shadowsocks/ProfilesFragment.scala index b8f6bda2..6abb9a70 100644 --- a/src/main/scala/com/github/shadowsocks/ProfilesFragment.scala +++ b/src/main/scala/com/github/shadowsocks/ProfilesFragment.scala @@ -138,7 +138,7 @@ final class ProfilesFragment extends ToolbarFragment with Toolbar.OnMenuItemClic app.switchProfile(item.id) profilesAdapter.refreshId(old) bind(item) - if (state == State.CONNECTED) activity.serviceLoad() + if (state == State.CONNECTED) activity.bgService.use(item.id) // reconnect to new profile } } diff --git a/src/main/scala/com/github/shadowsocks/ServiceBoundContext.scala b/src/main/scala/com/github/shadowsocks/ServiceBoundContext.scala index 2be868a1..da0d62da 100644 --- a/src/main/scala/com/github/shadowsocks/ServiceBoundContext.scala +++ b/src/main/scala/com/github/shadowsocks/ServiceBoundContext.scala @@ -30,7 +30,7 @@ import com.github.shadowsocks.ShadowsocksApplication.app * @author Mygod */ trait ServiceBoundContext extends Context with IBinder.DeathRecipient { - class ShadowsocksServiceConnection extends ServiceConnection { + private class ShadowsocksServiceConnection extends ServiceConnection { override def onServiceConnected(name: ComponentName, service: IBinder) { binder = service service.linkToDeath(ServiceBoundContext.this, 0) @@ -52,7 +52,7 @@ trait ServiceBoundContext extends Context with IBinder.DeathRecipient { } } - def setListeningForBandwidth(value: Boolean) { + protected def setListeningForBandwidth(value: Boolean) { if (listeningForBandwidth != value && bgService != null && callback != null) if (value) bgService.startListeningForBandwidth(callback) else bgService.stopListeningForBandwidth(callback) listeningForBandwidth = value @@ -66,8 +66,11 @@ trait ServiceBoundContext extends Context with IBinder.DeathRecipient { callbackRegistered = false } - def onServiceConnected(): Unit = () - def onServiceDisconnected(): Unit = () + protected def onServiceConnected(): Unit = () + /** + * Different from Android framework, this method will be called even when you call `detachService`. + */ + protected def onServiceDisconnected(): Unit = () override def binderDied(): Unit = () private var callback: IShadowsocksServiceCallback.Stub = _ @@ -76,10 +79,10 @@ trait ServiceBoundContext extends Context with IBinder.DeathRecipient { private var listeningForBandwidth: Boolean = _ // Variables - var binder: IBinder = _ + private var binder: IBinder = _ var bgService: IShadowsocksService = _ - def attachService(callback: IShadowsocksServiceCallback.Stub = null) { + protected def attachService(callback: IShadowsocksServiceCallback.Stub = null) { this.callback = callback if (bgService == null) { val s = if (app.isNatEnabled) classOf[ShadowsocksNatService] else classOf[ShadowsocksVpnService] @@ -92,8 +95,9 @@ trait ServiceBoundContext extends Context with IBinder.DeathRecipient { } } - def detachService() { + protected def detachService() { unregisterCallback() + onServiceDisconnected() callback = null if (connection != null) { try unbindService(connection) catch { diff --git a/src/main/scala/com/github/shadowsocks/utils/Constants.scala b/src/main/scala/com/github/shadowsocks/utils/Constants.scala index 1da063a5..f7bad735 100644 --- a/src/main/scala/com/github/shadowsocks/utils/Constants.scala +++ b/src/main/scala/com/github/shadowsocks/utils/Constants.scala @@ -162,17 +162,20 @@ object Key { } object State { + /** + * This state will never be broadcast by the service. This state is only used to indicate that the current context + * hasn't bound to any context. + */ + val IDLE = 0 val CONNECTING = 1 val CONNECTED = 2 val STOPPING = 3 val STOPPED = 4 - def isAvailable(state: Int): Boolean = state != CONNECTED && state != CONNECTING } object Action { final val SERVICE = "com.github.shadowsocks.SERVICE" final val CLOSE = "com.github.shadowsocks.CLOSE" - final val CONNECTED = "com.github.shadowsocks.CONNECTED" // TODO final val PROFILE_CHANGED = "com.github.shadowsocks.PROFILE_CHANGED" final val PROFILE_REMOVED = "com.github.shadowsocks.PROFILE_REMOVED"