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

5 files changed, 283 insertions, 86 deletions

diff --git a/Android.mk b/Android.mk
index 166db4e..4e5427a 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 b12c922..9d384fb 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 0000000..4443df0
--- /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 dbd8278..955b4d8 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;
-                }
-
-                // Layout cancels write and overrides layout.
-                if (mLastWriteSpec != null) {
-                    IoUtils.closeQuietly(mLastWriteSpec.fd);
-                    mLastWriteSpec = null;
-                }
+                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;
+                    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;
+                    }
 
-                // Cancel the previous cancellable operation.When the
-                // cancellation completes we will do the pending work.
-                if (cancelPreviousCancellableOperationLocked()) {
-                    return;
+                    doPendingWorkLocked();
+                }
+            }
+            if (destroyed) {
+                try {
+                    callback.onLayoutFailed(null, sequence);
+                } catch (RemoteException re) {
+                    Log.i(LOG_TAG, "Error notifying for cancelled layout", re);
                 }
-
-                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;
+                    }
 
-                // 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;
 
-                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;
+                    }
 
-                // Cancel the previous cancellable operation.When the
-                // cancellation completes we will do the pending work.
-                if (cancelPreviousCancellableOperationLocked()) {
-                    return;
+                    doPendingWorkLocked();
+                }
+            }
+            if (destroyed) {
+                try {
+                    callback.onWriteFailed(null, sequence);
+                } catch (RemoteException re) {
+                    Log.i(LOG_TAG, "Error notifying for cancelled write", re);
                 }
-
-                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 2997707..c90cb1f 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()) {
-                return true;
-            }
-            if (event.isTracking() && !event.isCanceled()) {
-                if (!mController.isWorking()) {
-                    PrintJobConfigActivity.this.finish();
+        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;
             }
-            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<PrintJobConfigActivity> mWeakActvity;
+
+        public PrintDocumentAdapterObserver(PrintJobConfigActivity activity) {
+            mWeakActvity = new WeakReference<PrintJobConfigActivity>(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();
+                    }
+                });
+            }
+        }
+    }
 }