update progress handling in PgpDecryptVerifyOperation

- less progress messages overall
- report progress again based on input stream position, if filesize is known
- limit progress messages to one every 200ms while decrypting
- also fixed a bug in DecryptListFragment, the recycler view now reuses
  old views as intended instead of crossfading all the time.
This commit is contained in:
Vincent Breitmoser
2016-05-11 16:10:53 +02:00
parent b362668f81
commit 48758cdec5
4 changed files with 36 additions and 40 deletions

View File

@@ -84,6 +84,8 @@ import org.sufficientlysecure.keychain.util.ProgressScaler;
public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInputParcel> { public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInputParcel> {
public static final int PROGRESS_STRIDE_MILLISECONDS = 200;
public PgpDecryptVerifyOperation(Context context, ProviderHelper providerHelper, Progressable progressable) { public PgpDecryptVerifyOperation(Context context, ProviderHelper providerHelper, Progressable progressable) {
super(context, providerHelper, progressable); super(context, providerHelper, progressable);
} }
@@ -153,10 +155,10 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
return verifyDetachedSignature(input, inputData, outputStream, 0); return verifyDetachedSignature(input, inputData, outputStream, 0);
} else { } else {
// automatically works with PGP ascii armor and PGP binary // automatically works with PGP ascii armor and PGP binary
InputStream in = PGPUtil.getDecoderStream(inputData.getInputStream()); InputStream inputStream = PGPUtil.getDecoderStream(inputData.getInputStream());
if (in instanceof ArmoredInputStream) { if (inputStream instanceof ArmoredInputStream) {
ArmoredInputStream aIn = (ArmoredInputStream) in; ArmoredInputStream aIn = (ArmoredInputStream) inputStream;
// it is ascii armored // it is ascii armored
Log.d(Constants.TAG, "ASCII Armor Header Line: " + aIn.getArmorHeaderLine()); Log.d(Constants.TAG, "ASCII Armor Header Line: " + aIn.getArmorHeaderLine());
@@ -165,10 +167,10 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
return verifyCleartextSignature(aIn, outputStream, 0); return verifyCleartextSignature(aIn, outputStream, 0);
} else { } else {
// else: ascii armored encryption! go on... // else: ascii armored encryption! go on...
return decryptVerify(input, cryptoInput, in, outputStream, 0); return decryptVerify(input, cryptoInput, inputData, inputStream, outputStream, 0);
} }
} else { } else {
return decryptVerify(input, cryptoInput, in, outputStream, 0); return decryptVerify(input, cryptoInput, inputData, inputStream, outputStream, 0);
} }
} }
} catch (PGPException e) { } catch (PGPException e) {
@@ -272,15 +274,14 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
@NonNull @NonNull
private DecryptVerifyResult decryptVerify( private DecryptVerifyResult decryptVerify(
PgpDecryptVerifyInputParcel input, CryptoInputParcel cryptoInput, PgpDecryptVerifyInputParcel input, CryptoInputParcel cryptoInput,
InputStream in, OutputStream out, int indent) throws IOException, PGPException { InputData inputData, InputStream in, OutputStream out, int indent) throws IOException, PGPException {
OperationLog log = new OperationLog(); OperationLog log = new OperationLog();
log.add(LogType.MSG_DC, indent); log.add(LogType.MSG_DC, indent);
indent += 1; indent += 1;
int currentProgress = 0; updateProgress(R.string.progress_reading_data, 0, 100);
updateProgress(R.string.progress_reading_data, currentProgress, 100);
// parse ASCII Armor headers // parse ASCII Armor headers
ArmorHeaders armorHeaders = parseArmorHeaders(in, log, indent); ArmorHeaders armorHeaders = parseArmorHeaders(in, log, indent);
@@ -301,8 +302,7 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
if (obj instanceof PGPEncryptedDataList) { if (obj instanceof PGPEncryptedDataList) {
esResult = handleEncryptedPacket( esResult = handleEncryptedPacket(
input, cryptoInput, (PGPEncryptedDataList) obj, log, indent, input, cryptoInput, (PGPEncryptedDataList) obj, log, indent, useBackupCode);
currentProgress, useBackupCode);
// if there is an error, nothing left to do here // if there is an error, nothing left to do here
if (esResult.errorResult != null) { if (esResult.errorResult != null) {
@@ -346,8 +346,6 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
// resolve compressed data // resolve compressed data
if (dataChunk instanceof PGPCompressedData) { if (dataChunk instanceof PGPCompressedData) {
log.add(LogType.MSG_DC_CLEAR_DECOMPRESS, indent + 1); log.add(LogType.MSG_DC_CLEAR_DECOMPRESS, indent + 1);
currentProgress += 2;
updateProgress(R.string.progress_decompressing_data, currentProgress, 100);
PGPCompressedData compressedData = (PGPCompressedData) dataChunk; PGPCompressedData compressedData = (PGPCompressedData) dataChunk;
@@ -377,8 +375,6 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
log.add(LogType.MSG_DC_CLEAR_DATA, indent + 1); log.add(LogType.MSG_DC_CLEAR_DATA, indent + 1);
indent += 2; indent += 2;
currentProgress += 4;
updateProgress(R.string.progress_decrypting, currentProgress, 100);
PGPLiteralData literalData = (PGPLiteralData) dataChunk; PGPLiteralData literalData = (PGPLiteralData) dataChunk;
@@ -436,20 +432,22 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
return result; return result;
} }
ProgressScaler progressScaler =
new ProgressScaler(mProgressable, currentProgress, 95, 100);
InputStream dataIn = literalData.getInputStream(); InputStream dataIn = literalData.getInputStream();
long opTime, startTime = System.currentTimeMillis(); long opTime, startTime = System.currentTimeMillis();
long alreadyWritten = 0; long alreadyWritten = 0;
long wholeSize = 0; // TODO inputData.getSize() - inputData.getStreamPosition(); long wholeSize = inputData.getSize() - inputData.getStreamPosition();
boolean sizeIsKnown = inputData.getSize() != InputData.UNKNOWN_FILESIZE && wholeSize > 0;
int length; int length;
byte[] buffer = new byte[8192]; byte[] buffer = new byte[8192];
byte[] firstBytes = new byte[48]; byte[] firstBytes = new byte[48];
CharsetVerifier charsetVerifier = new CharsetVerifier(buffer, mimeType, charset); CharsetVerifier charsetVerifier = new CharsetVerifier(buffer, mimeType, charset);
updateProgress(R.string.progress_decrypting, 1, 100);
long nextProgressTime = 0L;
int lastReportedProgress = 1;
while ((length = dataIn.read(buffer)) > 0) { while ((length = dataIn.read(buffer)) > 0) {
// Log.d(Constants.TAG, "read bytes: " + length); // Log.d(Constants.TAG, "read bytes: " + length);
if (out != null) { if (out != null) {
@@ -467,14 +465,17 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
} }
alreadyWritten += length; alreadyWritten += length;
// noinspection ConstantConditions, TODO progress if (sizeIsKnown && nextProgressTime < System.currentTimeMillis()) {
if (wholeSize > 0) { long progress = 100 * inputData.getStreamPosition() / wholeSize;
long progress = 100 * alreadyWritten / wholeSize;
// stop at 100% for wrong file sizes... // stop at 100% for wrong file sizes...
if (progress > 100) { if (progress > 100) {
progress = 100; progress = 100;
} }
progressScaler.setProgress((int) progress, 100); if (progress > lastReportedProgress) {
updateProgress((int) progress, 100);
lastReportedProgress = (int) progress;
nextProgressTime = System.currentTimeMillis() + PROGRESS_STRIDE_MILLISECONDS;
}
} }
} }
@@ -490,7 +491,8 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
} }
opTime = System.currentTimeMillis()-startTime; opTime = System.currentTimeMillis()-startTime;
Log.d(Constants.TAG, "decrypt time taken: " + String.format("%.2f", opTime / 1000.0) + "s"); Log.d(Constants.TAG, "decrypt time taken: " + String.format("%.2f", opTime / 1000.0) + "s, for "
+ alreadyWritten + " bytes");
// special treatment to detect pgp mime types // special treatment to detect pgp mime types
// TODO move into CharsetVerifier? seems like that would be a plausible place for this logic // TODO move into CharsetVerifier? seems like that would be a plausible place for this logic
@@ -513,8 +515,6 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
if (esResult != null) { if (esResult != null) {
if (esResult.encryptedData.isIntegrityProtected()) { if (esResult.encryptedData.isIntegrityProtected()) {
updateProgress(R.string.progress_verifying_integrity, 95, 100);
if (esResult.encryptedData.verify()) { if (esResult.encryptedData.verify()) {
log.add(LogType.MSG_DC_INTEGRITY_CHECK_OK, indent); log.add(LogType.MSG_DC_INTEGRITY_CHECK_OK, indent);
} else { } else {
@@ -548,7 +548,7 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
} }
private EncryptStreamResult handleEncryptedPacket(PgpDecryptVerifyInputParcel input, CryptoInputParcel cryptoInput, private EncryptStreamResult handleEncryptedPacket(PgpDecryptVerifyInputParcel input, CryptoInputParcel cryptoInput,
PGPEncryptedDataList enc, OperationLog log, int indent, int currentProgress, boolean useBackupCode) throws PGPException { PGPEncryptedDataList enc, OperationLog log, int indent, boolean useBackupCode) throws PGPException {
EncryptStreamResult result = new EncryptStreamResult(); EncryptStreamResult result = new EncryptStreamResult();
@@ -574,9 +574,6 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
if (obj instanceof PGPPublicKeyEncryptedData) { if (obj instanceof PGPPublicKeyEncryptedData) {
anyPacketFound = true; anyPacketFound = true;
currentProgress += 2;
updateProgress(R.string.progress_finding_key, currentProgress, 100);
PGPPublicKeyEncryptedData encData = (PGPPublicKeyEncryptedData) obj; PGPPublicKeyEncryptedData encData = (PGPPublicKeyEncryptedData) obj;
long subKeyId = encData.getKeyID(); long subKeyId = encData.getKeyID();
@@ -741,9 +738,6 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
// we made sure above one of these two would be true // we made sure above one of these two would be true
if (symmetricPacketFound) { if (symmetricPacketFound) {
currentProgress += 2;
updateProgress(R.string.progress_preparing_streams, currentProgress, 100);
PGPDigestCalculatorProvider digestCalcProvider = new JcaPGPDigestCalculatorProviderBuilder() PGPDigestCalculatorProvider digestCalcProvider = new JcaPGPDigestCalculatorProviderBuilder()
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(); .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build();
PBEDataDecryptorFactory decryptorFactory = new JcePBEDataDecryptorFactoryBuilder( PBEDataDecryptorFactory decryptorFactory = new JcePBEDataDecryptorFactoryBuilder(
@@ -764,9 +758,6 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
result.symmetricEncryptionAlgo = encryptedDataSymmetric.getSymmetricAlgorithm(decryptorFactory); result.symmetricEncryptionAlgo = encryptedDataSymmetric.getSymmetricAlgorithm(decryptorFactory);
} else if (asymmetricPacketFound) { } else if (asymmetricPacketFound) {
currentProgress += 2;
updateProgress(R.string.progress_extracting_key, currentProgress, 100);
CachingDataDecryptorFactory decryptorFactory; CachingDataDecryptorFactory decryptorFactory;
if (decryptedSessionKeyAvailable) { if (decryptedSessionKeyAvailable) {
decryptorFactory = cachedKeyDecryptorFactory; decryptorFactory = cachedKeyDecryptorFactory;
@@ -782,9 +773,6 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
return result.with(new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log)); return result.with(new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log));
} }
currentProgress += 2;
updateProgress(R.string.progress_preparing_streams, currentProgress, 100);
decryptorFactory = decryptionKey.getCachingDecryptorFactory(cryptoInput); decryptorFactory = decryptionKey.getCachingDecryptorFactory(cryptoInput);
// special case: if the decryptor does not have a session key cached for this encrypted // special case: if the decryptor does not have a session key cached for this encrypted

View File

@@ -169,7 +169,12 @@ public class DecryptListFragment
vFilesList.setHasFixedSize(true); vFilesList.setHasFixedSize(true);
// TODO make this a grid, for tablets! // TODO make this a grid, for tablets!
vFilesList.setLayoutManager(new LinearLayoutManager(getActivity())); vFilesList.setLayoutManager(new LinearLayoutManager(getActivity()));
vFilesList.setItemAnimator(new DefaultItemAnimator()); vFilesList.setItemAnimator(new DefaultItemAnimator() {
@Override
public boolean canReuseUpdatedViewHolder(@NonNull RecyclerView.ViewHolder viewHolder) {
return true;
}
});
mAdapter = new DecryptFilesAdapter(); mAdapter = new DecryptFilesAdapter();
vFilesList.setAdapter(mAdapter); vFilesList.setAdapter(mAdapter);

View File

@@ -23,6 +23,8 @@ import java.io.InputStream;
* Wrapper to include size besides an InputStream * Wrapper to include size besides an InputStream
*/ */
public class InputData { public class InputData {
public static final int UNKNOWN_FILESIZE = -1;
private PositionAwareInputStream mInputStream; private PositionAwareInputStream mInputStream;
private long mSize; private long mSize;
String mOriginalFilename; String mOriginalFilename;

View File

@@ -457,7 +457,8 @@
<string name="progress_extracting_key">"extracting key…"</string> <string name="progress_extracting_key">"extracting key…"</string>
<string name="progress_preparing_streams">"preparing streams…"</string> <string name="progress_preparing_streams">"preparing streams…"</string>
<string name="progress_encrypting">"encrypting data…"</string> <string name="progress_encrypting">"encrypting data…"</string>
<string name="progress_decrypting">"decrypting data…"</string> <string name="progress_prepare_decryption">"Starting decryption…"</string>
<string name="progress_decrypting">"Decrypting data…"</string>
<string name="progress_preparing_signature">"preparing signature…"</string> <string name="progress_preparing_signature">"preparing signature…"</string>
<string name="progress_processing_signature">processing signature…</string> <string name="progress_processing_signature">processing signature…</string>
<string name="progress_generating_signature">"generating signature…"</string> <string name="progress_generating_signature">"generating signature…"</string>