Change DownloadHandler from using runOnUiThread() to send/handleMessage().

It still executes the download on another thread, and receives progress
events on that thread. However it now posts those events to the main UI
thread for them to get handled.

Also refactored Downloader/HttpDownloader a tad further. Some caching
stuff was pushed down from HttpDownloader -> Downloader (renamed from
eTag to just cacheTag). HttpDownloader.download() was recursively calling
itself, so I made the base class download() method abstract, and instead
the stream downloading is done in "protected void downloadFromStream()"

Other minor stuff:
 * Prefixed some log TAGs with package names, to make them easier to grep.
 * Removed a little bit of unused code (parameter to DownloadHandler).
This commit is contained in:
Peter Serwylo 2014-02-28 09:11:34 +11:00
parent 91f2f1014e
commit 0e0e042cd0
4 changed files with 87 additions and 69 deletions

View File

@ -38,6 +38,7 @@ import android.graphics.Bitmap;
import android.net.Uri;
import android.os.Bundle;
import android.os.Handler;
import android.os.Message;
import android.preference.PreferenceManager;
import android.support.v4.app.NavUtils;
import android.support.v4.view.MenuItemCompat;
@ -932,8 +933,8 @@ public class AppDetails extends ListActivity {
@Override
public void onClick(DialogInterface dialog,
int whichButton) {
downloadHandler = new DownloadHandler(activity, apk,
repoaddress, Utils.getApkCacheDir(getBaseContext()));
downloadHandler = new DownloadHandler(
apk, repoaddress, Utils.getApkCacheDir(getBaseContext()));
}
});
ask_alrt.setNegativeButton(getString(R.string.no),
@ -962,8 +963,8 @@ public class AppDetails extends ListActivity {
alert.show();
return;
}
downloadHandler = new DownloadHandler(activity, apk, repoaddress,
Utils.getApkCacheDir(getBaseContext()));
downloadHandler = new DownloadHandler(
apk, repoaddress, Utils.getApkCacheDir(getBaseContext()));
}
private void installApk(File file, String packageName) {
setProgressBarIndeterminateVisibility(true);
@ -1076,14 +1077,12 @@ public class AppDetails extends ListActivity {
// Handler used to update the progress dialog while downloading.
private class DownloadHandler extends Handler implements ProgressListener {
private static final String TAG = "AppDetails.DownloadHandler";
private Activity activity;
private static final String TAG = "org.fdroid.fdroid.AppDetails.DownloadHandler";
private ApkDownloader download;
private ProgressDialog pd;
private String id;
public DownloadHandler(Activity activity, Apk apk, String repoaddress, File destdir) {
this.activity = activity;
public DownloadHandler(Apk apk, String repoaddress, File destdir) {
id = apk.id;
download = new ApkDownloader(apk, repoaddress, destdir);
download.setProgressListener(this);
@ -1096,23 +1095,22 @@ public class AppDetails extends ListActivity {
}
}
public void onProgress(final ProgressListener.Event event) {
private static final String MSG_EVENT_DATA = "msgEvent";
/**
* Subclasses must implement this to receive messages.
*/
public void handleMessage(Message msg) {
ProgressListener.Event event = msg.getData().getParcelable(MSG_EVENT_DATA);
boolean finished = false;
switch (event.type) {
case Downloader.EVENT_PROGRESS:
activity.runOnUiThread(new Runnable() {
@Override
public void run() {
// this must be on the main UI thread
if (pd == null) {
pd = createProgressDialog(download.getRemoteAddress(),
event.progress, event.total);
} else {
pd.setProgress(event.progress);
}
}
});
if (pd == null) {
pd = createProgressDialog(download.getRemoteAddress(),
event.progress, event.total);
} else {
pd.setProgress(event.progress);
}
break;
case ApkDownloader.EVENT_ERROR_DOWNLOAD_FAILED:
@ -1123,14 +1121,8 @@ public class AppDetails extends ListActivity {
text = getString(R.string.corrupt_download);
else
text = getString(R.string.details_notinstalled);
activity.runOnUiThread(new Runnable() {
@Override
public void run() {
// this must be on the main UI thread
Toast.makeText(AppDetails.this, text, Toast.LENGTH_LONG).show();
}
});
// this must be on the main UI thread
Toast.makeText(AppDetails.this, text, Toast.LENGTH_LONG).show();
finished = true;
break;
@ -1138,7 +1130,6 @@ public class AppDetails extends ListActivity {
installApk(download.localFile(), id);
finished = true;
break;
}
if (finished) {
@ -1146,6 +1137,20 @@ public class AppDetails extends ListActivity {
}
}
/**
* We receive events on the download thread, and then post them to
* whatever thread the DownloadHandler was run on (in our case, the UI
* thread).
* @param event
*/
public void onProgress(final ProgressListener.Event event) {
Message message = new Message();
Bundle bundle = new Bundle(1);
bundle.putParcelable(MSG_EVENT_DATA, event);
message.setData(bundle);
sendMessage(message);
}
public void cancel() {
// TODO: Re-implement...
}

View File

@ -15,14 +15,14 @@ import java.io.OutputStream;
import java.net.MalformedURLException;
public abstract class Downloader {
private static final String TAG = "Downloader";
private static final String TAG = "org.fdroid.fdroid.net.Downloader";
private OutputStream outputStream;
private ProgressListener progressListener = null;
private ProgressListener.Event progressEvent = null;
private File outputFile;
protected String eTag = null;
protected String cacheTag = null;
public static final int EVENT_PROGRESS = 1;
@ -68,20 +68,24 @@ public abstract class Downloader {
}
/**
* If you ask for the eTag before calling download(), you will get the
* If you ask for the cacheTag before calling download(), you will get the
* same one you passed in (if any). If you call it after download(), you
* will get the new eTag from the server, or null if there was none.
* will get the new cacheTag from the server, or null if there was none.
*/
public String getETag() {
return eTag;
public String getCacheTag() {
return cacheTag;
}
/**
* If this eTag matches that returned by the server, then no download will
* If this cacheTag matches that returned by the server, then no download will
* take place, and a status code of 304 will be returned by download().
*/
public void setETag(String eTag) {
this.eTag = eTag;
public void setCacheTag(String cacheTag) {
this.cacheTag = cacheTag;
}
protected boolean wantToCheckCache() {
return cacheTag != null;
}
/**
@ -104,8 +108,12 @@ public abstract class Downloader {
}
}
public void download() throws IOException {
Log.i(TAG, "download");
public abstract void download() throws IOException;
public abstract boolean isCached();
protected void downloadFromStream() throws IOException {
Log.d(TAG, "Downloading from stream");
setupProgressListener();
InputStream input = null;
try {

View File

@ -11,11 +11,12 @@ import java.io.OutputStream;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.UnknownHostException;
import javax.net.ssl.SSLHandshakeException;
public class HttpDownloader extends Downloader {
private static final String TAG = "HttpDownloader";
private static final String TAG = "org.fdroid.fdroid.net.HttpDownloader";
private static final String HEADER_IF_NONE_MATCH = "If-None-Match";
private static final String HEADER_FIELD_ETAG = "ETag";
@ -68,29 +69,22 @@ public class HttpDownloader extends Downloader {
public void download() throws IOException {
try {
connection = (HttpURLConnection)sourceUrl.openConnection();
setupCacheCheck();
statusCode = connection.getResponseCode();
Log.i(TAG, "download " + statusCode);
if (statusCode == 304) {
// The index is unchanged since we last read it. We just mark
// everything that came from this repo as being updated.
Log.d("FDroid", "Repo index for " + sourceUrl
+ " is up to date (by etag)");
} else if (statusCode == 200) {
download();
updateCacheCheck();
if (wantToCheckCache()) {
setupCacheCheck();
Log.i(TAG, "Checking cached status of " + sourceUrl);
statusCode = connection.getResponseCode();
}
if (isCached()) {
Log.i(TAG, sourceUrl + " is cached, so not downloading (HTTP " + statusCode + ")");
} else {
// Is there any code other than 200 which still returns
// content? Just in case, lets try to clean up.
if (getFile() != null) {
getFile().delete();
}
throw new IOException(
"Failed to update repo " + sourceUrl +
" - HTTP response " + statusCode);
Log.i(TAG, "Downloading from " + sourceUrl);
downloadFromStream();
updateCacheCheck();
}
} catch (SSLHandshakeException e) {
// TODO this should be handled better
// TODO this should be handled better, it is not internationalised here.
throw new IOException(
"A problem occurred while establishing an SSL " +
"connection. If this problem persists, AND you have a " +
@ -99,14 +93,18 @@ public class HttpDownloader extends Downloader {
}
}
public boolean isCached() {
return wantToCheckCache() && statusCode == 304;
}
private void setupCacheCheck() {
if (eTag != null) {
connection.setRequestProperty(HEADER_IF_NONE_MATCH, eTag);
if (cacheTag != null) {
connection.setRequestProperty(HEADER_IF_NONE_MATCH, cacheTag);
}
}
private void updateCacheCheck() {
eTag = connection.getHeaderField(HEADER_FIELD_ETAG);
cacheTag = connection.getHeaderField(HEADER_FIELD_ETAG);
}
// Testing in the emulator for me, showed that figuring out the

View File

@ -26,7 +26,6 @@ import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import javax.net.ssl.SSLHandshakeException;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;
@ -89,7 +88,7 @@ abstract public class RepoUpdater {
Downloader downloader = null;
try {
downloader = new HttpDownloader(getIndexAddress(), context);
downloader.setETag(repo.lastetag);
downloader.setCacheTag(repo.lastetag);
if (progressListener != null) { // interactive session, show progress
downloader.setProgressListener(progressListener,
@ -97,6 +96,14 @@ abstract public class RepoUpdater {
}
downloader.download();
if (downloader.isCached()) {
// The index is unchanged since we last read it. We just mark
// everything that came from this repo as being updated.
Log.d("FDroid", "Repo index for " + getIndexAddress()
+ " is up to date (by etag)");
}
} catch (IOException e) {
if (downloader != null && downloader.getFile() != null) {
downloader.getFile().delete();
@ -160,7 +167,7 @@ abstract public class RepoUpdater {
reader.parse(is);
apps = handler.getApps();
apks = handler.getApks();
updateRepo(handler, downloader.getETag());
updateRepo(handler, downloader.getCacheTag());
}
} catch (SAXException e) {
throw new UpdateException(