ABP/Android preferences UI

src/org/adblockplus/android/Preferences.java

Index: src/org/adblockplus/android/Preferences.java
===================================================================
new file mode 100644
--- /dev/null
+++ b/src/org/adblockplus/android/Preferences.java
@@ -0,0 +1,494 @@
+package org.adblockplus.android;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.Calendar;
+import java.util.Date;
+import java.util.List;
+
+import android.app.ActivityManager;
+import android.app.ActivityManager.RunningServiceInfo;
+import android.app.AlertDialog;
+import android.content.BroadcastReceiver;
+import android.content.Context;
+import android.content.DialogInterface;
+import android.content.DialogInterface.OnDismissListener;
+import android.content.Intent;
+import android.content.IntentFilter;
+import android.content.SharedPreferences;
+import android.content.pm.PackageManager.NameNotFoundException;
+import android.content.res.AssetManager;
+import android.net.Uri;
+import android.os.Build;
+import android.os.Bundle;
+import android.preference.CheckBoxPreference;
+import android.preference.ListPreference;
+import android.preference.PreferenceManager;
+import android.preference.PreferenceScreen;
+import android.text.format.DateFormat;
+import android.util.Log;
+import android.view.Menu;
+import android.view.MenuInflater;
+import android.view.MenuItem;
+import android.view.View;
+import android.view.Window;
+import android.widget.TextView;
+
+public class Preferences extends SummarizedPreferences
+{
+	private final static String TAG = "Preferences";
+
+	private AboutDialog aboutDialog;
+	private boolean showAbout = false;
+	private String configurationMsg;
+	private String subscriptionSummary;
+
+	@Override
+	public void onCreate(Bundle savedInstanceState)
+	{
+		requestWindowFeature(Window.FEATURE_NO_TITLE);
+
+		super.onCreate(savedInstanceState);
+
+		PreferenceManager.setDefaultValues(this, R.xml.preferences, false);
+		setContentView(R.layout.preferences);
+		addPreferencesFromResource(R.xml.preferences);
+
+		SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
+
+		int lastVersion = prefs.getInt(getString(R.string.pref_version), 0);
+		try
+		{
+			int thisVersion = getPackageManager().getPackageInfo(getPackageName(), 0).versionCode;
+			if (lastVersion != thisVersion)
+			{
+				copyAssets();
+				SharedPreferences.Editor editor = prefs.edit();
+				editor.putInt(getString(R.string.pref_version), thisVersion);
+				editor.commit();
+			}
+		}
+		catch (NameNotFoundException e)
+		{
+			copyAssets();
+		}
+	}
+
+	@Override
+	protected void onStart()
+	{
+		super.onStart();
+		AdblockPlus.getApplication().startEngine();

[Felix Dahlke, 2012/09/18 15:32:49]

Since you're using the result of getApplication() twice, it would be a good idea
to assign it to a temporary variable and call getApplication() just once.
Probably won't be an issue, but you can't be sure that the result won't be
different unless you go and inspect getApplication().

+		AdblockPlus.getApplication().startInteractive();
+	}
+
+	@Override
+	public void onResume()

[Felix Dahlke, 2012/09/18 15:32:49]

I'd love it if you could move some of this method's code into helper methods.
It's a bit hard for me to wrap my head around long methods.

+	{
+		super.onResume();
+		SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
+
+		final AdblockPlus application = AdblockPlus.getApplication();
+
+		RefreshableListPreference subscriptionList = (RefreshableListPreference) findPreference(getString(R.string.pref_subscription));
+		List<Subscription> subscriptions = application.getSubscriptions();
+		String[] entries = new String[subscriptions.size()];

[Felix Dahlke, 2012/09/18 15:32:49]

Like above, I'd appreciate it if you call subscriptions.size() just once.

+		String[] entryValues = new String[subscriptions.size()];
+		String current = prefs.getString(getString(R.string.pref_subscription), (String) null);
+		int i = 0;
+		for (Subscription subscription : subscriptions)
+		{
+			entries[i] = subscription.title;
+			entryValues[i] = subscription.url;
+			i++;
+		}
+		subscriptionList.setEntries(entries);
+		subscriptionList.setEntryValues(entryValues);
+
+		boolean firstRun = false;
+
+		if (current == null)
+		{
+			firstRun = true;
+			Subscription offer = application.offerSubscription();
+			current = offer.url;
+			if (offer != null)
+			{
+				subscriptionList.setValue(offer.url);
+				application.setSubscription(offer);
+				new AlertDialog.Builder(this)
+					.setTitle(R.string.app_name)
+					.setMessage(String.format(getString(R.string.msg_subscription_offer, offer.title)))
+					.setIcon(android.R.drawable.ic_dialog_info)
+					.setPositiveButton(R.string.ok, null)
+					.create()
+					.show();
+			}
+		}
+
+		subscriptionList.setOnRefreshClickListener(new View.OnClickListener() {
+			@Override
+			public void onClick(View v)
+			{
+				application.refreshSubscription();
+			}
+		});
+
+		if (subscriptionSummary != null)
+			subscriptionList.setSummary(subscriptionSummary);
+		else
+			setPrefSummary(subscriptionList);
+
+		registerReceiver(receiver, new IntentFilter(AdblockPlus.BROADCAST_SUBSCRIPTION_STATUS));
+		registerReceiver(receiver, new IntentFilter(AdblockPlus.BROADCAST_FILTER_MATCHES));
+		registerReceiver(receiver, new IntentFilter(ProxyService.BROADCAST_STATE_CHANGED));
+		registerReceiver(receiver, new IntentFilter(ProxyService.BROADCAST_PROXY_FAILED));
+
+		final String url = current;
+
+		(new Thread() {
+			@Override
+			public void run()
+			{
+				if (!application.verifySubscriptions())
+				{
+					Subscription subscription = application.getSubscription(url);
+					application.setSubscription(subscription);
+				}
+			}
+		}).start();
+
+		boolean enabled = prefs.getBoolean(getString(R.string.pref_enabled), false);
+		if (enabled && !isServiceRunning())
+		{
+			setEnabled(false);
+			enabled = false;
+		}
+		else if (! enabled && firstRun)

[Felix Dahlke, 2012/09/18 15:32:49]

I think "!enabled" read better than "! enabled". I know, nit picking :)

+		{
+			startService(new Intent(this, ProxyService.class));
+			setEnabled(true);
+		}
+
+		if (configurationMsg != null)
+			showConfigurationMsg(configurationMsg);
+		
+		if (showAbout)
+			onAbout(findViewById(R.id.btn_about));
+	}
+
+	@Override
+	public void onPause()
+	{
+		super.onPause();
+		unregisterReceiver(receiver);
+	}
+
+	@Override
+	protected void onStop()
+	{
+		super.onStop();
+		SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
+		boolean enabled = prefs.getBoolean(getString(R.string.pref_enabled), false);
+		AdblockPlus.getApplication().stopInteractive();
+		if (!enabled)
+			AdblockPlus.getApplication().stopEngine(true);

[Felix Dahlke, 2012/09/18 15:32:49]

Double getApplication() again, see above.

+		
+		if (aboutDialog != null)
+			aboutDialog.dismiss();
+	}
+
+	@Override
+	public boolean onCreateOptionsMenu(Menu menu)
+	{
+		MenuInflater inflater = getMenuInflater();
+		inflater.inflate(R.menu.menu_preferences, menu);
+		return true;
+	}
+
+	@Override
+	public boolean onOptionsItemSelected(MenuItem item)
+	{
+		switch (item.getItemId())
+		{
+			case R.id.menu_advanced:
+				Class<?> activity = Preferences.InnerPreferences.class;
+				startActivity(new Intent(Preferences.this, activity).putExtra("KEY", "preferences_advanced"));
+				return true;
+			default:
+				return super.onOptionsItemSelected(item);
+		}
+	}
+
+	private void setEnabled(boolean enabled)
+	{
+		SharedPreferences.Editor editor = PreferenceManager.getDefaultSharedPreferences(this).edit();
+		editor.putBoolean(getString(R.string.pref_enabled), enabled);
+		editor.commit();
+		((CheckBoxPreference) findPreference(getString(R.string.pref_enabled))).setChecked(enabled);
+	}
+
+	private boolean isServiceRunning()
+	{
+		ActivityManager manager = (ActivityManager) getSystemService(ACTIVITY_SERVICE);
+		for (RunningServiceInfo service : manager.getRunningServices(Integer.MAX_VALUE))
+		{
+			if ("org.adblockplus.android.ProxyService".equals(service.service.getClassName()))
+				return true;
+		}
+		return false;
+	}
+
+	private void copyAssets()
+	{
+		AssetManager assetManager = getAssets();
+		String[] files = null;
+		try
+		{
+			files = assetManager.list("install");
+		}
+		catch (IOException e)
+		{
+			Log.e(TAG, e.getMessage());
+		}
+		for (int i = 0; i < files.length; i++)
+		{
+			InputStream in = null;
+			OutputStream out = null;
+			try
+			{
+				Log.d(TAG, "Copy: install/" + files[i]);
+				in = assetManager.open("install/" + files[i]);
+				out = openFileOutput(files[i], MODE_PRIVATE);
+				byte[] buffer = new byte[1024];
+				int read;
+				while ((read = in.read(buffer)) != -1)
+				{
+					out.write(buffer, 0, read);
+				}
+				in.close();
+				in = null;

[Felix Dahlke, 2012/09/18 15:32:49]

You don't need to set in to null, it's going out of scope for the next iteration
of the for loop. Same with out below.

+				out.flush();
+				out.close();
+				out = null;
+
+			}
+			catch (Exception e)
+			{
+				Log.e(TAG, "Asset copy error", e);
+			}
+		}
+	}
+	
+	public void onHelp(View view)
+	{
+		Uri uri = Uri.parse(getString(R.string.configuring_url));
+		final Intent intent = new Intent(Intent.ACTION_VIEW, uri);
+		startActivity(intent);
+	}
+
+	public void onAbout(View view)
+	{
+		aboutDialog = new AboutDialog(this);
+		aboutDialog.setOnDismissListener(new OnDismissListener() {
+
+			@Override
+			public void onDismiss(DialogInterface dialog)
+			{
+				showAbout = false;
+				aboutDialog = null;
+			}
+		});
+		showAbout = true;
+		aboutDialog.show();
+	}
+
+	@Override
+	public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, String key)
+	{
+		if (getString(R.string.pref_enabled).equals(key))
+		{
+			boolean enabled = sharedPreferences.getBoolean(key, false);
+			if (enabled && !isServiceRunning())
+				startService(new Intent(this, ProxyService.class));
+			else if (!enabled && isServiceRunning())
+				stopService(new Intent(this, ProxyService.class));
+		}
+		if (getString(R.string.pref_subscription).equals(key))
+		{
+			String current = sharedPreferences.getString(key, null);
+			AdblockPlus application = AdblockPlus.getApplication();
+			Subscription subscription = application.getSubscription(current);
+			application.setSubscription(subscription);
+		}
+		if (getString(R.string.pref_refresh).equals(key))
+		{
+			int refresh = Integer.valueOf(sharedPreferences.getString(getString(R.string.pref_refresh), "0"));
+			findPreference(getString(R.string.pref_wifirefresh)).setEnabled(refresh > 0);
+		}
+		if (getString(R.string.pref_crashreport).equals(key))
+		{
+			AdblockPlus application = AdblockPlus.getApplication();
+			application.updateCrashReportStatus();			
+		}
+		super.onSharedPreferenceChanged(sharedPreferences, key);
+	}
+
+	private void showConfigurationMsg(String message)
+	{
+		TextView msg = (TextView) findViewById(R.id.txt_configuration);
+		msg.setText(message);
+		msg.setVisibility(View.VISIBLE);
+		configurationMsg = message;
+	}
+
+	private void hideConfigurationMsg()
+	{
+		if (configurationMsg == null)
+			return;
+		TextView msg = (TextView) findViewById(R.id.txt_configuration);
+		msg.setVisibility(View.GONE);
+		configurationMsg = null;
+	}
+
+	private BroadcastReceiver receiver = new BroadcastReceiver() {
+		@Override
+		public void onReceive(final Context context, Intent intent)

[Felix Dahlke, 2012/09/18 15:32:49]

Another somewhat large method, can you split it up a bit?

+		{
+			String action = intent.getAction();
+			Bundle extra = intent.getExtras();
+			if (action.equals(ProxyService.BROADCAST_STATE_CHANGED))
+			{
+				if (extra.getBoolean("enabled"))
+				{
+					if (extra.getBoolean("manual"))
+					{
+						showConfigurationMsg(getString(R.string.msg_configuration, extra.getInt("port")));

[Felix Dahlke, 2012/09/18 15:32:49]

This string is used in multiple places, how about a constant?

+					}
+				}
+				else
+				{
+					setEnabled(false);
+					hideConfigurationMsg();
+				}
+			}
+			if (action.equals(AdblockPlus.BROADCAST_FILTER_MATCHES))
+			{
+				hideConfigurationMsg();
+			}
+			if (action.equals(ProxyService.BROADCAST_PROXY_FAILED))
+			{
+				String msg = extra.getString("msg");
+				new AlertDialog.Builder(Preferences.this).setTitle(R.string.error).setMessage(msg).setIcon(android.R.drawable.ic_dialog_alert).setPositiveButton(R.string.ok, null).create().show();
+				setEnabled(false);
+			}
+			if (action.equals(AdblockPlus.BROADCAST_SUBSCRIPTION_STATUS))
+			{
+				final String text = extra.getString("text");
+				final long time = extra.getLong("time");
+				runOnUiThread(new Runnable() {
+					public void run()
+					{
+						ListPreference subscriptionList = (ListPreference) findPreference(getString(R.string.pref_subscription));
+						CharSequence summary = subscriptionList.getEntry();
+						StringBuilder builder = new StringBuilder();
+						if (summary != null)
+						{
+							builder.append(summary);
+							if (text != "")
+							{
+								builder.append(" (");
+								int id = getResources().getIdentifier(text, "string", getPackageName());
+								if (id > 0)

[Felix Dahlke, 2012/09/18 15:32:49]

How about using ?: here? Would safe three lines. I don't mind if you'd rather
not :)

+									builder.append(getString(id, text));
+								else
+									builder.append(text);
+								if (time > 0)
+								{
+									builder.append(": ");

[Felix Dahlke, 2012/09/18 15:32:49]

I think 8 nesting levels are too much. Can you refactor this?

+									Calendar calendar = Calendar.getInstance();
+									calendar.setTimeInMillis(time);
+									Date date = calendar.getTime();
+									builder.append(DateFormat.getDateFormat(context).format(date));
+									builder.append(" ");
+									builder.append(DateFormat.getTimeFormat(context).format(date));
+								}
+								builder.append(")");
+							}
+							subscriptionSummary = builder.toString();
+							subscriptionList.setSummary(subscriptionSummary);
+						}
+					}
+				});
+			}
+		}
+	};
+
+	public static class InnerPreferences extends SummarizedPreferences
+	{
+		@Override
+		public void onCreate(Bundle savedInstanceState)
+		{
+			super.onCreate(savedInstanceState);
+
+			String key = getIntent().getExtras().getString("KEY");
+			int res = getResources().getIdentifier(key, "xml", getPackageName());
+
+			addPreferencesFromResource(res);
+
+			PreferenceScreen screen = getPreferenceScreen();
+			if (Build.VERSION.SDK_INT >= 12) // Honeycomb 3.1
+			{
+				screen.removePreference(findPreference(getString(R.string.pref_proxy)));
+			}
+			if (getResources().getBoolean(R.bool.def_release))
+			{
+				screen.removePreference(findPreference(getString(R.string.pref_support)));
+			}
+		}
+
+		@Override
+		public void onResume()
+		{
+			super.onResume();
+			SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
+			int refresh = Integer.valueOf(prefs.getString(getString(R.string.pref_refresh), "0"));
+			findPreference(getString(R.string.pref_wifirefresh)).setEnabled(refresh > 0);
+		}
+
+		@Override
+		public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, String key)
+		{
+			if (getString(R.string.pref_refresh).equals(key))
+			{
+				int refresh = Integer.valueOf(sharedPreferences.getString(getString(R.string.pref_refresh), "0"));
+				findPreference(getString(R.string.pref_wifirefresh)).setEnabled(refresh > 0);
+			}
+			if (getString(R.string.pref_crashreport).equals(key))
+			{
+				AdblockPlus application = AdblockPlus.getApplication();
+				application.updateCrashReportStatus();
+			}
+			super.onSharedPreferenceChanged(sharedPreferences, key);
+		}
+	}
+
+	@Override
+	protected void onRestoreInstanceState(Bundle state)
+	{
+		super.onRestoreInstanceState(state);
+		showAbout = state.getBoolean("showAbout");
+		configurationMsg = state.getString("configurationMsg");
+		subscriptionSummary = state.getString("subscriptionSummary");
+	}
+
+	@Override
+	protected void onSaveInstanceState(Bundle outState)
+	{
+		outState.putString("subscriptionSummary", subscriptionSummary);
+		outState.putString("configurationMsg", configurationMsg);
+		outState.putBoolean("showAbout", showAbout);
+		super.onSaveInstanceState(outState);
+	}
+}