Refactor Android printing code to make it more testable.

* Move printing logic from Tab to TabBase (i.e. to upstream), and
  also in the relevant files tab_android.*, TabPrinter.java.
* Remove obsolete Android printing feature detection code.
* Move PrintingControllerFactory logic into PrintingControllerImpl.
* Create a new PrintingControllerFactory, so the clients have a
ligher weight creation process (5-6 lines to 1).
* Instead of depending on Context to create a PrintManager, depend
  on an interface, namely PrintManagerDelegate.
* Remove setErrorText (move the logic inside factory).
* Remove the hardcoded default file name (use Printable#getTitle)
  instead.

BUG=315229

Review URL: https://codereview.chromium.org/63483007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@236256 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 130 insertions, 117 deletions

diff --git a/printing/android/java/src/org/chromium/printing/PrintManagerDelegate.java b/printing/android/java/src/org/chromium/printing/PrintManagerDelegate.java
new file mode 100644
index 0000000..6e2cb67
--- /dev/null
+++ b/printing/android/java/src/org/chromium/printing/PrintManagerDelegate.java
@@ -0,0 +1,23 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.printing;
+
+import android.print.PrintAttributes;
+import android.print.PrintDocumentAdapter;
+
+/**
+ * Defines an interface for the Android system printing service, for easier testing.
+ * We can't simply extend from {@link android.print.PrintManager}, since it's a final class.
+ */
+public interface PrintManagerDelegate {
+
+    /**
+     * Same as {@link android.print.PrintManager#print}, except this doesn't return a
+     * {@link android.print.PrintJob} since the clients don't need it.
+     */
+    void print(String printJobName,
+               PrintDocumentAdapter documentAdapter,
+               PrintAttributes attributes);
+}
diff --git a/printing/android/java/src/org/chromium/printing/PrintManagerDelegateImpl.java b/printing/android/java/src/org/chromium/printing/PrintManagerDelegateImpl.java
new file mode 100644
index 0000000..99422a4
--- /dev/null
+++ b/printing/android/java/src/org/chromium/printing/PrintManagerDelegateImpl.java
@@ -0,0 +1,27 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.printing;
+
+import android.print.PrintAttributes;
+import android.print.PrintDocumentAdapter;
+import android.print.PrintManager;
+
+/**
+ * An implementation of {@link PrintManagerDelegate} using the Android framework print manager.
+ */
+public class PrintManagerDelegateImpl implements PrintManagerDelegate {
+    private final PrintManager mPrintManager;
+
+    public PrintManagerDelegateImpl(PrintManager printManager) {
+        mPrintManager = printManager;
+    }
+
+    @Override
+    public void print(String printJobName, PrintDocumentAdapter documentAdapter,
+            PrintAttributes attributes) {
+        mPrintManager.print(printJobName, documentAdapter, attributes);
+    }
+
+}
diff --git a/printing/android/java/src/org/chromium/printing/PrintingContext.java b/printing/android/java/src/org/chromium/printing/PrintingContext.java
index e63c978..175e331 100644
--- a/printing/android/java/src/org/chromium/printing/PrintingContext.java
+++ b/printing/android/java/src/org/chromium/printing/PrintingContext.java
@@ -8,9 +8,6 @@ import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
 import org.chromium.base.ThreadUtils;
 
-import android.content.Context;
-
-import android.util.Log;
 import android.util.SparseArray;
 
 /**
@@ -21,17 +18,6 @@ import android.util.SparseArray;
 @JNINamespace("printing")
 public class PrintingContext implements PrintingContextInterface {
 
-    private static final String TAG = "PrintingContext";
-
-    /** Whether the framework supports printing. */
-    public static final boolean sIsPrintingAvailable = isPrintingAvailable();
-
-    /**
-     * The full class name of the print manager used to test whether printing functionality is
-     * available.
-     */
-    private static final String PRINT_MANAGER_CLASS_NAME = "android.print.PrintManager";
-
     /**
      * Mapping from a file descriptor (as originally provided from
      * {@link PrintDocumentAdapter#onWrite}) to a PrintingContext.
@@ -39,7 +25,7 @@ public class PrintingContext implements PrintingContextInterface {
      * This is static because a static method of the native code (inside PrintingContextAndroid)
      * needs to find Java PrintingContext class corresponding to a file descriptor.
      **/
-    private static final SparseArray<PrintingContext> sPrintingContextMap =
+    private static final SparseArray<PrintingContext> PRINTING_CONTEXT_MAP =
             new SparseArray<PrintingContext>();
 
     /** The controller this object interacts with, which in turn communicates with the framework. */
@@ -48,27 +34,13 @@ public class PrintingContext implements PrintingContextInterface {
     /** The pointer to the native PrintingContextAndroid object. */
     private final int mNativeObject;
 
-    private PrintingContext(Context context, int ptr) {
-        mController = PrintingControllerFactory.getPrintingController(context);
+    private PrintingContext(int ptr) {
+        mController = PrintingControllerImpl.getInstance();
         mNativeObject = ptr;
     }
 
     /**
-     * @return Whether printing is supported by the platform.
-     */
-    private static boolean isPrintingAvailable() {
-        // TODO(cimamoglu): Get rid of reflection once Build.VERSION_CODES.KEY_LIME_PIE is fixed.
-        try {
-            Class.forName(PRINT_MANAGER_CLASS_NAME);
-        } catch (ClassNotFoundException e) {
-            Log.d(TAG, "PrintManager not found on device");
-            return false;
-        }
-        return true;
-    }
-
-    /**
-     * Updates sPrintingContextMap to map from the file descriptor to this object.
+     * Updates PRINTING_CONTEXT_MAP to map from the file descriptor to this object.
      * @param fileDescriptor The file descriptor passed down from
      *                       {@link PrintDocumentAdapter#onWrite}.
      * @param delete If true, delete the entry (if it exists). If false, add it to the map.
@@ -77,9 +49,9 @@ public class PrintingContext implements PrintingContextInterface {
     public void updatePrintingContextMap(int fileDescriptor, boolean delete) {
         ThreadUtils.assertOnUiThread();
         if (delete) {
-            sPrintingContextMap.remove(fileDescriptor);
+            PRINTING_CONTEXT_MAP.remove(fileDescriptor);
         } else {
-            sPrintingContextMap.put(fileDescriptor, this);
+            PRINTING_CONTEXT_MAP.put(fileDescriptor, this);
         }
     }
 
@@ -94,9 +66,9 @@ public class PrintingContext implements PrintingContextInterface {
     }
 
     @CalledByNative
-    public static PrintingContext create(Context context, int nativeObjectPointer) {
+    public static PrintingContext create(int nativeObjectPointer) {
         ThreadUtils.assertOnUiThread();
-        return new PrintingContext(context, nativeObjectPointer);
+        return new PrintingContext(nativeObjectPointer);
     }
 
     @CalledByNative
@@ -127,11 +99,11 @@ public class PrintingContext implements PrintingContextInterface {
     public static void pdfWritingDone(int fd, boolean success) {
         ThreadUtils.assertOnUiThread();
         // TODO(cimamoglu): Do something when fd == -1.
-        if (sPrintingContextMap.get(fd) != null) {
+        if (PRINTING_CONTEXT_MAP.get(fd) != null) {
             ThreadUtils.assertOnUiThread();
-            PrintingContext printingContext = sPrintingContextMap.get(fd);
+            PrintingContext printingContext = PRINTING_CONTEXT_MAP.get(fd);
             printingContext.mController.pdfWritingDone(success);
-            sPrintingContextMap.remove(fd);
+            PRINTING_CONTEXT_MAP.remove(fd);
         }
     }
 
@@ -147,7 +119,7 @@ public class PrintingContext implements PrintingContextInterface {
         // If the printing dialog has already finished, tell Chromium that operation is cancelled.
         if (mController.hasPrintingFinished()) {
             // NOTE: We don't call nativeAskUserForSettingsReply (hence Chromium callback in
-            // AskUserForSettings callback) twice.  See PrintingControllerImpl#onFinish
+            // AskUserForSettings callback) twice.  See {@link PrintingControllerImpl#onFinish}
             // for more explanation.
             nativeAskUserForSettingsReply(mNativeObject, false);
         } else {
diff --git a/printing/android/java/src/org/chromium/printing/PrintingController.java b/printing/android/java/src/org/chromium/printing/PrintingController.java
index beed86b..76a2e5a 100644
--- a/printing/android/java/src/org/chromium/printing/PrintingController.java
+++ b/printing/android/java/src/org/chromium/printing/PrintingController.java
@@ -62,7 +62,7 @@ public interface PrintingController {
     void pageCountEstimationDone(final int maxPages);
 
     /**
-     * Sets PrintingContext.
+     * Sets PrintingContext currently associated with the controller.
      *
      * This needs to be called after PrintingContext object is created. Firstly its native
      * counterpart is created, and then the Java.  PrintingController implementation
@@ -71,14 +71,6 @@ public interface PrintingController {
     void setPrintingContext(final PrintingContextInterface printingContext);
 
     /**
-     * TODO(cimamoglu): Remove errorText stuff once KitKat is public and we can move this code.
-     * @param errorText The error message to be shown to user in case something goes wrong in PDF
-     *                  generation in Chromium.  We pass it here as a string because this folder
-     *                  cannot use resources directly (or any other Clank code).
-     */
-    void setErrorText(final String errorText);
-
-    /**
      * @return Whether a complete PDF generation cycle inside Chromium has been completed.
      */
     boolean hasPrintingFinished();
diff --git a/printing/android/java/src/org/chromium/printing/PrintingControllerFactory.java b/printing/android/java/src/org/chromium/printing/PrintingControllerFactory.java
deleted file mode 100644
index 78c67db..0000000
--- a/printing/android/java/src/org/chromium/printing/PrintingControllerFactory.java
+++ /dev/null
@@ -1,33 +0,0 @@
-// Copyright 2013 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package org.chromium.printing;
-
-import org.chromium.base.ThreadUtils;
-
-import android.content.Context;
-
-public class PrintingControllerFactory {
-    /** The singleton instance for this class. */
-    private static PrintingController sInstance;
-
-    private PrintingControllerFactory() {} // Static factory
-
-    /**
-     * Creates a controller for handling printing with the framework.
-     *
-     * The controller is a singleton, since there can be only one printing action at any time.
-     *
-     * @param context The application context.
-     * @return The resulting PrintingController.
-     */
-    public static PrintingController getPrintingController(
-            Context context) {
-        ThreadUtils.assertOnUiThread();
-        if (sInstance == null) {
-            sInstance = new PrintingControllerImpl(context);
-        }
-        return sInstance;
-    }
-}
diff --git a/printing/android/java/src/org/chromium/printing/PrintingControllerImpl.java b/printing/android/java/src/org/chromium/printing/PrintingControllerImpl.java
index 34c4dc4..ccd297c 100644
--- a/printing/android/java/src/org/chromium/printing/PrintingControllerImpl.java
+++ b/printing/android/java/src/org/chromium/printing/PrintingControllerImpl.java
@@ -4,11 +4,13 @@
 
 package org.chromium.printing;
 
-import android.content.Context;
+import org.chromium.base.ThreadUtils;
+
+import android.annotation.SuppressLint;
+import android.os.Build;
 import android.os.Bundle;
 import android.os.CancellationSignal;
 import android.os.ParcelFileDescriptor;
-import android.os.CancellationSignal.OnCancelListener;
 import android.print.PageRange;
 import android.print.PrintAttributes;
 import android.print.PrintAttributes.MediaSize;
@@ -17,15 +19,10 @@ import android.print.PrintDocumentAdapter;
 import android.print.PrintDocumentAdapter.LayoutResultCallback;
 import android.print.PrintDocumentAdapter.WriteResultCallback;
 import android.print.PrintDocumentInfo;
-import android.print.PrintJob;
-import android.print.PrintManager;
-import android.util.Log;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Iterator;
-import java.util.List;
 
 /**
  * Controls the interactions with Android framework related to printing.
@@ -35,15 +32,26 @@ import java.util.List;
  * print button. The singleton object lives in UI thread. Interaction with the native side is
  * carried through PrintingContext class.
  */
-class PrintingControllerImpl extends PrintDocumentAdapter implements PrintingController {
+public class PrintingControllerImpl extends PrintDocumentAdapter implements PrintingController {
 
     private static final String LOG_TAG = "PrintingControllerImpl";
 
-    private static final String PDF_FILE_NAME = "chrome_print_document.pdf";
+    /**
+     * This is used for both initial state and a completed state (i.e. starting from either
+     * onLayout or onWrite, a PDF generation cycle is completed another new one can safely start).
+     */
+    private static final int PRINTING_STATE_READY = 0;
+    private static final int PRINTING_STATE_STARTED_FROM_ONLAYOUT = 1;
+    private static final int PRINTING_STATE_STARTED_FROM_ONWRITE = 2;
+    /** Printing dialog has been dismissed and cleanup has been done. */
+    private static final int PRINTING_STATE_FINISHED = 3;
+
+    /** The singleton instance for this class. */
+    private static PrintingController sInstance;
 
-    private String mErrorMessage;
+    private final String mErrorMessage;
 
-    private final PrintManager mPrintManager;
+    private final PrintManagerDelegate mPrintManager;
 
     private PrintingContextInterface mPrintingContext;
 
@@ -73,16 +81,6 @@ class PrintingControllerImpl extends PrintDocumentAdapter implements PrintingCon
     /** The object through which native PDF generation process is initiated. */
     private Printable mPrintable;
 
-    /**
-     * This is used for both initial state and a completed state (i.e. starting from either
-     * onLayout or onWrite, a PDF generation cycle is completed another new one can safely start).
-     */
-    private final static int PRINTING_STATE_READY = 0;
-    private final static int PRINTING_STATE_STARTED_FROM_ONLAYOUT = 1;
-    private final static int PRINTING_STATE_STARTED_FROM_ONWRITE = 2;
-    /** Printing dialog has been dismissed and cleanup has been done. */
-    private final static int PRINTING_STATE_FINISHED = 3;
-
     private int mPrintingState = PRINTING_STATE_READY;
 
     /** Whether layouting parameters have been changed to require a new PDF generation. */
@@ -91,18 +89,54 @@ class PrintingControllerImpl extends PrintDocumentAdapter implements PrintingCon
     /** Total number of pages to print with initial print dialog settings. */
     private int mLastKnownMaxPages = PrintDocumentInfo.PAGE_COUNT_UNKNOWN;
 
-    PrintingControllerImpl(final Context context) {
-        mPrintManager = (PrintManager) context.getSystemService(Context.PRINT_SERVICE);
+    private PrintingControllerImpl(PrintManagerDelegate printManager, String errorText) {
+        mPrintManager = printManager;
+        mErrorMessage = errorText;
     }
 
-    @Override
-    public boolean hasPrintingFinished() {
-      return mPrintingState == PRINTING_STATE_FINISHED;
+    /**
+     * Creates a controller for handling printing with the framework.
+     *
+     * The controller is a singleton, since there can be only one printing action at any time.
+     *
+     * @param errorText The error message to be shown to user in case something goes wrong in PDF
+     *                  generation in Chromium. We pass it here as a string so src/printing/android
+     *                  doesn't need any string dependency.
+     * @return The resulting PrintingController.
+     */
+    public static PrintingController create(PrintManagerDelegate printManager,
+           String errorText) {
+        ThreadUtils.assertOnUiThread();
+
+        if (sInstance == null) {
+            sInstance = new PrintingControllerImpl(printManager, errorText);
+        }
+        return sInstance;
+    }
+
+    /**
+     * Returns the singleton instance, created by the {@link PrintingControllerImpl#create}.
+     *
+     * This method must be called once {@link PrintingControllerImpl#create} is called, and always
+     * thereafter.
+     *
+     * @return The singleton instance.
+     */
+    public static PrintingController getInstance() {
+        return sInstance;
+    }
+
+    /**
+     * @return True if the running version of the Android supports printing.
+     */
+    @SuppressLint("InlinedApi")
+    public static boolean isPrintingSupported() {
+        return Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT;
     }
 
     @Override
-    public void setErrorText(final String errorText) {
-      mErrorMessage = errorText;
+    public boolean hasPrintingFinished() {
+      return mPrintingState == PRINTING_STATE_FINISHED;
     }
 
     @Override
@@ -127,7 +161,7 @@ class PrintingControllerImpl extends PrintDocumentAdapter implements PrintingCon
 
     @Override
     public int[] getPageNumbers() {
-        return mPages;
+        return mPages.clone();
     }
 
     @Override
@@ -203,8 +237,7 @@ class PrintingControllerImpl extends PrintDocumentAdapter implements PrintingCon
             mLastKnownMaxPages = maxPages;
         }
         if (mPrintingState == PRINTING_STATE_STARTED_FROM_ONLAYOUT) {
-            // TODO(cimamoglu): Choose a meaningful filename.
-            PrintDocumentInfo info = new PrintDocumentInfo.Builder(PDF_FILE_NAME)
+            PrintDocumentInfo info = new PrintDocumentInfo.Builder(mPrintable.getTitle())
                     .setContentType(PrintDocumentInfo.CONTENT_TYPE_DOCUMENT)
                     .setPageCount(mLastKnownMaxPages)
                     .build();
@@ -299,7 +332,7 @@ class PrintingControllerImpl extends PrintDocumentAdapter implements PrintingCon
         mOnLayoutCallback = null;
     }
 
-    private void closeFileDescriptor(int fd) {
+    private static void closeFileDescriptor(int fd) {
         if (fd != -1) return;
         ParcelFileDescriptor fileDescriptor = ParcelFileDescriptor.adoptFd(fd);
         if (fileDescriptor != null) {
@@ -311,7 +344,7 @@ class PrintingControllerImpl extends PrintDocumentAdapter implements PrintingCon
         }
     }
 
-    private PageRange[] convertIntegerArrayToPageRanges(int[] pagesArray) {
+    private static PageRange[] convertIntegerArrayToPageRanges(int[] pagesArray) {
         PageRange[] pageRanges;
         if (pagesArray != null) {
             pageRanges = new PageRange[pagesArray.length];
@@ -329,7 +362,7 @@ class PrintingControllerImpl extends PrintDocumentAdapter implements PrintingCon
     /**
      * Gets an array of page ranges and returns an array of integers with all ranges expanded.
      */
-    private int[] normalizeRanges(final PageRange[] ranges) {
+    private static int[] normalizeRanges(final PageRange[] ranges) {
         // Expand ranges into a list of individual numbers.
         ArrayList<Integer> pages = new ArrayList<Integer>();
         for (PageRange range : ranges) {
diff --git a/printing/printing_context_android.cc b/printing/printing_context_android.cc
index 79693d7..d1df905 100644
--- a/printing/printing_context_android.cc
+++ b/printing/printing_context_android.cc
@@ -91,7 +91,6 @@ void PrintingContextAndroid::AskUserForSettings(
   if (j_printing_context_.is_null()) {
     j_printing_context_.Reset(Java_PrintingContext_create(
         env,
-        base::android::GetApplicationContext(),
         reinterpret_cast<int>(this)));
   }