From 858a1850e2e1c4516129d27ecdf54aaeade606ca Mon Sep 17 00:00:00 2001 From: Svetoslav Ganov Date: Thu, 17 Oct 2013 22:20:40 -0700 Subject: [PATCH] Hide the print dialog if the printing activity is destroyed. 1. For an app to print it creates a PrintDocumentAdapter implementation which is passed to the print dialog activity. If the activity that created the adapter is destroyed then the adapter, which may rely on the activity state, may be in an invalid state. For example, an app creates an adapter and calls print resuting in the app activity and the print dialog activity being stacked. Now the user rotates the device which triggers the recreating of the activity stack (assume the app does not handle rotation). The recreated print dialog activity receives the intent that originally created it with containing the adapter that was constructed in the context of the old, now destroyed, app activity instance. To handle this we are limiting an app to be able to print only from and activity and when this activity is destroyed we mark the adapter as invalid which will result in hiding the print dialog activity. Note that if the app process is killed we already handle this in the print dialog activiy by registering a death recipient on the adapter binder. 2. In the PrintManager.PrintDocumentAdapterDelegate some of the state is accessed only on the main thread and some from miltiple threads. The code was trying to avoid locking for state that is not accessed by multiple threads but this is error prone and the benefit does not justify the complexity and added fragility. Now grabbing a lock all the time. 3. The PrintJobConfigActivity waits for it to bind to the print spooler service before instantiating its print controller and editor. However, these can be accessed by invoking some of the activity cycle callbacks. This change is adding null checks for the case where the activity callbacks are called before the binding to the spooler is completed. bug:11242661 Change-Id: Id906b3170e4f0a0553772dfa62686f06fdca0eaf --- Android.mk | 1 + .../android/print/IPrintDocumentAdapter.aidl | 2 + .../print/IPrintDocumentAdapterObserver.aidl | 26 ++ core/java/android/print/PrintManager.java | 275 +++++++++++++----- .../printspooler/PrintJobConfigActivity.java | 71 ++++- 5 files changed, 286 insertions(+), 89 deletions(-) create mode 100644 core/java/android/print/IPrintDocumentAdapterObserver.aidl diff --git a/Android.mk b/Android.mk index 166db4ee016bb..4e5427adab330 100644 --- a/Android.mk +++ b/Android.mk @@ -165,6 +165,7 @@ LOCAL_SRC_FILES += \ core/java/android/print/ILayoutResultCallback.aidl \ core/java/android/print/IPrinterDiscoveryObserver.aidl \ core/java/android/print/IPrintDocumentAdapter.aidl \ + core/java/android/print/IPrintDocumentAdapterObserver.aidl \ core/java/android/print/IPrintJobStateChangeListener.aidl \ core/java/android/print/IPrintManager.aidl \ core/java/android/print/IPrintSpooler.aidl \ diff --git a/core/java/android/print/IPrintDocumentAdapter.aidl b/core/java/android/print/IPrintDocumentAdapter.aidl index b12c922cca6e3..9d384fba874df 100644 --- a/core/java/android/print/IPrintDocumentAdapter.aidl +++ b/core/java/android/print/IPrintDocumentAdapter.aidl @@ -19,6 +19,7 @@ package android.print; import android.os.Bundle; import android.os.ParcelFileDescriptor; import android.print.ILayoutResultCallback; +import android.print.IPrintDocumentAdapterObserver; import android.print.IWriteResultCallback; import android.print.PageRange; import android.print.PrintAttributes; @@ -29,6 +30,7 @@ import android.print.PrintAttributes; * @hide */ oneway interface IPrintDocumentAdapter { + void setObserver(in IPrintDocumentAdapterObserver observer); void start(); void layout(in PrintAttributes oldAttributes, in PrintAttributes newAttributes, ILayoutResultCallback callback, in Bundle metadata, int sequence); diff --git a/core/java/android/print/IPrintDocumentAdapterObserver.aidl b/core/java/android/print/IPrintDocumentAdapterObserver.aidl new file mode 100644 index 0000000000000..4443df09ae385 --- /dev/null +++ b/core/java/android/print/IPrintDocumentAdapterObserver.aidl @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.print; + +/** + * Interface for observing the state of a print document adapter. + * + * @hide + */ +oneway interface IPrintDocumentAdapterObserver { + void onDestroy(); +} diff --git a/core/java/android/print/PrintManager.java b/core/java/android/print/PrintManager.java index dbd8278d3a1fb..955b4d8f457bd 100644 --- a/core/java/android/print/PrintManager.java +++ b/core/java/android/print/PrintManager.java @@ -16,6 +16,8 @@ package android.print; +import android.app.Activity; +import android.app.Application.ActivityLifecycleCallbacks; import android.content.Context; import android.content.IntentSender; import android.content.IntentSender.SendIntentException; @@ -302,8 +304,8 @@ public final class PrintManager { if (TextUtils.isEmpty(printJobName)) { throw new IllegalArgumentException("priintJobName cannot be empty"); } - PrintDocumentAdapterDelegate delegate = new PrintDocumentAdapterDelegate(documentAdapter, - mContext.getMainLooper()); + PrintDocumentAdapterDelegate delegate = new PrintDocumentAdapterDelegate( + mContext, documentAdapter); try { Bundle result = mService.print(printJobName, delegate, attributes, mContext.getPackageName(), mAppId, mUserId); @@ -369,17 +371,21 @@ public final class PrintManager { return new PrinterDiscoverySession(mService, mContext, mUserId); } - private static final class PrintDocumentAdapterDelegate extends IPrintDocumentAdapter.Stub { + private static final class PrintDocumentAdapterDelegate extends IPrintDocumentAdapter.Stub + implements ActivityLifecycleCallbacks { private final Object mLock = new Object(); private CancellationSignal mLayoutOrWriteCancellation; - private PrintDocumentAdapter mDocumentAdapter; // Strong reference OK - - // cleared in finish() + private Activity mActivity; // Strong reference OK - cleared in finish() + + private PrintDocumentAdapter mDocumentAdapter; // Strong reference OK - cleared in finish private Handler mHandler; // Strong reference OK - cleared in finish() + private IPrintDocumentAdapterObserver mObserver; // Strong reference OK - cleared in finish + private LayoutSpec mLastLayoutSpec; private WriteSpec mLastWriteSpec; @@ -390,16 +396,42 @@ public final class PrintManager { private boolean mFinishRequested; private boolean mFinished; - public PrintDocumentAdapterDelegate(PrintDocumentAdapter documentAdapter, Looper looper) { + private boolean mDestroyed; + + public PrintDocumentAdapterDelegate(Context context, + PrintDocumentAdapter documentAdapter) { + if (!(context instanceof Activity)) { + throw new IllegalStateException("Can print only from an activity"); + } + mActivity = (Activity) context; mDocumentAdapter = documentAdapter; - mHandler = new MyHandler(looper); + mHandler = new MyHandler(mActivity.getMainLooper()); + mActivity.getApplication().registerActivityLifecycleCallbacks(this); + } + + @Override + public void setObserver(IPrintDocumentAdapterObserver observer) { + final boolean destroyed; + synchronized (mLock) { + if (!mDestroyed) { + mObserver = observer; + } + destroyed = mDestroyed; + } + if (destroyed) { + try { + observer.onDestroy(); + } catch (RemoteException re) { + Log.e(LOG_TAG, "Error announcing destroyed state", re); + } + } } @Override public void start() { synchronized (mLock) { - // Started or finished - nothing to do. - if (mStartReqeusted || mFinishRequested) { + // Started called or finish called or destroyed - nothing to do. + if (mStartReqeusted || mFinishRequested || mDestroyed) { return; } @@ -412,71 +444,85 @@ public final class PrintManager { @Override public void layout(PrintAttributes oldAttributes, PrintAttributes newAttributes, ILayoutResultCallback callback, Bundle metadata, int sequence) { + final boolean destroyed; synchronized (mLock) { - // Start not called or finish called - nothing to do. - if (!mStartReqeusted || mFinishRequested) { - return; + destroyed = mDestroyed; + // If start called and not finished called and not destroyed - do some work. + if (mStartReqeusted && !mFinishRequested && !mDestroyed) { + // Layout cancels write and overrides layout. + if (mLastWriteSpec != null) { + IoUtils.closeQuietly(mLastWriteSpec.fd); + mLastWriteSpec = null; + } + + mLastLayoutSpec = new LayoutSpec(); + mLastLayoutSpec.callback = callback; + mLastLayoutSpec.oldAttributes = oldAttributes; + mLastLayoutSpec.newAttributes = newAttributes; + mLastLayoutSpec.metadata = metadata; + mLastLayoutSpec.sequence = sequence; + + // Cancel the previous cancellable operation.When the + // cancellation completes we will do the pending work. + if (cancelPreviousCancellableOperationLocked()) { + return; + } + + doPendingWorkLocked(); } - - // Layout cancels write and overrides layout. - if (mLastWriteSpec != null) { - IoUtils.closeQuietly(mLastWriteSpec.fd); - mLastWriteSpec = null; + } + if (destroyed) { + try { + callback.onLayoutFailed(null, sequence); + } catch (RemoteException re) { + Log.i(LOG_TAG, "Error notifying for cancelled layout", re); } - - mLastLayoutSpec = new LayoutSpec(); - mLastLayoutSpec.callback = callback; - mLastLayoutSpec.oldAttributes = oldAttributes; - mLastLayoutSpec.newAttributes = newAttributes; - mLastLayoutSpec.metadata = metadata; - mLastLayoutSpec.sequence = sequence; - - // Cancel the previous cancellable operation.When the - // cancellation completes we will do the pending work. - if (cancelPreviousCancellableOperationLocked()) { - return; - } - - doPendingWorkLocked(); } } @Override public void write(PageRange[] pages, ParcelFileDescriptor fd, IWriteResultCallback callback, int sequence) { + final boolean destroyed; synchronized (mLock) { - // Start not called or finish called - nothing to do. - if (!mStartReqeusted || mFinishRequested) { - return; + destroyed = mDestroyed; + // If start called and not finished called and not destroyed - do some work. + if (mStartReqeusted && !mFinishRequested && !mDestroyed) { + // Write cancels previous writes. + if (mLastWriteSpec != null) { + IoUtils.closeQuietly(mLastWriteSpec.fd); + mLastWriteSpec = null; + } + + mLastWriteSpec = new WriteSpec(); + mLastWriteSpec.callback = callback; + mLastWriteSpec.pages = pages; + mLastWriteSpec.fd = fd; + mLastWriteSpec.sequence = sequence; + + // Cancel the previous cancellable operation.When the + // cancellation completes we will do the pending work. + if (cancelPreviousCancellableOperationLocked()) { + return; + } + + doPendingWorkLocked(); } - - // Write cancels previous writes. - if (mLastWriteSpec != null) { - IoUtils.closeQuietly(mLastWriteSpec.fd); - mLastWriteSpec = null; + } + if (destroyed) { + try { + callback.onWriteFailed(null, sequence); + } catch (RemoteException re) { + Log.i(LOG_TAG, "Error notifying for cancelled write", re); } - - mLastWriteSpec = new WriteSpec(); - mLastWriteSpec.callback = callback; - mLastWriteSpec.pages = pages; - mLastWriteSpec.fd = fd; - mLastWriteSpec.sequence = sequence; - - // Cancel the previous cancellable operation.When the - // cancellation completes we will do the pending work. - if (cancelPreviousCancellableOperationLocked()) { - return; - } - - doPendingWorkLocked(); } } @Override public void finish() { synchronized (mLock) { - // Start not called or finish called - nothing to do. - if (!mStartReqeusted || mFinishRequested) { + // Start not called or finish called or destroyed - nothing to do. + if (!mStartReqeusted || mFinishRequested || mDestroyed) { return; } @@ -495,15 +541,78 @@ public final class PrintManager { } } + @Override + public void onActivityPaused(Activity activity) { + /* do nothing */ + } + + @Override + public void onActivityCreated(Activity activity, Bundle savedInstanceState) { + /* do nothing */ + } + + @Override + public void onActivityStarted(Activity activity) { + /* do nothing */ + } + + @Override + public void onActivityResumed(Activity activity) { + /* do nothing */ + } + + @Override + public void onActivityStopped(Activity activity) { + /* do nothing */ + } + + @Override + public void onActivitySaveInstanceState(Activity activity, Bundle outState) { + /* do nothing */ + } + + @Override + public void onActivityDestroyed(Activity activity) { + // We really care only if the activity is being destroyed to + // notify the the print spooler so it can close the print dialog. + // Note the the spooler has a death recipient that observes if + // this process gets killed so we cover the case of onDestroy not + // being called due to this process being killed to reclaim memory. + final IPrintDocumentAdapterObserver observer; + synchronized (mLock) { + if (activity == mActivity) { + mDestroyed = true; + observer = mObserver; + clearLocked(); + } else { + observer = null; + activity = null; + } + } + if (observer != null) { + activity.getApplication().unregisterActivityLifecycleCallbacks( + PrintDocumentAdapterDelegate.this); + try { + observer.onDestroy(); + } catch (RemoteException re) { + Log.e(LOG_TAG, "Error announcing destroyed state", re); + } + } + } + private boolean isFinished() { return mDocumentAdapter == null; } - private void doFinish() { + private void clearLocked() { + mActivity = null; mDocumentAdapter = null; mHandler = null; - synchronized (mLock) { - mLayoutOrWriteCancellation = null; + mLayoutOrWriteCancellation = null; + mLastLayoutSpec = null; + if (mLastWriteSpec != null) { + IoUtils.closeQuietly(mLastWriteSpec.fd); + mLastWriteSpec = null; } } @@ -564,63 +673,81 @@ public final class PrintManager { } switch (message.what) { case MSG_START: { - mDocumentAdapter.onStart(); - } - break; + final PrintDocumentAdapter adapter; + synchronized (mLock) { + adapter = mDocumentAdapter; + } + if (adapter != null) { + adapter.onStart(); + } + } break; case MSG_LAYOUT: { + final PrintDocumentAdapter adapter; final CancellationSignal cancellation; final LayoutSpec layoutSpec; synchronized (mLock) { + adapter = mDocumentAdapter; layoutSpec = mLastLayoutSpec; mLastLayoutSpec = null; cancellation = new CancellationSignal(); mLayoutOrWriteCancellation = cancellation; } - if (layoutSpec != null) { + if (layoutSpec != null && adapter != null) { if (DEBUG) { Log.i(LOG_TAG, "Performing layout"); } - mDocumentAdapter.onLayout(layoutSpec.oldAttributes, + adapter.onLayout(layoutSpec.oldAttributes, layoutSpec.newAttributes, cancellation, new MyLayoutResultCallback(layoutSpec.callback, layoutSpec.sequence), layoutSpec.metadata); } - } - break; + } break; case MSG_WRITE: { + final PrintDocumentAdapter adapter; final CancellationSignal cancellation; final WriteSpec writeSpec; synchronized (mLock) { + adapter = mDocumentAdapter; writeSpec = mLastWriteSpec; mLastWriteSpec = null; cancellation = new CancellationSignal(); mLayoutOrWriteCancellation = cancellation; } - if (writeSpec != null) { + if (writeSpec != null && adapter != null) { if (DEBUG) { Log.i(LOG_TAG, "Performing write"); } - mDocumentAdapter.onWrite(writeSpec.pages, writeSpec.fd, + adapter.onWrite(writeSpec.pages, writeSpec.fd, cancellation, new MyWriteResultCallback(writeSpec.callback, writeSpec.fd, writeSpec.sequence)); } - } - break; + } break; case MSG_FINISH: { if (DEBUG) { Log.i(LOG_TAG, "Performing finish"); } - mDocumentAdapter.onFinish(); - doFinish(); - } - break; + final PrintDocumentAdapter adapter; + final Activity activity; + synchronized (mLock) { + adapter = mDocumentAdapter; + activity = mActivity; + clearLocked(); + } + if (adapter != null) { + adapter.onFinish(); + } + if (activity != null) { + activity.getApplication().unregisterActivityLifecycleCallbacks( + PrintDocumentAdapterDelegate.this); + } + } break; default: { throw new IllegalArgumentException("Unknown message: " diff --git a/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java b/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java index 2997707c8cab4..c90cb1f56f68f 100644 --- a/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java +++ b/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java @@ -40,6 +40,7 @@ import android.os.Message; import android.os.RemoteException; import android.print.ILayoutResultCallback; import android.print.IPrintDocumentAdapter; +import android.print.IPrintDocumentAdapterObserver; import android.print.IWriteResultCallback; import android.print.PageRange; import android.print.PrintAttributes; @@ -201,6 +202,14 @@ public class PrintJobConfigActivity extends Activity { throw new IllegalArgumentException("PrintDocumentAdapter cannot be null"); } + try { + IPrintDocumentAdapter.Stub.asInterface(mIPrintDocumentAdapter) + .setObserver(new PrintDocumentAdapterObserver(this)); + } catch (RemoteException re) { + finish(); + return; + } + PrintAttributes attributes = printJob.getAttributes(); if (attributes != null) { mCurrPrintAttributes.copyFrom(attributes); @@ -249,27 +258,29 @@ public class PrintJobConfigActivity extends Activity { // We can safely do the work in here since at this point // the system is bound to our (spooler) process which // guarantees that this process will not be killed. - if (mController.hasStarted()) { + if (mController != null && mController.hasStarted()) { mController.finish(); } - if (mEditor.isPrintConfirmed() && mController.isFinished()) { - mSpoolerProvider.getSpooler().setPrintJobState(mPrintJobId, - PrintJobInfo.STATE_QUEUED, null); + if (mEditor != null && mEditor.isPrintConfirmed() + && mController != null && mController.isFinished()) { + mSpoolerProvider.getSpooler().setPrintJobState(mPrintJobId, + PrintJobInfo.STATE_QUEUED, null); } else { mSpoolerProvider.getSpooler().setPrintJobState(mPrintJobId, PrintJobInfo.STATE_CANCELED, null); } - mIPrintDocumentAdapter.unlinkToDeath(mDeathRecipient, 0); if (mGeneratingPrintJobDialog != null) { mGeneratingPrintJobDialog.dismiss(); mGeneratingPrintJobDialog = null; } + mIPrintDocumentAdapter.unlinkToDeath(mDeathRecipient, 0); mSpoolerProvider.destroy(); super.onDestroy(); } public boolean onTouchEvent(MotionEvent event) { - if (!mEditor.isPrintConfirmed() && mEditor.shouldCloseOnTouch(event)) { + if (mController != null && mEditor != null && + !mEditor.isPrintConfirmed() && mEditor.shouldCloseOnTouch(event)) { if (!mController.isWorking()) { PrintJobConfigActivity.this.finish(); } @@ -287,17 +298,19 @@ public class PrintJobConfigActivity extends Activity { } public boolean onKeyUp(int keyCode, KeyEvent event) { - if (keyCode == KeyEvent.KEYCODE_BACK) { - if (mEditor.isShwoingGeneratingPrintJobUi()) { + if (mController != null && mEditor != null) { + if (keyCode == KeyEvent.KEYCODE_BACK) { + if (mEditor.isShwoingGeneratingPrintJobUi()) { + return true; + } + if (event.isTracking() && !event.isCanceled()) { + if (!mController.isWorking()) { + PrintJobConfigActivity.this.finish(); + } + } + mEditor.cancel(); return true; } - if (event.isTracking() && !event.isCanceled()) { - if (!mController.isWorking()) { - PrintJobConfigActivity.this.finish(); - } - } - mEditor.cancel(); - return true; } return super.onKeyUp(keyCode, event); } @@ -2703,4 +2716,32 @@ public class PrintJobConfigActivity extends Activity { /* do noting - we are in the same process */ } } + + private static final class PrintDocumentAdapterObserver + extends IPrintDocumentAdapterObserver.Stub { + private final WeakReference mWeakActvity; + + public PrintDocumentAdapterObserver(PrintJobConfigActivity activity) { + mWeakActvity = new WeakReference(activity); + } + + @Override + public void onDestroy() { + final PrintJobConfigActivity activity = mWeakActvity.get(); + if (activity != null) { + activity.mController.mHandler.post(new Runnable() { + @Override + public void run() { + if (activity.mController != null) { + activity.mController.cancel(); + } + if (activity.mEditor != null) { + activity.mEditor.cancel(); + } + activity.finish(); + } + }); + } + } + } }