Commit 46582a40 authored by Shimi Zhang's avatar Shimi Zhang Committed by Commit Bot

[Printing] Change |showSystemDialogDone()| timing

In |window.print()| code path for Android, we have the call stack like
this:

  PrintJobWorker::GetSettingsWithUI() ->
  PrintingContextAndroid::AskUserForSettings() ->
  PrintingContext.showPrintDialog() (Java)

We have set |callback_| in |PrintingContextAndroid| to be
|PrintJobWorker::GetSettingsDone()|, but didn't call it in later stage
unless something fails.

This CL makes |PrintingControllerImpl| call
|PrintingContext.showSystemDialogDone()| no matter what happens. This is
because the first |GetSettingsWithUI()| for |window.print()| on Android
is only to wake up |PrintManager| to start printing job. We will set
the real settings in later |GetSettingsWithUI()| call.

Bug: 863297
Change-Id: I3be0a3dceab134db7a47e0c1b48ccd1802e74447
Reviewed-on: https://chromium-review.googlesource.com/1162754Reviewed-by: default avatarChangwan Ryu <changwan@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582980}
parent ac2e56bc
......@@ -246,7 +246,7 @@ public class PrintingControllerTest {
new TabPrinter(currentTab), mockPrintManagerDelegate, -1, -1);
TabModelUtils.closeCurrentTab(mActivityTestRule.getActivity().getCurrentTabModel());
Assert.assertFalse("currentTab should be closed already.", currentTab.isInitialized());
printingController.startPendingPrint(null);
printingController.startPendingPrint();
});
}
......
......@@ -87,16 +87,16 @@ public class PrintingContext implements PrintingContextInterface {
return mController.getPageHeight();
}
// Called along window.print() path to initialize a printing job.
@CalledByNative
public void showPrintDialog() {
ThreadUtils.assertOnUiThread();
if (mController != null) { // The native side doesn't check if printing is enabled
mController.startPendingPrint(this);
} else {
Log.d(TAG, "Unable to start printing, feature not available.");
// Printing disabled. Notify the native side to stop waiting.
showSystemDialogDone();
mController.startPendingPrint();
}
// Reply to native side with |CANCEL| since there is no printing settings available yet at
// this stage.
showSystemDialogDone();
}
@CalledByNative
......@@ -139,8 +139,8 @@ public class PrintingContext implements PrintingContextInterface {
nativeAskUserForSettingsReply(mNativeObject, success);
}
@Override
public void showSystemDialogDone() {
private void showSystemDialogDone() {
assert mNativeObject != 0;
nativeShowSystemDialogDone(mNativeObject);
}
......
......@@ -15,10 +15,4 @@ public interface PrintingContextInterface {
* @param delete If true, delete the entry (if it exists). If false, add it to the map.
*/
void updatePrintingContextMap(int fileDescriptor, boolean delete);
/**
* Notifies the native side that the printing process is completed. This method should be
* called when the process was initiated by the native side (window.print())
*/
void showSystemDialogDone();
}
......@@ -77,7 +77,7 @@ public interface PrintingController {
/**
* Sets the data required to initiate a printing process. The process can later be started using
* {@link #startPendingPrint(PrintingContextInterface)}.
* {@link #startPendingPrint()}.
*
* @param printable An object capable of starting native side PDF generation, i.e. typically
* a Tab.
......@@ -92,11 +92,6 @@ public interface PrintingController {
/**
* Starts printing, provided that the current object already has sufficient data to start the
* process. (using {@link #setPendingPrint(Printable, PrintManagerDelegate)} for example)
*
* @param jsOriginatedPrintingContext The printingContext holding the callback to be used to
* reply when javascript can resume. When printing is done (or could not start),
* {@link PrintingContextInterface#showSystemDialogDone()} will be called on this object.
*/
void startPendingPrint(PrintingContextInterface jsOriginatedPrintingContext);
void startPendingPrint();
}
......@@ -51,11 +51,6 @@ public class PrintingControllerImpl implements PrintingController, PdfGenerator
private PrintingContextInterface mPrintingContext;
/**
* The context of a query initiated by window.print(), stored here to allow syncrhonization
* with javascript.
*/
private PrintingContextInterface mContextFromScriptInitiation;
private int mRenderProcessId;
private int mRenderFrameId;
......@@ -190,7 +185,7 @@ public class PrintingControllerImpl implements PrintingController, PdfGenerator
}
@Override
public void startPendingPrint(PrintingContextInterface printingContext) {
public void startPendingPrint() {
boolean canStartPrint = false;
if (mIsBusy) {
Log.d(TAG, "Pending print can't be started. PrintingController is busy.");
......@@ -202,12 +197,8 @@ public class PrintingControllerImpl implements PrintingController, PdfGenerator
canStartPrint = true;
}
if (!canStartPrint) {
if (printingContext != null) printingContext.showSystemDialogDone();
return;
}
if (!canStartPrint) return;
mContextFromScriptInitiation = printingContext;
mIsBusy = true;
mPrintDocumentAdapterWrapper.print(mPrintManager, mPrintable.getTitle());
mPrintManager = null;
......@@ -217,7 +208,7 @@ public class PrintingControllerImpl implements PrintingController, PdfGenerator
public void startPrint(final Printable printable, PrintManagerDelegate printManager) {
if (mIsBusy) return;
setPendingPrint(printable, printManager, mRenderProcessId, mRenderFrameId);
startPendingPrint(null);
startPendingPrint();
}
@Override
......@@ -313,10 +304,6 @@ public class PrintingControllerImpl implements PrintingController, PdfGenerator
mPrintingContext = null;
}
if (mContextFromScriptInitiation != null) {
mContextFromScriptInitiation.showSystemDialogDone();
mContextFromScriptInitiation = null;
}
mRenderProcessId = -1;
mRenderFrameId = -1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment