From 9186d0cb2bd325d9b52da15dbd513937c1e42caa Mon Sep 17 00:00:00 2001 From: Svetoslav Ganov Date: Tue, 3 Sep 2013 00:11:58 -0700 Subject: Bug fixes in the printer dialog activity and fused printer loader. 1. Fused printers loader was not using the discovered printers to update the historical ones. Now if a historical printer is discovered we update its state with the discovered, i.e. most recent, information. 2. Fixed a bug in the destination adapter of the print job config dialog that was leading to a crash if all printers item is selected when there are no discovered printers. 3. Updated the add printers asset in the all printers screen. 4. Historical printers were not persisted by the print dialog activity. 5. Reduced the number of printers we send per transation to avoid the binder transaction size limit. Added sending of printers in chunks in a place this was missing. Change-Id: I88b54888360bc0e53b06bd260c2b832d0d6454b6 --- .../printservice/PrinterDiscoverySession.java | 2 +- .../PrintSpooler/res/drawable-hdpi/ic_menu_add.png | Bin 667 -> 0 bytes .../PrintSpooler/res/drawable-mdpi/ic_menu_add.png | Bin 596 -> 0 bytes .../res/drawable-xhdpi/ic_menu_add.png | Bin 761 -> 0 bytes .../res/menu/select_printer_activity.xml | 2 +- .../printspooler/FusedPrintersProvider.java | 88 +++++++++++++-------- .../printspooler/PrintJobConfigActivity.java | 36 +++++++-- .../android/server/print/RemotePrintService.java | 12 +-- .../java/com/android/server/print/UserState.java | 45 +++++++---- 9 files changed, 120 insertions(+), 65 deletions(-) delete mode 100644 packages/PrintSpooler/res/drawable-hdpi/ic_menu_add.png delete mode 100644 packages/PrintSpooler/res/drawable-mdpi/ic_menu_add.png delete mode 100644 packages/PrintSpooler/res/drawable-xhdpi/ic_menu_add.png diff --git a/core/java/android/printservice/PrinterDiscoverySession.java b/core/java/android/printservice/PrinterDiscoverySession.java index 1f86ecc8bbd1..6464cc117108 100644 --- a/core/java/android/printservice/PrinterDiscoverySession.java +++ b/core/java/android/printservice/PrinterDiscoverySession.java @@ -80,7 +80,7 @@ import java.util.List; public abstract class PrinterDiscoverySession { private static final String LOG_TAG = "PrinterDiscoverySession"; - private static final int MAX_ITEMS_PER_CALLBACK = 100; + private static final int MAX_ITEMS_PER_CALLBACK = 50; private static int sIdCounter = 0; diff --git a/packages/PrintSpooler/res/drawable-hdpi/ic_menu_add.png b/packages/PrintSpooler/res/drawable-hdpi/ic_menu_add.png deleted file mode 100644 index 4b68f52ad0a4..000000000000 Binary files a/packages/PrintSpooler/res/drawable-hdpi/ic_menu_add.png and /dev/null differ diff --git a/packages/PrintSpooler/res/drawable-mdpi/ic_menu_add.png b/packages/PrintSpooler/res/drawable-mdpi/ic_menu_add.png deleted file mode 100644 index 15ffadd3606f..000000000000 Binary files a/packages/PrintSpooler/res/drawable-mdpi/ic_menu_add.png and /dev/null differ diff --git a/packages/PrintSpooler/res/drawable-xhdpi/ic_menu_add.png b/packages/PrintSpooler/res/drawable-xhdpi/ic_menu_add.png deleted file mode 100644 index 420510e935cf..000000000000 Binary files a/packages/PrintSpooler/res/drawable-xhdpi/ic_menu_add.png and /dev/null differ diff --git a/packages/PrintSpooler/res/menu/select_printer_activity.xml b/packages/PrintSpooler/res/menu/select_printer_activity.xml index 28fbd3536c4d..d4ce1cf17cb5 100644 --- a/packages/PrintSpooler/res/menu/select_printer_activity.xml +++ b/packages/PrintSpooler/res/menu/select_printer_activity.xml @@ -29,7 +29,7 @@ diff --git a/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java b/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java index ad8d95aa45dc..88da21f9cff2 100644 --- a/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java +++ b/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java @@ -47,7 +47,6 @@ import java.io.FileOutputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -66,15 +65,16 @@ public class FusedPrintersProvider extends Loader> { private static final int MAX_FAVORITE_PRINTER_COUNT = 4; - private final Map mPrinters = - new LinkedHashMap(); + private final List mPrinters = + new ArrayList(); + + private final List mFavoritePrinters = + new ArrayList(); private final PersistenceManager mPersistenceManager; private PrinterDiscoverySession mDiscoverySession; - private List mFavoritePrinters; - private PrinterId mTrackedPrinter; public FusedPrintersProvider(Context context) { @@ -86,15 +86,40 @@ public class FusedPrintersProvider extends Loader> { mPersistenceManager.addPrinterAndWritePrinterHistory(printer); } - public List getPrinters() { - return new ArrayList(mPrinters.values()); - } + private void computeAndDeliverResult() { + if (!isStarted()) { + return; + } - @Override - public void deliverResult(List printers) { - if (isStarted()) { - super.deliverResult(printers); + List printers = new ArrayList(); + + // We want the first few favorite printers on top of the list. + final int favoriteCount = Math.min(mFavoritePrinters.size(), + MAX_FAVORITE_PRINTER_COUNT); + for (int i = 0; i < favoriteCount; i++) { + printers.add(mFavoritePrinters.get(i)); } + + // Add discovered printers updating favorites if needed. + final int printerCount = mPrinters.size(); + for (int i = 0; i < printerCount; i++) { + PrinterInfo discoveredPrinter = mPrinters.get(i); + boolean printerHandled = false; + for (int j = 0; j< favoriteCount; j++) { + PrinterInfo favoritePrinter = printers.get(j); + if (favoritePrinter.getId().equals(discoveredPrinter.getId())) { + printers.set(j, discoveredPrinter); + printerHandled = true; + break; + } + } + if (!printerHandled) { + printers.add(discoveredPrinter); + } + } + + // Deliver the printers. + deliverResult(printers); } @Override @@ -104,14 +129,12 @@ public class FusedPrintersProvider extends Loader> { } // The contract is that if we already have a valid, // result the we have to deliver it immediately. - if (!mPrinters.isEmpty()) { - deliverResult(new ArrayList(mPrinters.values())); - } - // If the data has changed since the last load - // or is not available, start a load. - if (takeContentChanged() || mPrinters.isEmpty()) { - onForceLoad(); + if (!mPrinters.isEmpty() || !mFavoritePrinters.isEmpty()) { + computeAndDeliverResult(); } + // Always load the data to ensure discovery period is + // started and to make sure obsolete printers are updated. + onForceLoad(); } @Override @@ -127,7 +150,6 @@ public class FusedPrintersProvider extends Loader> { if (DEBUG) { Log.i(LOG_TAG, "onForceLoad()" + FusedPrintersProvider.this.hashCode()); } - onCancelLoad(); loadInternal(); } @@ -139,8 +161,9 @@ public class FusedPrintersProvider extends Loader> { mDiscoverySession.setOnPrintersChangeListener(new OnPrintersChangeListener() { @Override public void onPrintersChanged() { - deliverResult(new ArrayList( - mDiscoverySession.getPrinters())); + mPrinters.clear(); + mPrinters.addAll(mDiscoverySession.getPrinters()); + computeAndDeliverResult(); } }); mPersistenceManager.readPrinterHistory(); @@ -244,27 +267,20 @@ public class FusedPrintersProvider extends Loader> { @Override protected void onPostExecute(List printers) { if (DEBUG) { - Log.i(LOG_TAG, "read history completed" + FusedPrintersProvider.this.hashCode()); + Log.i(LOG_TAG, "read history completed" + + FusedPrintersProvider.this.hashCode()); } mHistoricalPrinters = printers; // Compute the favorite printers. - mFavoritePrinters = computeFavoritePrinters(printers); - - // We want the first few favorite printers on top of the list. - final int favoriteCount = Math.min(mFavoritePrinters.size(), - MAX_FAVORITE_PRINTER_COUNT); - for (int i = 0; i < favoriteCount; i++) { - PrinterInfo favoritePrinter = mFavoritePrinters.get(i); - mPrinters.put(favoritePrinter.getId(), favoritePrinter); - } + mFavoritePrinters.addAll(computeFavoritePrinters(printers)); mReadHistoryInProgress = false; mReadHistoryCompleted = true; // Deliver the favorites. - deliverResult(mFavoritePrinters); + computeAndDeliverResult(); // Start loading the available printers. loadInternal(); @@ -420,8 +436,9 @@ public class FusedPrintersProvider extends Loader> { serializer.startTag(null, TAG_PRINTER); serializer.attribute(null, ATTR_NAME, printer.getName()); + // Historical printers are always stored as unavailable. serializer.attribute(null, ATTR_STATUS, String.valueOf( - printer.getStatus())); + PrinterInfo.STATUS_UNAVAILABLE)); String description = printer.getDescription(); if (description != null) { serializer.attribute(null, ATTR_DESCRIPTION, description); @@ -488,7 +505,8 @@ public class FusedPrintersProvider extends Loader> { mHistoricalPrinters.remove(0); } mHistoricalPrinters.add(printer); - mWriteTask.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR, mHistoricalPrinters); + mWriteTask.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR, + new ArrayList(mHistoricalPrinters)); } private List computeFavoritePrinters(List printers) { diff --git a/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java b/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java index 520331cb58be..5361a1eba7c7 100644 --- a/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java +++ b/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java @@ -817,6 +817,10 @@ public class PrintJobConfigActivity extends Activity { } if (id == DEST_ADAPTER_ITEM_ID_ALL_PRINTERS) { + // The selection changed to the all printers item. We + // want to select back the last selected printer. + mIgnoreNextDestinationChange = true; + mEditor.selectPrinter(mCurrentPrinter.getId()); startSelectPrinterActivity(); return; } @@ -1024,7 +1028,7 @@ public class PrintJobConfigActivity extends Activity { mDestinationSpinnerAdapter.registerDataSetObserver(new DataSetObserver() { @Override public void onChanged() { - // Initially, we have only sage to PDF as a printer but after some + // Initially, we have only safe to PDF as a printer but after some // printers are loaded we want to select the user's favorite one // which is the first. if (!mFavoritePrinterSelected && mDestinationSpinnerAdapter.getCount() > 2) { @@ -1044,6 +1048,15 @@ public class PrintJobConfigActivity extends Activity { continue; } + // If the current printer became available and has no + // capabilities, we refresh it. + if (mCurrentPrinter.getStatus() == PrinterInfo.STATUS_UNAVAILABLE + && printer.getStatus() != PrinterInfo.STATUS_UNAVAILABLE + && printer.getCapabilities() == null) { + refreshCurrentPrinter(); + return; + } + // Update the UI if capabilities changed. boolean capabilitiesChanged = false; @@ -1127,6 +1140,18 @@ public class PrintJobConfigActivity extends Activity { } } + public void addCurrentPrinterToHistory() { + PrinterInfo printer = (PrinterInfo) mDestinationSpinner.getSelectedItem(); + if (printer != null) { + FusedPrintersProvider printersLoader = (FusedPrintersProvider) + (Loader) getLoaderManager().getLoader( + LOADER_ID_PRINTERS_LOADER); + if (printersLoader != null) { + printersLoader.addHistoricalPrinter(printer); + } + } + } + public void selectPrinter(PrinterId printerId) { mDestinationSpinnerAdapter.ensurePrinterShownPrinterShown(printerId); final int position = mDestinationSpinnerAdapter.getPrinterIndex(printerId); @@ -1348,6 +1373,7 @@ public class PrintJobConfigActivity extends Activity { } public void confirmPrint() { + addCurrentPrinterToHistory(); mEditorState = EDITOR_STATE_CONFIRMED_PRINT; showUi(UI_GENERATING_PRINT_JOB, null); } @@ -1772,7 +1798,6 @@ public class PrintJobConfigActivity extends Activity { mPageRangeEditText.setVisibility(View.INVISIBLE); mPageRangeTitle.setVisibility(View.INVISIBLE); } - mRangeOptionsSpinner.setEnabled(true); // Print/Print preview if (mDestinationSpinner.getSelectedItemId() @@ -1871,8 +1896,6 @@ public class PrintJobConfigActivity extends Activity { } private void startSelectPrinterActivity() { - mIgnoreNextDestinationChange = true; - mDestinationSpinner.setSelection(0); Intent intent = new Intent(PrintJobConfigActivity.this, SelectPrinterActivity.class); startActivityForResult(intent, ACTIVITY_REQUEST_SELECT_PRINTER); @@ -1959,6 +1982,9 @@ public class PrintJobConfigActivity extends Activity { if (position == 0) { return DEST_ADAPTER_ITEM_ID_SAVE_AS_PDF; } + if (position == 1) { + return DEST_ADAPTER_ITEM_ID_ALL_PRINTERS; + } } else { if (position == 1) { return DEST_ADAPTER_ITEM_ID_SAVE_AS_PDF; @@ -2059,7 +2085,7 @@ public class PrintJobConfigActivity extends Activity { PrinterInfo printer = (PrinterInfo) mPrinters.get(i); if (printer.getId().equals(mLastShownPrinterId)) { // If already in the list - do nothing. - if (i < getCount() - 1) { + if (i < getCount() - 2) { return; } // Else replace the last one. diff --git a/services/java/com/android/server/print/RemotePrintService.java b/services/java/com/android/server/print/RemotePrintService.java index 14af9d83aa5e..3c67aa9babd6 100644 --- a/services/java/com/android/server/print/RemotePrintService.java +++ b/services/java/com/android/server/print/RemotePrintService.java @@ -163,7 +163,7 @@ final class RemotePrintService implements DeathRecipient { if (isBound()) { if (DEBUG) { - Slog.i(LOG_TAG, "[user: " + mUserId + "] handleOnAllPrintJobsHandled()"); + Slog.i(LOG_TAG, "[user: " + mUserId + "] onAllPrintJobsHandled()"); } // If the service has a printer discovery session @@ -185,7 +185,7 @@ final class RemotePrintService implements DeathRecipient { // which means that there are no print jobs to be cancelled. if (isBound()) { if (DEBUG) { - Slog.i(LOG_TAG, "[user: " + mUserId + "] handleRequestCancelPrintJob()"); + Slog.i(LOG_TAG, "[user: " + mUserId + "] requestCancelPrintJob()"); } try { mPrintService.requestCancelPrintJob(printJob); @@ -215,7 +215,7 @@ final class RemotePrintService implements DeathRecipient { }); } else { if (DEBUG) { - Slog.i(LOG_TAG, "[user: " + mUserId + "] handleOnPrintJobQueued()"); + Slog.i(LOG_TAG, "[user: " + mUserId + "] onPrintJobQueued()"); } try { mPrintService.onPrintJobQueued(printJob); @@ -358,7 +358,7 @@ final class RemotePrintService implements DeathRecipient { }); } else { if (DEBUG) { - Slog.i(LOG_TAG, "[user: " + mUserId + "] handleValidatePrinters()"); + Slog.i(LOG_TAG, "[user: " + mUserId + "] validatePrinters()"); } try { mPrintService.validatePrinters(printerIds); @@ -385,7 +385,7 @@ final class RemotePrintService implements DeathRecipient { }); } else { if (DEBUG) { - Slog.i(LOG_TAG, "[user: " + mUserId + "] handleStartPrinterTracking()"); + Slog.i(LOG_TAG, "[user: " + mUserId + "] startPrinterTracking()"); } try { mPrintService.startPrinterStateTracking(printerId); @@ -412,7 +412,7 @@ final class RemotePrintService implements DeathRecipient { }); } else { if (DEBUG) { - Slog.i(LOG_TAG, "[user: " + mUserId + "] handleStopPrinterTracking()"); + Slog.i(LOG_TAG, "[user: " + mUserId + "] stopPrinterTracking()"); } try { mPrintService.stopPrinterStateTracking(printerId); diff --git a/services/java/com/android/server/print/UserState.java b/services/java/com/android/server/print/UserState.java index 7d94a42b666b..b9c676d0e28d 100644 --- a/services/java/com/android/server/print/UserState.java +++ b/services/java/com/android/server/print/UserState.java @@ -62,7 +62,7 @@ final class UserState implements PrintSpoolerCallbacks { private static final boolean DEBUG = true && Build.IS_DEBUGGABLE; - private static final int MAX_ITEMS_PER_CALLBACK = 100; + private static final int MAX_ITEMS_PER_CALLBACK = 50; private static final char COMPONENT_NAME_SEPARATOR = ':'; @@ -576,10 +576,12 @@ final class UserState implements PrintSpoolerCallbacks { // Remember we got a start request to match with an end. mStartedPrinterDiscoveryTokens.add(observer.asBinder()); + // The service are already performing discovery - nothing to do. if (mStartedPrinterDiscoveryTokens.size() > 1) { return; } + List services = new ArrayList( mActiveServices.values()); SomeArgs args = SomeArgs.obtain(); @@ -858,11 +860,7 @@ final class UserState implements PrintSpoolerCallbacks { final int observerCount = mDiscoveryObservers.beginBroadcast(); for (int i = 0; i < observerCount; i++) { IPrinterDiscoveryObserver observer = mDiscoveryObservers.getBroadcastItem(i); - try { - observer.onPrintersAdded(addedPrinters); - } catch (RemoteException re) { - Log.i(LOG_TAG, "Error dispatching added printers", re); - } + handlePrintersAdded(observer, addedPrinters); } mDiscoveryObservers.finishBroadcast(); } @@ -871,11 +869,7 @@ final class UserState implements PrintSpoolerCallbacks { final int observerCount = mDiscoveryObservers.beginBroadcast(); for (int i = 0; i < observerCount; i++) { IPrinterDiscoveryObserver observer = mDiscoveryObservers.getBroadcastItem(i); - try { - observer.onPrintersRemoved(removedPrinterIds); - } catch (RemoteException re) { - Log.i(LOG_TAG, "Error dispatching removed printers", re); - } + handlePrintersRemoved(observer, removedPrinterIds); } mDiscoveryObservers.finishBroadcast(); } @@ -884,11 +878,7 @@ final class UserState implements PrintSpoolerCallbacks { final int observerCount = mDiscoveryObservers.beginBroadcast(); for (int i = 0; i < observerCount; i++) { IPrinterDiscoveryObserver observer = mDiscoveryObservers.getBroadcastItem(i); - try { - observer.onPrintersUpdated(updatedPrinters); - } catch (RemoteException re) { - Log.i(LOG_TAG, "Error dispatching updated printers", re); - } + handlePrintersUpdated(observer, updatedPrinters); } mDiscoveryObservers.finishBroadcast(); } @@ -957,7 +947,7 @@ final class UserState implements PrintSpoolerCallbacks { final int start = i * MAX_ITEMS_PER_CALLBACK; final int end = Math.min(start + MAX_ITEMS_PER_CALLBACK, printerCount); List subPrinters = printers.subList(start, end); - observer.onPrintersAdded(subPrinters); + observer.onPrintersAdded(subPrinters); } } } catch (RemoteException re) { @@ -986,6 +976,27 @@ final class UserState implements PrintSpoolerCallbacks { } } + private void handlePrintersUpdated(IPrinterDiscoveryObserver observer, + List updatedPrinters) { + try { + final int printerCount = updatedPrinters.size(); + if (printerCount <= MAX_ITEMS_PER_CALLBACK) { + observer.onPrintersUpdated(updatedPrinters); + } else { + // Send the added printers in chunks avoiding the binder transaction limit. + final int transactionCount = (printerCount / MAX_ITEMS_PER_CALLBACK) + 1; + for (int i = 0; i < transactionCount; i++) { + final int start = i * MAX_ITEMS_PER_CALLBACK; + final int end = Math.min(start + MAX_ITEMS_PER_CALLBACK, printerCount); + List subPrinters = updatedPrinters.subList(start, end); + observer.onPrintersUpdated(subPrinters); + } + } + } catch (RemoteException re) { + Log.e(LOG_TAG, "Error sending updated printers", re); + } + } + private final class SessionHandler extends Handler { public static final int MSG_PRINTERS_ADDED = 1; public static final int MSG_PRINTERS_REMOVED = 2; -- cgit v1.2.3-59-g8ed1b