On Wed, 19 May 2021 15:19:23 GMT, Phil Race <p...@openjdk.org> wrote:
>> Hello,
>>
>> Could you please review the following fix for the bug specific to macOS. The
>> bug consists in the fact that if the method
>> "java.awt.print.Printable.print​(Graphics, PageFormat, int)" throws
>> "java.awt.print.PrinterException" or "java.lang.RuntimeException" during the
>> call "java.awt.print.PrinterJob.print()", then the exception is caught and
>> ignored by JDK and a user cannot learn that printing failed and what caused
>> failure of printing, because "PrinterJob.print()" method does not throw
>> "PrinterException" or the occurred exception is not reported by JDK through
>> the error stream.
>>
>> ROOT CAUSE OF THE BUG:
>> The root cause of the bug is the fact that in the method
>> "sun.lwawt.macosx.CPrinterJob.printAndGetPageFormatArea(final Printable,
>> final Graphics, final PageFormat, final int)" from the file
>> "src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java" the
>> exception thrown during execution of the expression
>>
>> "int pageResult = printable.print(graphics, pageFormat, pageIndex);"
>>
>> is caught but is not returned to a developer by any mean or is not printed
>> out to the error stream.
>>
>> THE FIX:
>> The fix implements propagation of the occurred and caught exception to the
>> level of the user's code executing "PrinterJob.print()" method. Propagation
>> of the exception by storing it in the instance variable of "CPrinterJob"
>> object is implemented, because the engaged code always is executed:
>> - on 2 threads (non-EDT thread, EDT thread) in case when
>> "PrinterJob.print()" is called by the user on a non-EDT thread;
>> - on 3 threads (2 EDT threads, a temporary thread started by JDK to execute
>> "CPrinterJob._safePrintLoop(long, long );") when "PrinterJob.print()" is
>> called on EDT thread.
>>
>> The regression test which is part of the fix was also successfully executed
>> on MS Windows OS and Linux OS.
>>
>> Thank you,
>> Anton
>
> test/jdk/java/awt/print/PrinterJob/ExceptionFromPrintableIsIgnoredTest.java
> line 86:
>
>> 84: }
>> 85: });
>> 86: if (job.printDialog()) {
>
> why do we need a dialog ? If you remove this it can be an automated test.
Hello Phil. Thank you very much for review of this fix. First I am answering
your question, which is about why the test passes on Windows and which is your
first and separate comment not bound to the source code. The test passes on
Windows, because in JDK 17 for both Windows and Linux JDK works similarly
stably and reliably, JDK 17 on Window and Linux successfully propagates
"PrinterException" and "RuntimeException", if it is thrown from
"Printable.print" method. I had verified this using my standalone manual test
case "ExceptionFromPrintableIsIgnored.java" attached to the bug record in JBS,
before sending the 1st version of the fix for review. I did not explore, why it
is so, because it works on Windows and Linux and this bug is related to macOS,
but I think that it works on Windows and Linux, because in JDK implementations
for Windows and Linux printing is consistently done by JDK on the same thread
on which "PrinterJob.print" was called and no portions of the involved code are
exe
cuted asynchronously on other thread, like EDT in case of macOS.
I agree there is no need in the dialog in the regression test. In the 1st
version of the fix I did it deliberately, to make it more manual, because the
test initiates printing and there is no guarantee what type of printer would be
set up on the test host, if some virtual printer is used on the host, that
printer can show its own native dialog asking to specify location of a file, in
which the printed document should be saved. That is why I think that this test
cannot be fully automatic. In the 2nd version of the fix which I am submitting
right now I removed this unnecessary code showing JDK print dialog.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4036
