Print spooler crash when printing after a rotation.

1. Available printers are provided by a loader driven by the activity
   lifecycle. After a rotation of an app that does not handle rotation
   the print activity is destroyed in a delayed fashion. When another
   print task is started the loader of the destroed activity gets two
   cancellations, one when the activity is stopped and one when it is
   destroyed but when the second cancellation happens the loader has a
   bad state that it is loading while it is not and a NPE occurs.

2. If a rotation happens early before the print activity is fully initialized
   (there are asyncrconous operations that take place) we get a NPE as
   the print preview controller is not created yet.

3. A lockup of the print preview process occurs if the print print
   activity is destroyed after we initiated binding to the rendering
   service but before we receive the remote interface. In this case no
   remote service is received as exepcted since we already unbound from
   it resulting in a missed signal.

bug:18238590

Change-Id: I81817d81702f649ded97dfbab7d7bba28f22fa91

diff --git a/packages/PrintSpooler/src/com/android/printspooler/model/PageContentRepository.java b/packages/PrintSpooler/src/com/android/printspooler/model/PageContentRepository.java
index 882b364..0923649 100644 (file)
--- a/packages/PrintSpooler/src/com/android/printspooler/model/PageContentRepository.java
+++ b/packages/PrintSpooler/src/com/android/printspooler/model/PageContentRepository.java
@@ -412,6 +412,8 @@ public final class PageContentRepository {
         @GuardedBy("mLock")
         private IPdfRenderer mRenderer;
 
+        private OpenTask mOpenTask;
+
         private boolean mBoundToService;
         private boolean mDestroyed;
 
@@ -439,75 +441,15 @@ public final class PageContentRepository {
             }
         }
 
-        public void open(final ParcelFileDescriptor source, final OpenDocumentCallback callback) {
+        public void open(ParcelFileDescriptor source, OpenDocumentCallback callback) {
             // Opening a new document invalidates the cache as it has pages
             // from the last document. We keep the cache even when the document
             // is closed to show pages while the other side is writing the new
             // document.
             mPageContentCache.invalidate();
 
-            new AsyncTask<Void, Void, Integer>() {
-                @Override
-                protected void onPreExecute() {
-                    if (mDestroyed) {
-                        cancel(true);
-                        return;
-                    }
-                    Intent intent = new Intent(PdfManipulationService.ACTION_GET_RENDERER);
-                    intent.setClass(mContext, PdfManipulationService.class);
-                    intent.setData(Uri.fromParts("fake-scheme", String.valueOf(
-                            AsyncRenderer.this.hashCode()), null));
-                    mContext.bindService(intent, AsyncRenderer.this, Context.BIND_AUTO_CREATE);
-                    mBoundToService = true;
-                }
-
-                @Override
-                protected Integer doInBackground(Void... params) {
-                    synchronized (mLock) {
-                        while (mRenderer == null) {
-                            try {
-                                mLock.wait();
-                            } catch (InterruptedException ie) {
-                                /* ignore */
-                            }
-                        }
-                        try {
-                            return mRenderer.openDocument(source);
-                        } catch (RemoteException re) {
-                            Log.e(LOG_TAG, "Cannot open PDF document");
-                            return PdfManipulationService.ERROR_MALFORMED_PDF_FILE;
-                        } finally {
-                            // Close the fd as we passed it to another process
-                            // which took ownership.
-                            IoUtils.closeQuietly(source);
-                        }
-                    }
-                }
-
-                @Override
-                public void onPostExecute(Integer pageCount) {
-                    switch (pageCount) {
-                        case PdfManipulationService.ERROR_MALFORMED_PDF_FILE: {
-                            mPageCount = PrintDocumentInfo.PAGE_COUNT_UNKNOWN;
-                            if (callback != null) {
-                                callback.onFailure(OpenDocumentCallback.ERROR_MALFORMED_PDF_FILE);
-                            }
-                        } break;
-                        case PdfManipulationService.ERROR_SECURE_PDF_FILE: {
-                            mPageCount = PrintDocumentInfo.PAGE_COUNT_UNKNOWN;
-                            if (callback != null) {
-                                callback.onFailure(OpenDocumentCallback.ERROR_SECURE_PDF_FILE);
-                            }
-                        } break;
-                        default: {
-                            mPageCount = pageCount;
-                            if (callback != null) {
-                                callback.onSuccess();
-                            }
-                        } break;
-                    }
-                }
-            }.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR);
+            mOpenTask = new OpenTask(source, callback);
+            mOpenTask.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR);
         }
 
         public void close(final Runnable callback) {
@@ -549,6 +491,11 @@ public final class PageContentRepository {
                 mBoundToService = false;
                 mContext.unbindService(AsyncRenderer.this);
             }
+
+            if (mOpenTask != null) {
+                mOpenTask.cancel();
+            }
+
             mPageContentCache.invalidate();
             mPageContentCache.clear();
             mDestroyed = true;
@@ -687,6 +634,91 @@ public final class PageContentRepository {
             }
         }
 
