[OpenJDK 2D-Dev] [9] RFR JDK-8034239, , PrintServiceLookup.lookupPrintServices() returns different amount of services in comparison with lpstat -v

[Philip Race]

Approved.

-phil

On 3/8/16, 2:26 AM, prasanta sadhukhan wrote:
> Hi Phil,
>
> It's my mistake. There's no need to copy as we are not passing the 
> supportedDocFlavors outside the class. I was influenced by the lines 
> before this where copyFlavors was copied from supportedDocFlavors.
> Please find the updated webrev:
> http://cr.openjdk.java.net/~psadhukhan/8034239/webrev.02/
>
> Regards
> Prasanta
> On 3/8/2016 1:58 AM, Phil Race wrote:
>> Why do the copy ? Why not just add  one line
>>
>> supportedDocFlavors = flavor
>>
>> -phil.
>>
>> On 03/07/2016 03:20 AM, prasanta sadhukhan wrote:
>>> Hi Phil,
>>>
>>> Please find the updated webrev:
>>> http://cr.openjdk.java.net/~psadhukhan/8034239/webrev.01/
>>>
>>> I checked JDK-8040139 also passed with this modification.
>>>
>>> Regards
>>> Prasanya
>>> On 3/5/2016 1:14 AM, Phil Race wrote:
>>>> if the class is supposed to use the return value in this way what 
>>>> about these lines :-
>>>>
>>>> 1209
>>>> 1210     public boolean isIPPSupportedImages(String mimeType) {
>>>> 1211         if (supportedDocFlavors == null) {
>>>> 1212             getSupportedDocFlavors();
>>>> 1213         }
>>>>
>>>> It looks to me as if what you should really be doing is modifying
>>>> the last few lines of getSupportedDocFlavors() as you are using
>>>> the return value to update the variable which it should have
>>>> assigned itself.
>>>>
>>>> -phil
>>>>
>>>>
>>>>
>>>> On 03/04/2016 02:48 AM, prasanta sadhukhan wrote:
>>>>> Hi Phil,
>>>>>
>>>>> Please review a print service fix for jdk9
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8034239
>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8034239/webrev.00/
>>>>>
>>>>> The issue was lpstat -v was reporting 2 printer service but 
>>>>> PrintServiceLookup.lookupPrintServices() list only 1.
>>>>> #lpstat -v
>>>>> device for tta_printer: 
>>>>> ipp://<internal-printer-ip>/printers/tta_printer 
>>>>> <ipp://st-tta-adc-02.us.oracle.com/printers/tta_printer>
>>>>> device for Xerox-WorkCentre-4250: socket://spb04p12.ru.oracle.com:9100
>>>>>
>>>>> but lookupPrintServices() listed only "Xerox" printer and not 
>>>>> "tta_printer".
>>>>>
>>>>> It seems "tta_printer" was added using
>>>>> *lpadmin -pprinter-name  -E -vdevice  -m ppd
>>>>>
>>>>> *
>>>>> so it becomes a "Remote Printer" and even though Java sees that 
>>>>> printer, it calls getUnsupportedAttributes() on that printer 
>>>>> service to see if that printer matches the DocFlavor.
>>>>> In this case, this remote printer does not have any supported doc 
>>>>> flavor listed so when getSupportedDocFlavors() is called, it does 
>>>>> not see any "document-format-supported" attribute.
>>>>> Although JDK-8040139 fixed this problem by adding
>>>>> DocFlavor.SERVICE_FORMATTED.PAGEABLE andDocFlavor.SERVICE_FORMATTED.PRINTABLE
>>>>>
>>>>> to the supported DocFlavors since as per spec,
>>>>> as per spec
>>>>> /https://docs.oracle.com/javase/8/docs/api/index.html?javax/print/PrintService.html//
>>>>> //it clearly says, /
>>>>> getSupportedDocFlavors()
>>>>>
>>>>> /Returns:/
>>>>>     /Array of supported doc flavors, should have at least one
>>>>>     element./
>>>>>
>>>>>
>>>>> the returned flavours are not populated to supported DocFlavors 
>>>>> called by IPPPrintService.java#getUnsupportedAttributes() -> 
>>>>> isDocFlavorSupported() so "supportedDocFlavors" was still false 
>>>>> for this case.
>>>>>
>>>>> I added a fix to populate the PAGEABLE & PRINTABLE docflavor to 
>>>>> "supportedDocFlavors" and tested in sqe machine and found it working.
>>>>>
>>>>> I could not add a regression testcase as it involves adding a 
>>>>> "remote printer" using lpadmin command. I would add "noreg-sqe" to 
>>>>> the bug.
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160308/65f33e2f/attachment.html>