[OpenJDK 2D-Dev] [13] RFR: JDK-8221412: lookupPrintServices() does not always update the list of Windows remote printers

[Phil Race]

OK. Approved.

-phil

On 3/27/19 11:12 AM, Alexey Ivanov wrote:
> Hi Phil,
>
> On 27/03/2019 00:29, Philip Race wrote:
>>
>> > The old implementation of getRemotePrintersNames() got the list of 
>> both local and remote printers and then filtered out local ones.
>>
>> The old implementation checked that
>> if (info4->Attributes & PRINTER_ATTRIBUTE_NETWORK) { So you don't 
>> think this is needed ?
>
> No, I don't.
>
> The old implementation asked for both local and remote printers:
> ::EnumPrinters(PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS, …
>
> Then it removed local printers from the array.
>
> In getRemotePrintersNames() I ask for remote printers only:
> ::EnumPrinters(PRINTER_ENUM_CONNECTIONS, …
> If the list is not empty, it contains only remote printers. Nothing to 
> filter.
>
>
> Regards,
> Alexey
>
>> -phil.
>>
>> On 3/26/19, 1:22 PM, Alexey Ivanov wrote:
>>> Please see the updated webrev:
>>> http://cr.openjdk.java.net/~aivanov/8221412/webrev.1/
>>>
>>> The difference between .0 and .1 is in some minor white-space 
>>> adjustments, like parameter alignment.
>>>
>>> On 26/03/2019 17:01, Alexey Ivanov wrote:
>>>> Hi,
>>>>
>>>> Please review the fix for jdk 13:
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8221412
>>>> webrev: http://cr.openjdk.java.net/~aivanov/8221412/webrev.0/
>>>>
>>>> Description:
>>>> The list of printers is not updated in the situation where there 
>>>> are no remote printers on the system and later the user adds a 
>>>> remote printer.
>>>>
>>>> Root cause:
>>>> JDK-8153732 [1] added a new thread which polls remote printers on 
>>>> the system and updates the list if it detects a change. In this 
>>>> case it does not update the print services because of this condition:
>>>> 449 if (prevRemotePrinters != null && prevRemotePrinters.length > 0)
>>>> prevRemotePrinters is not null but its length is zero because no 
>>>> remote printers were initially present on the system.
>>>>
>>>> Fix:
>>>> I removed this if. We have to update the list printers if 
>>>> doCompare() returns true. Either prevRemotePrinters or 
>>>> currentRemotePrinters, or both can be null; doCompare() handles 
>>>> this situation gracefully after JDK-8212202 [2].
>>>>
>>>> The listener called getRemotePrintersNames() twice: in constructor 
>>>> and in run() before entering the first sleep. Now it's done only once.
>>>>
>>>> I consolidated the implementation of getAllPrinterNames() and 
>>>> getRemotePrintersNames(). They were very similar. The old 
>>>> implementation of getRemotePrintersNames() got the list of both 
>>>> local and remote printers and then filtered out local ones. The new 
>>>> implementation gets the list of remote printers only. So the 
>>>> difference between the two is limited to the flags passed to 
>>>> EnumPrinters.
>>>>
>>>> If there are no remote printers, the new version 
>>>> getRemotePrintersNames() returns null instead of empty array. As 
>>>> mentioned earlier, doCompare() handles this situation.
>>>>
>>>> I'll add the bugid (8221412) to the regression test when fixing 
>>>> JDK-8221263 [3]. I'm currently working on it.
>>>>
>>>>
>>>> Thank you in advance.
>>>>
>>>> Regards,
>>>> Alexey
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8153732
>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8212202
>>>> [3] https://bugs.openjdk.java.net/browse/JDK-8221263
>