+        private final class OpenTask extends AsyncTask<Void, Void, Integer> {
+            private final ParcelFileDescriptor mSource;
+            private final OpenDocumentCallback mCallback;
+
+            public OpenTask(ParcelFileDescriptor source, OpenDocumentCallback callback) {
+                mSource = source;
+                mCallback = callback;
+            }
+
+            @Override
+            protected void onPreExecute() {
+                if (mDestroyed) {
+                    cancel(true);
+                    return;
+                }
+                Intent intent = new Intent(PdfManipulationService.ACTION_GET_RENDERER);
+                intent.setClass(mContext, PdfManipulationService.class);
+                intent.setData(Uri.fromParts("fake-scheme", String.valueOf(
+                        AsyncRenderer.this.hashCode()), null));
+                mContext.bindService(intent, AsyncRenderer.this, Context.BIND_AUTO_CREATE);
+                mBoundToService = true;
+            }
+
+            @Override
+            protected Integer doInBackground(Void... params) {
+                synchronized (mLock) {
+                    while (mRenderer == null && !isCancelled()) {
+                        try {
+                            mLock.wait();
+                        } catch (InterruptedException ie) {
+                                /* ignore */
+                        }
+                    }
+                    try {
+                        return mRenderer.openDocument(mSource);
+                    } catch (RemoteException re) {
+                        Log.e(LOG_TAG, "Cannot open PDF document");
+                        return PdfManipulationService.ERROR_MALFORMED_PDF_FILE;
+                    } finally {
+                        // Close the fd as we passed it to another process
+                        // which took ownership.
+                        IoUtils.closeQuietly(mSource);
+                    }
+                }
+            }
+
+            @Override
+            public void onPostExecute(Integer pageCount) {
+                switch (pageCount) {
+                    case PdfManipulationService.ERROR_MALFORMED_PDF_FILE: {
+                        mPageCount = PrintDocumentInfo.PAGE_COUNT_UNKNOWN;
+                        if (mCallback != null) {
+                            mCallback.onFailure(OpenDocumentCallback.ERROR_MALFORMED_PDF_FILE);
+                        }
+                    } break;
+                    case PdfManipulationService.ERROR_SECURE_PDF_FILE: {
+                        mPageCount = PrintDocumentInfo.PAGE_COUNT_UNKNOWN;
+                        if (mCallback != null) {
+                            mCallback.onFailure(OpenDocumentCallback.ERROR_SECURE_PDF_FILE);
+                        }
+                    } break;
+                    default: {
+                        mPageCount = pageCount;
+                        if (mCallback != null) {
+                            mCallback.onSuccess();
+                        }
+                    } break;
+                }
+
+                mOpenTask = null;
+            }
+
+            @Override
+            protected void onCancelled(Integer integer) {
+                mOpenTask = null;
+            }
+
+            public void cancel() {
+                cancel(true);
+                synchronized(mLock) {
+                    mLock.notifyAll();
+                }
+            }
+        }
+
         private final class RenderPageTask extends AsyncTask<Void, Void, RenderedPage> {
             final int mPageIndex;
             final RenderSpec mRenderSpec;

diff --git a/packages/PrintSpooler/src/com/android/printspooler/ui/FusedPrintersProvider.java b/packages/PrintSpooler/src/com/android/printspooler/ui/FusedPrintersProvider.java
index 02d2715..22a7f86 100644 (file)
--- a/packages/PrintSpooler/src/com/android/printspooler/ui/FusedPrintersProvider.java
+++ b/packages/PrintSpooler/src/com/android/printspooler/ui/FusedPrintersProvider.java
@@ -346,7 +346,6 @@ public final class FusedPrintersProvider extends Loader<List<PrinterInfo>> {
         private List<PrinterInfo> mHistoricalPrinters = new ArrayList<>();
 
         private boolean mReadHistoryCompleted;
-        private boolean mReadHistoryInProgress;
 
         private ReadTask mReadTask;
 
@@ -358,7 +357,7 @@ public final class FusedPrintersProvider extends Loader<List<PrinterInfo>> {
         }
 
         public boolean isReadHistoryInProgress() {
-            return mReadHistoryInProgress;
+            return mReadTask != null;
         }
 
         public boolean isReadHistoryCompleted() {
@@ -366,9 +365,7 @@ public final class FusedPrintersProvider extends Loader<List<PrinterInfo>> {
         }
 
         public boolean stopReadPrinterHistory() {
-            final boolean cancelled = mReadTask.cancel(true);
-            mReadTask = null;
-            return cancelled;
+            return mReadTask.cancel(true);
         }
 
         public void readPrinterHistory() {
@@ -376,7 +373,6 @@ public final class FusedPrintersProvider extends Loader<List<PrinterInfo>> {
                 Log.i(LOG_TAG, "read history started "
                         + FusedPrintersProvider.this.hashCode());
             }
-            mReadHistoryInProgress = true;
             mReadTask = new ReadTask();
             mReadTask.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR, (Void[]) null);
         }
@@ -534,15 +530,20 @@ public final class FusedPrintersProvider extends Loader<List<PrinterInfo>> {
                 mFavoritePrinters.clear();
                 mFavoritePrinters.addAll(computeFavoritePrinters(mHistoricalPrinters));
 
-                mReadHistoryInProgress = false;
                 mReadHistoryCompleted = true;
 
                 // Deliver the printers.
                 updatePrinters(mDiscoverySession.getPrinters(), mFavoritePrinters);
 
+                // We are done.
+                mReadTask = null;
+
                 // Loading the available printers if needed.
                 loadInternal();
+            }
 
+            @Override
+            protected void onCancelled(List<PrinterInfo> printerInfos) {
                 // We are done.
                 mReadTask = null;
             }

diff --git a/packages/PrintSpooler/src/com/android/printspooler/ui/PrintActivity.java b/packages/PrintSpooler/src/com/android/printspooler/ui/PrintActivity.java
index b76a9cd..71d5340 100644 (file)
--- a/packages/PrintSpooler/src/com/android/printspooler/ui/PrintActivity.java
+++ b/packages/PrintSpooler/src/com/android/printspooler/ui/PrintActivity.java
@@ -291,6 +291,9 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat
                 if (isFinishing() || (isFinalState(mState) && !mPrintedDocument.isUpdating())) {
                     return;
                 }
+                if (mPrintedDocument.isUpdating()) {
+                    mPrintedDocument.cancel();
+                }
                 setState(STATE_PRINT_CANCELED);
                 doFinish();
             }
@@ -558,7 +561,9 @@ public class PrintActivity extends Activity implements RemotePrintDocument.Updat
     @Override
     public void onConfigurationChanged(Configuration newConfig) {
         super.onConfigurationChanged(newConfig);
-        mPrintPreviewController.onOrientationChanged();
+        if (mPrintPreviewController != null) {
+            mPrintPreviewController.onOrientationChanged();
+        }
     }
 
     @Override