[OpenJDK 2D-Dev] RFR: 8241829 Cleanup the code for PrinterJob on windows

[Philip Race]

Hmm ... I guess OpenPrinter doesn't fail often.

More of a concern is that I don't see where we (before or after this 
change) call ClosePrinter
in the case of success.

We certainly don't want to close it here because acc to
*https://docs.microsoft.com/en-us/windows/win32/printdocs/findfirstprinterchangenotification*
Callers of FindFirstPrinterChangeNotification must ensure that the 
printer handle passed into
FindFirstPrinterChangeNotification remains valid until 
FindClosePrinterChangeNotification is called.
If the printer handle is closed before the printer change notification 
handle, further notification
will fail to be delivered.


Are we leaking the handle ?  Looks that way to me ..

-phil



On 3/31/20, 3:21 PM, Sergey Bylokhov wrote:
> Hi, Prasanta.
>
> On 3/30/20 7:46 am, Prasanta Sadhukhan wrote:
>> I guess it is better if you directly pass NULL to OpenPrinter, rather 
>> than create an unnecessary variable.
>
> That was done only for documentation purposes.
>
>> Also, I think it will be good if we do a ClosePrinter() if 
>> FindFirstPrinterChangeNotification() fails.
>
> Nice catch! webrev is updated:
> http://cr.openjdk.java.net/~serb/8241829/webrev.01
>
>>
>> Regards
>> Prasanta
>> On 30-Mar-20 7:13 PM, Sergey Bylokhov wrote:
>>> Hello.
>>> Please review the fix for jdk/client.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8241829
>>> Fix: http://cr.openjdk.java.net/~serb/8241829/webrev.00
>>>
>>> In PrintServiceLookupProvider.java we always pass NULL to
>>> the notifyFirstPrinterChange as a name of the printer, so
>>> we can remove the code which expects a non-null value.
>>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20200402/ac45bbc0/attachment.htm>