From f6cd14dbc99b38af7afe1e5f72347395603d7de2 Mon Sep 17 00:00:00 2001 From: Svet Ganov Date: Thu, 20 Nov 2014 07:43:30 -0800 Subject: 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 --- .../printspooler/model/PageContentRepository.java | 158 +++++++++++++-------- .../printspooler/ui/FusedPrintersProvider.java | 15 +- .../com/android/printspooler/ui/PrintActivity.java | 7 +- 3 files changed, 109 insertions(+), 71 deletions(-) (limited to 'packages/PrintSpooler/src') 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 --- 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() { - @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 { + 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 { 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 --- 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> { private List mHistoricalPrinters = new ArrayList<>(); private boolean mReadHistoryCompleted; - private boolean mReadHistoryInProgress; private ReadTask mReadTask; @@ -358,7 +357,7 @@ public final class FusedPrintersProvider extends Loader> { } public boolean isReadHistoryInProgress() { - return mReadHistoryInProgress; + return mReadTask != null; } public boolean isReadHistoryCompleted() { @@ -366,9 +365,7 @@ public final class FusedPrintersProvider extends Loader> { } 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> { 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> { 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 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 --- 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 -- cgit v1.1