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

9 files changed, 120 insertions, 65 deletions

diff --git a/core/java/android/printservice/PrinterDiscoverySession.java b/core/java/android/printservice/PrinterDiscoverySession.java
index 1f86ecc..6464cc1 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 4b68f52..0000000
--- a/packages/PrintSpooler/res/drawable-hdpi/ic_menu_add.png
+++ /dev/null
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 15ffadd..0000000
--- a/packages/PrintSpooler/res/drawable-mdpi/ic_menu_add.png
+++ /dev/null
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 420510e..0000000
--- a/packages/PrintSpooler/res/drawable-xhdpi/ic_menu_add.png
+++ /dev/null
diff --git a/packages/PrintSpooler/res/menu/select_printer_activity.xml b/packages/PrintSpooler/res/menu/select_printer_activity.xml
index 28fbd35..d4ce1cf 100644
--- a/packages/PrintSpooler/res/menu/select_printer_activity.xml
+++ b/packages/PrintSpooler/res/menu/select_printer_activity.xml
@@ -29,7 +29,7 @@
     <item
         android:id="@+id/action_add_printer"
         android:title="@null"
-        android:icon="@drawable/ic_menu_add"
+        android:icon="@*android:drawable/create_contact"
         android:showAsAction="ifRoom"
         android:alphabeticShortcut="a">
     </item>
diff --git a/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java b/packages/PrintSpooler/src/com/android/printspooler/FusedPrintersProvider.java
index ad8d95a..88da21f 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<List<PrinterInfo>> {
 
     private static final int MAX_FAVORITE_PRINTER_COUNT = 4;
 
-    private final Map<PrinterId, PrinterInfo> mPrinters =
-            new LinkedHashMap<PrinterId, PrinterInfo>();
+    private final List<PrinterInfo> mPrinters =
+            new ArrayList<PrinterInfo>();
+
+    private final List<PrinterInfo> mFavoritePrinters =
+            new ArrayList<PrinterInfo>();
 
     private final PersistenceManager mPersistenceManager;
 
     private PrinterDiscoverySession mDiscoverySession;
 
-    private List<PrinterInfo> mFavoritePrinters;
-
     private PrinterId mTrackedPrinter;
 
     public FusedPrintersProvider(Context context) {
@@ -86,15 +86,40 @@ public class FusedPrintersProvider extends Loader<List<PrinterInfo>> {
         mPersistenceManager.addPrinterAndWritePrinterHistory(printer);
     }
 
-    public List<PrinterInfo> getPrinters() {
-        return new ArrayList<PrinterInfo>(mPrinters.values());
-    }
+    private void computeAndDeliverResult() {
+        if (!isStarted()) {
+            return;
+        }
 
-    @Override
-    public void deliverResult(List<PrinterInfo> printers) {
-        if (isStarted()) {
-            super.deliverResult(printers);
+        List<PrinterInfo> printers = new ArrayList<PrinterInfo>();
+
+        // 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<List<PrinterInfo>> {
         }
         // The contract is that if we already have a valid,
         // result the we have to deliver it immediately.
-        if (!mPrinters.isEmpty()) {
-            deliverResult(new ArrayList<PrinterInfo>(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<List<PrinterInfo>> {
         if (DEBUG) {
             Log.i(LOG_TAG, "onForceLoad()" + FusedPrintersProvider.this.hashCode());
         }
-        onCancelLoad();
         loadInternal();
     }
 
@@ -139,8 +161,9 @@ public class FusedPrintersProvider extends Loader<List<PrinterInfo>> {
             mDiscoverySession.setOnPrintersChangeListener(new OnPrintersChangeListener() {
                 @Override
                 public void onPrintersChanged() {
-                    deliverResult(new ArrayList<PrinterInfo>(
-                            mDiscoverySession.getPrinters()));
+                    mPrinters.clear();
+                    mPrinters.addAll(mDiscoverySession.getPrinters());
+                    computeAndDeliverResult();
                 }
             });
             mPersistenceManager.readPrinterHistory();
@@ -244,27 +267,20 @@ public class FusedPrintersProvider extends Loader<List<PrinterInfo>> {
             @Override
             protected void onPostExecute(List<PrinterInfo> 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<List<PrinterInfo>> {
                         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<List<PrinterInfo>> {
                 mHistoricalPrinters.remove(0);
             }
             mHistoricalPrinters.add(printer);
-            mWriteTask.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR, mHistoricalPrinters);
+            mWriteTask.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR,
+                    new ArrayList<PrinterInfo>(mHistoricalPrinters));
         }
 
         private List<PrinterInfo> computeFavoritePrinters(List<PrinterInfo> printers) {
diff --git a/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java b/packages/PrintSpooler/src/com/android/printspooler/PrintJobConfigActivity.java
index 520331c..5361a1e 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 14af9d8..3c67aa9 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 7d94a42..b9c676d 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<RemotePrintService> services = new ArrayList<RemotePrintService>(
                     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<PrinterInfo> 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<PrinterInfo> 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<PrinterInfo> 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;