[Android] Report correct printing PageRange to onWrite* callbacks

Currently we report onWrite* callbacks PageRange from what we get from `onWrite`. However, It will give PageRange.ALL_PAGES when printing all pages. This behavior is fine for now but will not be accepted in future. This CL changed the behavior to report how many pages we need to print if print all pages. Bug: 733852 Change-Id: Iff1fe57baf1989910f0c29329b42bfbf26c86e05 Reviewed-on: https://chromium-review.googlesource.com/551278 Reviewed-by: Mustafa Emre Acer <meacer@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Selim Gurun <sgurun@chromium.org> Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org> Commit-Queue: Shimi Zhang <ctzsm@chromium.org> Cr-Commit-Position: refs/heads/master@{#483597}
qweri0p · Jun 30, 2017 · b414956 · b414956
1 parent 33a59e7
commit b414956
Show file tree

Hide file tree

Showing 17 changed files with 77 additions and 50 deletions.
diff --git a/android_webview/browser/aw_pdf_exporter.cc b/android_webview/browser/aw_pdf_exporter.cc
@@ -68,7 +68,7 @@ void AwPdfExporter::ExportToPdf(JNIEnv* env,
       base::Bind(&AwPdfExporter::DidExportPdf, base::Unretained(this)));
 
   if (!print_manager->PrintNow())
