From f7a5b4fb30792789d9e436bb5107f1a6d743d49c Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Thu, 16 Mar 2017 09:13:01 -0700 Subject: [PATCH] Handle corrupted files when cutting out pages Before the print spooler crashes, now we crash the printing app. Bonus: Renamed and documented fields as I could never remeber what they mean. Test: Added new (disabled) test to CTS print tests that emulated the scenario Bug: 35350768 Change-Id: I41c094960d96f46d274e9f87381bcda5274d5612 --- .../printspooler/model/RemotePrintDocument.java | 41 ++++++++++++------- .../com/android/printspooler/ui/PrintActivity.java | 46 ++++++++++++---------- .../android/printspooler/util/PageRangeUtils.java | 33 ++++++++++------ 3 files changed, 73 insertions(+), 47 deletions(-) diff --git a/packages/PrintSpooler/src/com/android/printspooler/model/RemotePrintDocument.java b/packages/PrintSpooler/src/com/android/printspooler/model/RemotePrintDocument.java index 6e1385a04f6b..187e35ac842c 100644 --- a/packages/PrintSpooler/src/com/android/printspooler/model/RemotePrintDocument.java +++ b/packages/PrintSpooler/src/com/android/printspooler/model/RemotePrintDocument.java @@ -94,10 +94,10 @@ public final class RemotePrintDocument { // but the content has changed. if (mNextCommand == null) { if (mUpdateSpec.pages != null && (mDocumentInfo.changed - || mDocumentInfo.writtenPages == null + || mDocumentInfo.pagesWrittenToFile == null || (mDocumentInfo.info.getPageCount() != PrintDocumentInfo.PAGE_COUNT_UNKNOWN - && !PageRangeUtils.contains(mDocumentInfo.writtenPages, + && !PageRangeUtils.contains(mDocumentInfo.pagesWrittenToFile, mUpdateSpec.pages, mDocumentInfo.info.getPageCount())))) { mNextCommand = new WriteCommand(mContext, mLooper, mPrintDocumentAdapter, mDocumentInfo, @@ -106,9 +106,10 @@ public final class RemotePrintDocument { } else { if (mUpdateSpec.pages != null) { // If we have the requested pages, update which ones to be printed. - mDocumentInfo.printedPages = PageRangeUtils.computePrintedPages( - mUpdateSpec.pages, mDocumentInfo.writtenPages, - mDocumentInfo.info.getPageCount()); + mDocumentInfo.pagesInFileToPrint = + PageRangeUtils.computeWhichPagesInFileToPrint( + mUpdateSpec.pages, mDocumentInfo.pagesWrittenToFile, + mDocumentInfo.info.getPageCount()); } // Notify we are done. mState = STATE_UPDATED; @@ -514,8 +515,20 @@ public final class RemotePrintDocument { public PrintAttributes attributes; public Bundle metadata; public PrintDocumentInfo info; - public PageRange[] printedPages; - public PageRange[] writtenPages; + + /** + * Which pages out of the ones written to the file to print. This is not indexed by the + * document pages, but by the page number in the file. + *

E.g. if a document has 10 pages, we want pages 4-5 and 7, but only page 3-9 are in the + * file. This would contain 1-2 and 4.

+ * + * @see PageRangeUtils#computeWhichPagesInFileToPrint + */ + public PageRange[] pagesInFileToPrint; + + /** Pages of the whole document that are currently written to file */ + public PageRange[] pagesWrittenToFile; + public MutexFileProvider fileProvider; public boolean changed; public boolean updated; @@ -783,8 +796,8 @@ public final class RemotePrintDocument { if (changed || !equalsIgnoreSize(mDocument.info, info)) { // If the content changed we throw away all pages as // we will request them again with the new content. - mDocument.writtenPages = null; - mDocument.printedPages = null; + mDocument.pagesWrittenToFile = null; + mDocument.pagesInFileToPrint = null; mDocument.changed = true; } @@ -1098,17 +1111,17 @@ public final class RemotePrintDocument { } PageRange[] writtenPages = PageRangeUtils.normalize(pages); - PageRange[] printedPages = PageRangeUtils.computePrintedPages( + PageRange[] printedPages = PageRangeUtils.computeWhichPagesInFileToPrint( mPages, writtenPages, mPageCount); // Handle if we got invalid pages if (printedPages != null) { - mDocument.writtenPages = writtenPages; - mDocument.printedPages = printedPages; + mDocument.pagesWrittenToFile = writtenPages; + mDocument.pagesInFileToPrint = printedPages; completed(); } else { - mDocument.writtenPages = null; - mDocument.printedPages = null; + mDocument.pagesWrittenToFile = null; + mDocument.pagesInFileToPrint = null; failed(mContext.getString(R.string.print_error_default_message)); } diff --git a/packages/PrintSpooler/src/com/android/printspooler/ui/PrintActivity.java b/packages/PrintSpooler/src/com/android/printspooler/ui/PrintActivity.java index 4b519171b3eb..f6df9953bf0f 100644 --- a/packages/PrintSpooler/src/com/android/printspooler/ui/PrintActivity.java +++ b/packages/PrintSpooler/src/com/android/printspooler/ui/PrintActivity.java @@ -85,8 +85,8 @@ import android.widget.EditText; import android.widget.ImageView; import android.widget.Spinner; import android.widget.TextView; - import android.widget.Toast; + import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.printspooler.R; @@ -120,6 +120,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.function.Consumer; public class PrintActivity extends Activity implements RemotePrintDocument.UpdateResultCallbacks, PrintErrorFragment.OnActionListener, PageAdapter.ContentCallbacks, @@ -543,8 +544,8 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat // pages in the printed document. PrintDocumentInfo info = document.info; if (info != null) { - final int pageCount = PageRangeUtils.getNormalizedPageCount(document.writtenPages, - getAdjustedPageCount(info)); + final int pageCount = PageRangeUtils.getNormalizedPageCount( + document.pagesWrittenToFile, getAdjustedPageCount(info)); PrintDocumentInfo adjustedInfo = new PrintDocumentInfo.Builder(info.getName()) .setContentType(info.getContentType()) .setPageCount(pageCount) @@ -558,7 +559,7 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat } mPrintJob.setDocumentInfo(adjustedInfo); - mPrintJob.setPages(document.printedPages); + mPrintJob.setPages(document.pagesInFileToPrint); } switch (mState) { @@ -627,7 +628,7 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat // Update the preview controller. mPrintPreviewController.onContentUpdated(contentUpdated, getAdjustedPageCount(documentInfo.info), - mPrintedDocument.getDocumentInfo().writtenPages, + mPrintedDocument.getDocumentInfo().pagesWrittenToFile, mSelectedPages, mPrintJob.getAttributes().getMediaSize(), mPrintJob.getAttributes().getMinMargins()); } @@ -2105,14 +2106,15 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat // If saving to PDF, apply the attibutes as we are acting as a print service. PrintAttributes attributes = mDestinationSpinnerAdapter.getPdfPrinter() == mCurrentPrinter ? mPrintJob.getAttributes() : null; - new DocumentTransformer(this, mPrintJob, mFileProvider, attributes, new Runnable() { - @Override - public void run() { + new DocumentTransformer(this, mPrintJob, mFileProvider, attributes, error -> { + if (error == null) { if (writeToUri != null) { mPrintedDocument.writeContent(getContentResolver(), writeToUri); } setState(STATE_PRINT_COMPLETED); doFinish(); + } else { + onPrintDocumentError(error); } }).transform(); } @@ -3096,11 +3098,11 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat private final PrintAttributes mAttributesToApply; - private final Runnable mCallback; + private final Consumer mCallback; public DocumentTransformer(Context context, PrintJobInfo printJob, MutexFileProvider fileProvider, PrintAttributes attributes, - Runnable callback) { + Consumer callback) { mContext = context; mPrintJob = printJob; mFileProvider = fileProvider; @@ -3112,7 +3114,7 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat public void transform() { // If we have only the pages we want, done. if (mPagesToShred.length <= 0 && mAttributesToApply == null) { - mCallback.run(); + mCallback.accept(null); return; } @@ -3126,22 +3128,26 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat @Override public void onServiceConnected(ComponentName name, IBinder service) { final IPdfEditor editor = IPdfEditor.Stub.asInterface(service); - new AsyncTask() { + new AsyncTask() { @Override - protected Void doInBackground(Void... params) { + protected String doInBackground(Void... params) { // It's OK to access the data members as they are // final and this code is the last one to touch // them as shredding is the very last step, so the // UI is not interactive at this point. - doTransform(editor); - updatePrintJob(); - return null; + try { + doTransform(editor); + updatePrintJob(); + return null; + } catch (IOException | RemoteException | IllegalStateException e) { + return e.toString(); + } } @Override - protected void onPostExecute(Void aVoid) { + protected void onPostExecute(String error) { mContext.unbindService(DocumentTransformer.this); - mCallback.run(); + mCallback.accept(error); } }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); } @@ -3151,7 +3157,7 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat /* do nothing */ } - private void doTransform(IPdfEditor editor) { + private void doTransform(IPdfEditor editor) throws IOException, RemoteException { File tempFile = null; ParcelFileDescriptor src = null; ParcelFileDescriptor dst = null; @@ -3190,8 +3196,6 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat in = new FileInputStream(tempFile); out = new FileOutputStream(jobFile); Streams.copy(in, out); - } catch (IOException|RemoteException e) { - Log.e(LOG_TAG, "Error dropping pages", e); } finally { IoUtils.closeQuietly(src); IoUtils.closeQuietly(dst); diff --git a/packages/PrintSpooler/src/com/android/printspooler/util/PageRangeUtils.java b/packages/PrintSpooler/src/com/android/printspooler/util/PageRangeUtils.java index 7425c033a7a4..a36f5837ecb1 100644 --- a/packages/PrintSpooler/src/com/android/printspooler/util/PageRangeUtils.java +++ b/packages/PrintSpooler/src/com/android/printspooler/util/PageRangeUtils.java @@ -394,33 +394,42 @@ public final class PageRangeUtils { return pageRanges.getStart() == 0 && pageRanges.getEnd() == pageCount - 1; } - public static PageRange[] computePrintedPages(PageRange[] requestedPages, - PageRange[] writtenPages, int pageCount) { + /** + * Compute the pages of the file that correspond to the requested pages in the doc. + * + * @param pagesInDocRequested The requested pages, doc-indexed + * @param pagesWrittenToFile The pages in the file + * @param pageCount The number of pages in the doc + * + * @return The pages, file-indexed + */ + public static PageRange[] computeWhichPagesInFileToPrint(PageRange[] pagesInDocRequested, + PageRange[] pagesWrittenToFile, int pageCount) { // Adjust the print job pages based on what was requested and written. // The cases are ordered in the most expected to the least expected // with a special case first where the app does not know the page count // so we ask for all to be written. - if (Arrays.equals(requestedPages, ALL_PAGES_RANGE) + if (Arrays.equals(pagesInDocRequested, ALL_PAGES_RANGE) && pageCount == PrintDocumentInfo.PAGE_COUNT_UNKNOWN) { return ALL_PAGES_RANGE; - } else if (Arrays.equals(writtenPages, requestedPages)) { + } else if (Arrays.equals(pagesWrittenToFile, pagesInDocRequested)) { // We got a document with exactly the pages we wanted. Hence, // the printer has to print all pages in the data. return ALL_PAGES_RANGE; - } else if (Arrays.equals(writtenPages, ALL_PAGES_RANGE)) { + } else if (Arrays.equals(pagesWrittenToFile, ALL_PAGES_RANGE)) { // We requested specific pages but got all of them. Hence, // the printer has to print only the requested pages. - return requestedPages; - } else if (PageRangeUtils.contains(writtenPages, requestedPages, pageCount)) { + return pagesInDocRequested; + } else if (PageRangeUtils.contains(pagesWrittenToFile, pagesInDocRequested, pageCount)) { // We requested specific pages and got more but not all pages. // Hence, we have to offset appropriately the printed pages to // be based off the start of the written ones instead of zero. // The written pages are always non-null and not empty. - final int offset = -writtenPages[0].getStart(); - PageRangeUtils.offset(requestedPages, offset); - return requestedPages; - } else if (Arrays.equals(requestedPages, ALL_PAGES_RANGE) - && isAllPages(writtenPages, pageCount)) { + final int offset = -pagesWrittenToFile[0].getStart(); + PageRangeUtils.offset(pagesInDocRequested, offset); + return pagesInDocRequested; + } else if (Arrays.equals(pagesInDocRequested, ALL_PAGES_RANGE) + && isAllPages(pagesWrittenToFile, pageCount)) { // We requested all pages via the special constant and got all // of them as an explicit enumeration. Hence, the printer has // to print only the requested pages. -- 2.11.0