[8u] RFR 8153732: Windows remote printer changes do not reflect in lookupPrintServices()
Zhengyu Gu
zgu at redhat.com
Wed Jun 5 15:07:21 UTC 2019
Hi Alexey,
Thanks for reviewing.
On 6/5/19 10:08 AM, Alexey Ivanov wrote:
> Hi Zhengyu,
>
> In my opinion, backporting JDK-8055705 [1] which renames platform
> specific classes to a common name is unnecessary. The purpose of
> renaming was to simplify module definition which is irrelevant for 8.
>
We are trying to bring the code base close to later JDK, for the
benefits of future backports.
> *PrintServiceLookupProvider.java*
> 133 // start the local printer listener thread
> 134 Thread thr = new Thread(null, new PrinterChangeListener(),
> 135 "PrinterListener", 0);
>
> You create a new thread instance with PrinterChangeListener as the
> Runnable implementation. Yet PrinterChangeListener is still a subclass
> of Thread which seems redundant. You can make PrinterChangeListener
> implement Runnable without extending Thread.
>
> Otherwise, looks good to me.
Thanks.
Updated webrev: http://cr.openjdk.java.net/~zgu/JDK-8153732_8u/webrev.02/
> However, I'd recommend backporting the follow-up fixes in one go, see my
> comment [3] to JDK-8153732 [2].
>
> First of all, JDK-8153732 caused a regression JDK-8212202 [4]: NPE is
> thrown if no printers are installed on the system.
> JDK-8221412 [5] fixes another situation where the list of printers is
> not refreshed. In addition, it simplifies the native code.
> And JDK-8221263 [6] updates the test making it more user-friendly.
Yes, they are all on my backport list. Once this one is ready, all
others are ready to go (no conflicts or minor conflicts)
Thanks,
-Zhengyu
>
> Regards,
> Alexey
>
> P.S. I'm not a reviewer.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8055705
> [2] https://bugs.openjdk.java.net/browse/JDK-8153732
> [3]
> https://bugs.openjdk.java.net/browse/JDK-8153732?focusedCommentId=14256474#comment-14256474
>
> [4] https://bugs.openjdk.java.net/browse/JDK-8212202
> [5] https://bugs.openjdk.java.net/browse/JDK-8221412
> [6] https://bugs.openjdk.java.net/browse/JDK-8221263
>
> On 04/06/2019 16:08, Zhengyu Gu wrote:
>> Ping!
>>
>> Thanks,
>>
>> -Zhengyu
>>
>> On 5/28/19 9:03 AM, Zhengyu Gu wrote:
>>> Hi,
>>>
>>> Could you please review this 8u backport? After JDK-8055705 backport,
>>> the original patch still does not apply cleanly, due to missing
>>> following Thread API in JDK8u.
>>>
>>> public Thread(ThreadGroup group,
>>> Runnable target,
>>> String name,
>>> long stackSize,
>>> boolean inheritThreadLocals)
>>>
>>> The original patch has inheritThreadLocals = false, but replacement API
>>>
>>> public Thread(ThreadGroup group,
>>> Runnable target,
>>> String name,
>>> long stackSize)
>>> has it default to true.
>>>
>>> Could AWT folks take a look?
>>>
>>>
>>> JDK9 bug: https://bugs.openjdk.java.net/browse/JDK-8153732
>>> Original patch: http://hg.openjdk.java.net/jdk/client/rev/732a3b600098
>>>
>>>
>>> 8u Backport: http://cr.openjdk.java.net/~zgu/JDK-8153732_8u/webrev.01/
>>>
>>> Test:
>>> manual test: RemotePrinterStatusRefresh
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>
More information about the jdk8u-dev
mailing list