-    DidExportPdf(fd, false);
+    DidExportPdf(fd, 0);
 }
 
 namespace {
@@ -113,12 +113,12 @@ void AwPdfExporter::InitPdfSettings(JNIEnv* env,
   settings.set_should_print_backgrounds(true);
 }
 
-void AwPdfExporter::DidExportPdf(int fd, bool success) {
+void AwPdfExporter::DidExportPdf(int fd, int page_count) {
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
   if (obj.is_null())
     return;
-  Java_AwPdfExporter_didExportPdf(env, obj, success);
+  Java_AwPdfExporter_didExportPdf(env, obj, page_count);
 }
 
 bool RegisterAwPdfExporter(JNIEnv* env) {

diff --git a/android_webview/browser/aw_pdf_exporter.h b/android_webview/browser/aw_pdf_exporter.h
@@ -42,7 +42,7 @@ class AwPdfExporter {
                        const base::android::JavaRef<jobject>& obj,
                        const printing::PageRanges& page_ranges,
                        printing::PrintSettings& settings);
-  void DidExportPdf(int fd, bool success);
+  void DidExportPdf(int fd, int page_count);
 
   JavaObjectWeakGlobalRef java_ref_;
   content::WebContents* web_contents_;

diff --git a/android_webview/browser/aw_printing_message_filter.cc b/android_webview/browser/aw_printing_message_filter.cc
@@ -73,14 +73,15 @@ void AwPrintingMessageFilter::OnAllocateTempFileForPrinting(
   temp_file_fd->auto_close = false;
 }
 
-void AwPrintingMessageFilter::OnTempFileForPrintingWritten(
-    int render_frame_id,
-    int sequence_number) {
+void AwPrintingMessageFilter::OnTempFileForPrintingWritten(int render_frame_id,
+                                                           int sequence_number,
+                                                           int page_count) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_GT(page_count, 0);
   AwPrintManager* print_manager =
       GetPrintManager(render_process_id_, render_frame_id);
   if (print_manager)
-    print_manager->PdfWritingDone(true);
+    print_manager->PdfWritingDone(page_count);
 }
 
 }  // namespace android_webview
diff --git a/android_webview/browser/aw_printing_message_filter.h b/android_webview/browser/aw_printing_message_filter.h
@@ -33,7 +33,9 @@ class AwPrintingMessageFilter : public content::BrowserMessageFilter {
   void OnAllocateTempFileForPrinting(int render_frame_id,
                                      base::FileDescriptor* temp_file_fd,
                                      int* sequence_number);
-  void OnTempFileForPrintingWritten(int render_frame_id, int sequence_number);
+  void OnTempFileForPrintingWritten(int render_frame_id,
+                                    int sequence_number,
+                                    int page_count);
 
   const int render_process_id_;
 

diff --git a/android_webview/java/src/org/chromium/android_webview/AwPdfExporter.java b/android_webview/java/src/org/chromium/android_webview/AwPdfExporter.java
@@ -10,7 +10,6 @@
 import android.print.PrintAttributes;
 import android.util.Log;
 import android.view.ViewGroup;
-import android.webkit.ValueCallback;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
@@ -27,7 +26,7 @@ public class AwPdfExporter {
     private long mNativeAwPdfExporter;
     // TODO(sgurun) result callback should return an int/object indicating errors.
     // potential errors: invalid print parameters, already pending, IO error
-    private ValueCallback<Boolean> mResultCallback;
+    private AwPdfExporterCallback mResultCallback;
     private PrintAttributes mAttributes;
     private ParcelFileDescriptor mFd;
     // Maintain a reference to the top level object (i.e. WebView) since in a common
@@ -38,6 +37,18 @@ public class AwPdfExporter {
     // be reflected there.
     private ViewGroup mContainerView;
 
+    /**
+     * AwPdfExporter callback used to call onWrite* callbacks in Android framework.
+     */
+    public interface AwPdfExporterCallback {
+        /**
+         * Called by the native side when PDF generation is done.
+         * @param pageCount How many pages native side wrote to PDF file descriptor. Non-positive
+         *                  value indicates native side writing failed.
+         */
+        public void pdfWritingDone(int pageCount);
+    }
+
     AwPdfExporter(ViewGroup containerView) {
         setContainerView(containerView);
     }
@@ -47,7 +58,7 @@ public void setContainerView(ViewGroup containerView) {
     }
 
     public void exportToPdf(final ParcelFileDescriptor fd, PrintAttributes attributes, int[] pages,
-            ValueCallback<Boolean> resultCallback, CancellationSignal cancellationSignal) {
+            AwPdfExporterCallback resultCallback, CancellationSignal cancellationSignal) {
         if (fd == null) {
             throw new IllegalArgumentException("fd cannot be null");
         }
@@ -67,7 +78,7 @@ public void exportToPdf(final ParcelFileDescriptor fd, PrintAttributes attribute
             throw new IllegalArgumentException("attributes must specify margins");
         }
         if (mNativeAwPdfExporter == 0) {
-            resultCallback.onReceiveValue(false);
+            resultCallback.pdfWritingDone(0);
             return;
         }
         mResultCallback = resultCallback;
@@ -83,7 +94,7 @@ private void setNativeAwPdfExporter(long nativePdfExporter) {
         // via Webview.Destroy) before it has a chance to complete the pdf exporting.
         if (nativePdfExporter == 0 && mResultCallback != null) {
             try {
-                mResultCallback.onReceiveValue(false);
+                mResultCallback.pdfWritingDone(0);
                 mResultCallback = null;
             } catch (IllegalStateException ex) {
                 // Swallow the illegal state exception here. It is possible that app
@@ -105,8 +116,8 @@ private static int getPrintDpi(PrintAttributes attributes) {
     }
 
     @CalledByNative
-    private void didExportPdf(boolean success) {
-        mResultCallback.onReceiveValue(success);
+    private void didExportPdf(int pageCount) {
+        mResultCallback.pdfWritingDone(pageCount);
         mResultCallback = null;
         mAttributes = null;
         // The caller should close the file.

diff --git a/android_webview/java/src/org/chromium/android_webview/AwPrintDocumentAdapter.java b/android_webview/java/src/org/chromium/android_webview/AwPrintDocumentAdapter.java
@@ -12,7 +12,6 @@
 import android.print.PrintAttributes;
 import android.print.PrintDocumentAdapter;
 import android.print.PrintDocumentInfo;
-import android.webkit.ValueCallback;
 
 import java.util.ArrayList;
 
@@ -73,12 +72,12 @@ public void onWrite(final PageRange[] pages, ParcelFileDescriptor destination,
             return;
         }
 
-        mPdfExporter.exportToPdf(
-                destination, mAttributes, normalizeRanges(pages), new ValueCallback<Boolean>() {
+        mPdfExporter.exportToPdf(destination, mAttributes,
+                normalizeRanges(pages), new AwPdfExporter.AwPdfExporterCallback() {
                     @Override
-                    public void onReceiveValue(Boolean value) {
-                        if (value) {
-                            callback.onWriteFinished(pages);
+                    public void pdfWritingDone(int pageCount) {
+                        if (pageCount > 0) {
+                            callback.onWriteFinished(validatePageRanges(pages, pageCount));
                         } else {
                             // TODO(sgurun) provide a localized error message
                             callback.onWriteFailed(null);
@@ -87,7 +86,14 @@ public void onReceiveValue(Boolean value) {
                 }, cancellationSignal);
     }
 
-    private int[] normalizeRanges(final PageRange[] ranges) {
+    private static PageRange[] validatePageRanges(PageRange[] pages, int pageCount) {
+        if (pages.length == 1 && PageRange.ALL_PAGES.equals(pages[0])) {
+            return new PageRange[] {new PageRange(0, pageCount - 1)};
+        }
+        return pages;
+    }
+
+    private static int[] normalizeRanges(final PageRange[] ranges) {
         if (ranges.length == 1 && PageRange.ALL_PAGES.equals(ranges[0])) {
             return new int[0];
         }

diff --git a/chrome/browser/printing/printing_message_filter.cc b/chrome/browser/printing/printing_message_filter.cc
@@ -160,12 +160,14 @@ void PrintingMessageFilter::OnAllocateTempFileForPrinting(
 }
 
 void PrintingMessageFilter::OnTempFileForPrintingWritten(int render_frame_id,
-                                                         int sequence_number) {
+                                                         int sequence_number,
+                                                         int page_count) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_GT(page_count, 0);
   PrintViewManagerBasic* print_view_manager =
       GetPrintManager(render_process_id_, render_frame_id);
   if (print_view_manager)
-    print_view_manager->PdfWritingDone(true);
+    print_view_manager->PdfWritingDone(page_count);
 }
 #endif  // defined(OS_ANDROID)
 

diff --git a/chrome/browser/printing/printing_message_filter.h b/chrome/browser/printing/printing_message_filter.h
@@ -60,7 +60,9 @@ class PrintingMessageFilter : public content::BrowserMessageFilter {
   void OnAllocateTempFileForPrinting(int render_frame_id,
                                      base::FileDescriptor* temp_file_fd,
                                      int* sequence_number);
-  void OnTempFileForPrintingWritten(int render_frame_id, int sequence_number);
+  void OnTempFileForPrintingWritten(int render_frame_id,
+                                    int sequence_number,
+                                    int page_count);
 
   // Updates the file descriptor for the PrintViewManagerBasic of a given
   // |render_frame_id|.

diff --git a/components/printing/browser/print_manager.cc b/components/printing/browser/print_manager.cc
@@ -50,20 +50,20 @@ void PrintManager::OnPrintingFailed(int cookie) {
     return;
   }
 #if defined(OS_ANDROID)
-  PdfWritingDone(false);
+  PdfWritingDone(0);
 #endif
 }
 
 void PrintManager::PrintingRenderFrameDeleted() {
 #if defined(OS_ANDROID)
-  PdfWritingDone(false);
+  PdfWritingDone(0);
 #endif
 }
 
 #if defined(OS_ANDROID)
-void PrintManager::PdfWritingDone(bool result) {
+void PrintManager::PdfWritingDone(int page_count) {
   if (!pdf_writing_done_callback_.is_null())
-    pdf_writing_done_callback_.Run(file_descriptor().fd, result);
+    pdf_writing_done_callback_.Run(file_descriptor().fd, page_count);
   // Invalidate the file descriptor so it doesn't get reused.
   file_descriptor_ = base::FileDescriptor(-1, false);
 }

diff --git a/components/printing/browser/print_manager.h b/components/printing/browser/print_manager.h
@@ -22,9 +22,9 @@ class PrintManager : public content::WebContentsObserver {
 
 #if defined(OS_ANDROID)
   // TODO(timvolodine): consider introducing PrintManagerAndroid (crbug/500960)
-  typedef base::Callback<void(int, bool)> PdfWritingDoneCallback;
+  using PdfWritingDoneCallback = base::Callback<void(int, int)>;
 
-  void PdfWritingDone(bool result);
+  void PdfWritingDone(int page_count);
 
   // Sets the file descriptor into which the PDF will be written.
   void set_file_descriptor(const base::FileDescriptor& file_descriptor) {

diff --git a/components/printing/common/print_messages.h b/components/printing/common/print_messages.h
@@ -398,9 +398,10 @@ IPC_SYNC_MESSAGE_CONTROL1_2(PrintHostMsg_AllocateTempFileForPrinting,
                             int /* render_frame_id */,
                             base::FileDescriptor /* temp file fd */,
                             int /* fd in browser*/)
-IPC_MESSAGE_CONTROL2(PrintHostMsg_TempFileForPrintingWritten,
+IPC_MESSAGE_CONTROL3(PrintHostMsg_TempFileForPrintingWritten,
                      int /* render_frame_id */,
-                     int /* fd in browser */)
+                     int /* fd in browser */,
+                     int /* page count */)
 #endif  // defined(OS_ANDROID)
 
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)

diff --git a/components/printing/renderer/print_web_view_helper_linux.cc b/components/printing/renderer/print_web_view_helper_linux.cc
@@ -76,8 +76,8 @@ bool PrintWebViewHelper::PrintPagesNative(blink::WebLocalFrame* frame,
     return false;
 
   // Tell the browser we've finished writing the file.
-  Send(new PrintHostMsg_TempFileForPrintingWritten(routing_id(),
-                                                   sequence_number));
+  Send(new PrintHostMsg_TempFileForPrintingWritten(
+      routing_id(), sequence_number, printed_pages.size()));
   return true;
 #else
   PrintHostMsg_DidPrintPage_Params printed_page_params;

diff --git a/printing/android/java/src/org/chromium/printing/PrintingContext.java b/printing/android/java/src/org/chromium/printing/PrintingContext.java
@@ -110,13 +110,13 @@ public void showPrintDialog() {
     }
 
     @CalledByNative
-    public static void pdfWritingDone(int fd, boolean success) {
+    public static void pdfWritingDone(int fd, int pageCount) {
         ThreadUtils.assertOnUiThread();
         // TODO(cimamoglu): Do something when fd == -1.
         if (PRINTING_CONTEXT_MAP.get(fd) != null) {
             ThreadUtils.assertOnUiThread();
             PrintingContext printingContext = PRINTING_CONTEXT_MAP.get(fd);
-            printingContext.mController.pdfWritingDone(success);
+            printingContext.mController.pdfWritingDone(pageCount);
             PRINTING_CONTEXT_MAP.remove(fd);
         } else {
             Log.d(TAG, "No PrintingContext found for fd %d, can't notify print completion.", fd);

diff --git a/printing/android/java/src/org/chromium/printing/PrintingController.java b/printing/android/java/src/org/chromium/printing/PrintingController.java
@@ -56,9 +56,10 @@ public interface PrintingController {
     /**
      * This method is called by the native side to signal PDF writing process is completed.
      *
-     * @param success Whether the PDF is written into the provided file descriptor successfully.
+     * @param pageCount How many pages native side wrote to PDF file descriptor. Non-positive value
+     *                  indicates native side writing failed.
      */
-    void pdfWritingDone(boolean success);
+    void pdfWritingDone(int pageCount);
 
     /**
      * Sets PrintingContext currently associated with the controller.

diff --git a/printing/android/java/src/org/chromium/printing/PrintingControllerImpl.java b/printing/android/java/src/org/chromium/printing/PrintingControllerImpl.java
@@ -204,13 +204,13 @@ public void startPrint(final Printable printable, PrintManagerDelegate printMana
     }
 
     @Override
-    public void pdfWritingDone(boolean success) {
+    public void pdfWritingDone(int pageCount) {
         if (mPrintingState == PRINTING_STATE_FINISHED) return;
         mPrintingState = PRINTING_STATE_READY;
         closeFileDescriptor(mFileDescriptor);
         mFileDescriptor = -1;
-        if (success) {
-            PageRange[] pageRanges = convertIntegerArrayToPageRanges(mPages);
+        if (pageCount > 0) {
+            PageRange[] pageRanges = convertIntegerArrayToPageRanges(mPages, pageCount);
             mOnWriteCallback.onWriteFinished(pageRanges);
         } else {
             mOnWriteCallback.onWriteFailed(mErrorMessage);
@@ -328,7 +328,7 @@ private static void closeFileDescriptor(int fd) {
         }
     }
 
-    private static PageRange[] convertIntegerArrayToPageRanges(int[] pagesArray) {
+    private static PageRange[] convertIntegerArrayToPageRanges(int[] pagesArray, int pageCount) {
         PageRange[] pageRanges;
         if (pagesArray != null) {
             pageRanges = new PageRange[pagesArray.length];
@@ -338,7 +338,7 @@ private static PageRange[] convertIntegerArrayToPageRanges(int[] pagesArray) {
             }
         } else {
             // null corresponds to all pages in Chromium printing logic.
-            pageRanges = new PageRange[] { PageRange.ALL_PAGES };
+            pageRanges = new PageRange[] {new PageRange(0, pageCount - 1)};
         }
         return pageRanges;
     }

diff --git a/printing/printing_context_android.cc b/printing/printing_context_android.cc
@@ -67,9 +67,9 @@ std::unique_ptr<PrintingContext> PrintingContext::Create(Delegate* delegate) {
 }
 
 // static
-void PrintingContextAndroid::PdfWritingDone(int fd, bool success) {
+void PrintingContextAndroid::PdfWritingDone(int fd, int page_count) {
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_PrintingContext_pdfWritingDone(env, fd, success);
+  Java_PrintingContext_pdfWritingDone(env, fd, page_count);
 }
 
 PrintingContextAndroid::PrintingContextAndroid(Delegate* delegate)

diff --git a/printing/printing_context_android.h b/printing/printing_context_android.h
@@ -23,8 +23,10 @@ class PRINTING_EXPORT PrintingContextAndroid : public PrintingContext {
   ~PrintingContextAndroid() override;
 
   // Called when the page is successfully written to a PDF using the file
-  // descriptor specified, or when the printing operation failed.
-  static void PdfWritingDone(int fd, bool success);
+  // descriptor specified, or when the printing operation failed. On success,
+  // the PDF written to |fd| has |page_count| pages. Non-positive |page_count|
+  // indicates failure.
+  static void PdfWritingDone(int fd, int page_count);
 
   // Called from Java, when printing settings from the user are ready or the
   // printing operation is canceled.
@@ -70,4 +72,3 @@ class PRINTING_EXPORT PrintingContextAndroid : public PrintingContext {
 }  // namespace printing
 
 #endif  // PRINTING_PRINTING_CONTEXT_ANDROID_H_
-