OSDN Git Service

Handle corrupted files when cutting out pages
authorPhilip P. Moltmann <moltmann@google.com>
Thu, 16 Mar 2017 16:13:01 +0000 (09:13 -0700)
committerPhilip P. Moltmann <moltmann@google.com>
Thu, 16 Mar 2017 19:15:53 +0000 (12:15 -0700)
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

packages/PrintSpooler/src/com/android/printspooler/model/RemotePrintDocument.java
packages/PrintSpooler/src/com/android/printspooler/ui/PrintActivity.java
packages/PrintSpooler/src/com/android/printspooler/util/PageRangeUtils.java

index 6e1385a..187e35a 100644 (file)
@@ -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.
+         * <p>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.</p>
+         *
+         * @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));
             }
 
index 4b51917..f6df995 100644 (file)
@@ -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<String> mCallback;
 
         public DocumentTransformer(Context context, PrintJobInfo printJob,
                 MutexFileProvider fileProvider, PrintAttributes attributes,
-                Runnable callback) {
+                Consumer<String> 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<Void, Void, Void>() {
+            new AsyncTask<Void, Void, String>() {
                 @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);
index 7425c03..a36f583 100644 (file)
@@ -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.