From b5f180608db6de123b54ae94de569ff1ebca705c Mon Sep 17 00:00:00 2001 From: Svetoslav Date: Mon, 23 Sep 2013 18:48:34 -0700 Subject: Multiple printer discovery session instances and other bugs. 1. The fused printers provider was dropping on the floor received printers if it was not active. It is in fact a loaded and if not active it should compute the printers and not deliver them until activated. This fixes an issue where opening the print dialog, then enabling a print service results in the printers reported by the service not showing up in the print dialog. 2. Printer discovery session was created twice which leads to incorrect behavior as the pint system is designed around the contract that there is a single printer discovery session per service at a time. This was possible due to an incorrect initialization of a member variable resulting in double session creation when the print service is connected. 3. When a print service is enabled during discovery we did not use the correct condition to start printer discovery resulting in starting it all the time even if not needed. Also if some of the printers that had to be tracked are reported by the service just enabled (typically historical printers) we did not ask the service to start tracking them. 4. Removed some logging. bug:10903343 Change-Id: I46c049471a4b099fc668df3aee2aaedc8d7786ac --- core/java/android/printservice/PrintService.java | 40 ++++++++++++++++++++++ .../printspooler/FusedPrintersProvider.java | 10 +++--- .../android/printspooler/PrintSpoolerService.java | 2 +- .../android/server/print/RemotePrintService.java | 17 +++++---- .../java/com/android/server/print/UserState.java | 17 +++++++-- 5 files changed, 68 insertions(+), 18 deletions(-) diff --git a/core/java/android/printservice/PrintService.java b/core/java/android/printservice/PrintService.java index e5ebf7754cf6..e73a53bee819 100644 --- a/core/java/android/printservice/PrintService.java +++ b/core/java/android/printservice/PrintService.java @@ -383,6 +383,10 @@ public abstract class PrintService extends Service { final int action = message.what; switch (action) { case MSG_CREATE_PRINTER_DISCOVERY_SESSION: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_CREATE_PRINTER_DISCOVERY_SESSION " + + getPackageName()); + } PrinterDiscoverySession session = onCreatePrinterDiscoverySession(); if (session == null) { throw new NullPointerException("session cannot be null"); @@ -396,6 +400,10 @@ public abstract class PrintService extends Service { } break; case MSG_DESTROY_PRINTER_DISCOVERY_SESSION: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_DESTROY_PRINTER_DISCOVERY_SESSION " + + getPackageName()); + } if (mDiscoverySession != null) { mDiscoverySession.destroy(); mDiscoverySession = null; @@ -403,6 +411,10 @@ public abstract class PrintService extends Service { } break; case MSG_START_PRINTER_DISCOVERY: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_START_PRINTER_DISCOVERY " + + getPackageName()); + } if (mDiscoverySession != null) { List priorityList = (ArrayList) message.obj; mDiscoverySession.startPrinterDiscovery(priorityList); @@ -410,12 +422,20 @@ public abstract class PrintService extends Service { } break; case MSG_STOP_PRINTER_DISCOVERY: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_STOP_PRINTER_DISCOVERY " + + getPackageName()); + } if (mDiscoverySession != null) { mDiscoverySession.stopPrinterDiscovery(); } } break; case MSG_VALIDATE_PRINTERS: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_VALIDATE_PRINTERS " + + getPackageName()); + } if (mDiscoverySession != null) { List printerIds = (List) message.obj; mDiscoverySession.validatePrinters(printerIds); @@ -423,6 +443,10 @@ public abstract class PrintService extends Service { } break; case MSG_START_PRINTER_STATE_TRACKING: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_START_PRINTER_STATE_TRACKING " + + getPackageName()); + } if (mDiscoverySession != null) { PrinterId printerId = (PrinterId) message.obj; mDiscoverySession.startPrinterStateTracking(printerId); @@ -430,6 +454,10 @@ public abstract class PrintService extends Service { } break; case MSG_STOP_PRINTER_STATE_TRACKING: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_STOP_PRINTER_STATE_TRACKING " + + getPackageName()); + } if (mDiscoverySession != null) { PrinterId printerId = (PrinterId) message.obj; mDiscoverySession.stopPrinterStateTracking(printerId); @@ -437,11 +465,19 @@ public abstract class PrintService extends Service { } break; case MSG_ON_REQUEST_CANCEL_PRINTJOB: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_ON_REQUEST_CANCEL_PRINTJOB " + + getPackageName()); + } PrintJobInfo printJobInfo = (PrintJobInfo) message.obj; onRequestCancelPrintJob(new PrintJob(printJobInfo, mClient)); } break; case MSG_ON_PRINTJOB_QUEUED: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_ON_PRINTJOB_QUEUED " + + getPackageName()); + } PrintJobInfo printJobInfo = (PrintJobInfo) message.obj; if (DEBUG) { Log.i(LOG_TAG, "Queued: " + printJobInfo); @@ -450,6 +486,10 @@ public abstract class PrintService extends Service { } break; case MSG_SET_CLEINT: { + if (DEBUG) { + Log.i(LOG_TAG, "MSG_SET_CLEINT " + + getPackageName()); + } mClient = (IPrintServiceClient) message.obj; if (mClient != null) { onConnected(); diff --git a/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java b/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java index 72f6f0afb4fb..b4eb08a566d2 100644 --- a/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java +++ b/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java @@ -86,10 +86,6 @@ public class FusedPrintersProvider extends Loader> { } private void computeAndDeliverResult(Map discoveredPrinters) { - if (!isStarted()) { - return; - } - List printers = new ArrayList(); // Add the updated favorite printers. @@ -123,8 +119,10 @@ public class FusedPrintersProvider extends Loader> { mPrinters.clear(); mPrinters.addAll(printers); - // Deliver the printers. - deliverResult(printers); + if (isStarted()) { + // Deliver the printers. + deliverResult(printers); + } } @Override diff --git a/packages/PrintSpooler/src/com/android/printspooler/PrintSpoolerService.java b/packages/PrintSpooler/src/com/android/printspooler/PrintSpoolerService.java index fb2c935c3aba..ce1f6ecc3ad6 100644 --- a/packages/PrintSpooler/src/com/android/printspooler/PrintSpoolerService.java +++ b/packages/PrintSpooler/src/com/android/printspooler/PrintSpoolerService.java @@ -74,7 +74,7 @@ public final class PrintSpoolerService extends Service { private static final String LOG_TAG = "PrintSpoolerService"; - private static final boolean DEBUG_PRINT_JOB_LIFECYCLE = true; + private static final boolean DEBUG_PRINT_JOB_LIFECYCLE = false; private static final boolean DEBUG_PERSISTENCE = false; diff --git a/services/java/com/android/server/print/RemotePrintService.java b/services/java/com/android/server/print/RemotePrintService.java index a20973e2a9c7..5b9dc287eff0 100644 --- a/services/java/com/android/server/print/RemotePrintService.java +++ b/services/java/com/android/server/print/RemotePrintService.java @@ -85,7 +85,7 @@ final class RemotePrintService implements DeathRecipient { private boolean mHasPrinterDiscoverySession; - private boolean mServiceDead; + private boolean mServiceDied; private List mDiscoveryPriorityList; @@ -107,7 +107,6 @@ final class RemotePrintService implements DeathRecipient { mSpooler = spooler; mHandler = new MyHandler(context.getMainLooper()); mPrintServiceClient = new RemotePrintServiceClient(this); - mServiceDead = true; } public ComponentName getComponentName() { @@ -157,7 +156,7 @@ final class RemotePrintService implements DeathRecipient { private void handleBinderDied() { mPrintService.asBinder().unlinkToDeath(this, 0); mPrintService = null; - mServiceDead = true; + mServiceDied = true; mCallbacks.onServiceDied(this); } @@ -171,7 +170,7 @@ final class RemotePrintService implements DeathRecipient { if (!isBound()) { // The service is dead and neither has active jobs nor discovery // session, so ensure we are unbound since the service has no work. - if (mServiceDead && !mHasPrinterDiscoverySession) { + if (mServiceDied && !mHasPrinterDiscoverySession) { ensureUnbound(); return; } @@ -286,7 +285,7 @@ final class RemotePrintService implements DeathRecipient { if (!isBound()) { // The service is dead and neither has active jobs nor discovery // session, so ensure we are unbound since the service has no work. - if (mServiceDead && !mHasActivePrintJobs) { + if (mServiceDied && !mHasActivePrintJobs) { ensureUnbound(); return; } @@ -556,15 +555,15 @@ final class RemotePrintService implements DeathRecipient { return; } // If the service died and there is a discovery session, recreate it. - if (mServiceDead && mHasPrinterDiscoverySession) { + if (mServiceDied && mHasPrinterDiscoverySession) { handleCreatePrinterDiscoverySession(); } // If the service died and there is discovery started, restart it. - if (mServiceDead && mDiscoveryPriorityList != null) { + if (mServiceDied && mDiscoveryPriorityList != null) { handleStartPrinterDiscovery(mDiscoveryPriorityList); } // If the service died and printers were tracked, start tracking. - if (mServiceDead && mTrackedPrinterList != null) { + if (mServiceDied && mTrackedPrinterList != null) { final int trackedPrinterCount = mTrackedPrinterList.size(); for (int i = 0; i < trackedPrinterCount; i++) { handleStartPrinterStateTracking(mTrackedPrinterList.get(i)); @@ -581,7 +580,7 @@ final class RemotePrintService implements DeathRecipient { if (!mHasPrinterDiscoverySession && !mHasActivePrintJobs) { ensureUnbound(); } - mServiceDead = false; + mServiceDied = false; } @Override diff --git a/services/java/com/android/server/print/UserState.java b/services/java/com/android/server/print/UserState.java index fd4a3a48dbd8..8c2182732cea 100644 --- a/services/java/com/android/server/print/UserState.java +++ b/services/java/com/android/server/print/UserState.java @@ -1007,12 +1007,25 @@ final class UserState implements PrintSpoolerCallbacks, PrintServiceCallbacks { mHandler.obtainMessage( SessionHandler.MSG_CREATE_PRINTER_DISCOVERY_SESSION, service).sendToTarget(); - // If there are some observers that started discovery - tell the service. - if (mDiscoveryObservers.getRegisteredCallbackCount() > 0) { + // Start printer discovery if necessary. + if (!mStartedPrinterDiscoveryTokens.isEmpty()) { mHandler.obtainMessage( SessionHandler.MSG_START_PRINTER_DISCOVERY, service).sendToTarget(); } + // Start tracking printers if necessary + final int trackedPrinterCount = mStateTrackedPrinters.size(); + for (int i = 0; i < trackedPrinterCount; i++) { + PrinterId printerId = mStateTrackedPrinters.get(i); + if (printerId.getServiceName().equals(service.getComponentName())) { + SomeArgs args = SomeArgs.obtain(); + args.arg1 = service; + args.arg2 = printerId; + mHandler.obtainMessage(SessionHandler + .MSG_START_PRINTER_STATE_TRACKING, args) + .sendToTarget(); + } + } } public void dump(PrintWriter pw, String prefix) { -- cgit v1.2.3-59-g8ed1b