[OpenJDK 2D-Dev] <Swing Dev> [11] JDK-8153732: Windows remote printer changes do not reflect in lookupPrintServices()
Prasanta Sadhukhan
prasanta.sadhukhan at oracle.com
Fri Jun 22 05:10:50 UTC 2018
I guess
if (pollStr.equalsIgnoreCase("true")) {
70 pollServices = true;
71 } else if (pollStr.equalsIgnoreCase("false")) {
72 pollServices = false;
73 } can be written as
if (pollStr.equalsIgnoreCase("false")) pollServices = false; as it is
already defaulted to true.
78 * for polling PrintServices. The default is 120.
I guess default is 240.
also, EnumPrinters returns bool, which we should check like
263 if (cReturned > 0 && enumprintersret !=0 ) { Regards Prasanta
On 6/22/2018 10:03 AM, Shashidhara Veerabhadraiah wrote:
>
> Hi Phil, Thanks for your review.
>
> I have made your suggested changes and here is the updated webrev:
>
> http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.04/
> <http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.04/>
>
> Need one more review to commit this.
>
> Note: A network printer can be added via the ‘Add a printer’ under the
> ‘Devices and Printers’ dialog. A network printer will have the
> property ‘Device description’ set to ‘Network printer connection’.
>
> Thanks and regards,
>
> Shashi
>
> *From:*Phil Race
> *Sent:* Friday, June 22, 2018 2:16 AM
> *To:* Shashidhara Veerabhadraiah
> <shashidhara.veerabhadraiah at oracle.com>; 2d-dev <2d-dev at openjdk.java.net>
> *Subject:* Re: <Swing Dev> [11] JDK-8153732: Windows remote printer
> changes do not reflect in lookupPrintServices()
>
> I thought you were going to make the refresh time 4 minutes ?
> I don't see that it has to be the same on Windows as it was on Unix,
> if you say 4 minutes is a sensible value there .. and 4 mins will be
> less CPU wake up, so I'd back that (4 mins) ahead of 2 minutes as the
> default here.
>
> You still have missing white space
>
> 432 while(true) {
>
>
> With these two changes you have my +1 ..
>
> BTW the native diff looks MUCH better now - thanks !
>
> -phil.
>
> On 06/21/2018 01:33 PM, Shashidhara Veerabhadraiah wrote:
>
> Hi Phil, Here is the new Webrev for changes you suggested.
>
> http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.03/
> <http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.03/>
>
> Thanks and regards,
>
> Shashi
>
> *From:*Phil Race
> *Sent:* Friday, June 22, 2018 1:02 AM
> *To:* Shashidhara Veerabhadraiah
> <shashidhara.veerabhadraiah at oracle.com>
> <mailto:shashidhara.veerabhadraiah at oracle.com>; 2d-dev
> <2d-dev at openjdk.java.net> <mailto:2d-dev at openjdk.java.net>
> *Subject:* Re: <Swing Dev> [11] JDK-8153732: Windows remote
> printer changes do not reflect in lookupPrintServices()
>
>
>
> + private static final long DELAY = 1000 * 60 * 4; // 4 min
> pooling
>
> I think we need a System Property that can control this.
>
> I suggest the name "sun.java2d.print.minRefreshTime" which is what
> we use on Unix.
>
> and similarly to there we should have "sun.java2d.print.polling"
> which is a boolean
>
> and controls whether we do this at all ..
>
> See
> src/java.desktop/unix/classes/sun/print/PrintServiceLookupProvider.java
>
> Lots of places where you are missing a space before "("
>
> + if(str1.length != str2.length) {
>
> + for(int i = 0;i < str1.length;i++) {
>
> + for(int j = 0;j < str2.length;j++) {
>
> + if(!str1[i].equals(str2[j])) {
>
>
>
> + while(true) {
>
> + if(doCompare(prevRemotePrinters,
> currentRemotePrinters)) {
>
> There's some of that in native too
>
> 266 if(info4->Attributes & 0x00000010) {
>
> In
> http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.01/src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp.sdiff.html
> <http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.01/src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp.sdiff.html>
>
> you seem to have moved all the existing and (I think) unchanged
> related functions so the DIFF appears
>
> much bigger than it really is. Please move them back and re-generate.
>
> The test really needs to provide an "out" for someone running the
> test who has no way
>
> to add a network printer .. they don't want to have to fail the test.
>
> As well as that, arguably this should be an @ignore test, so it is
> not run unless
>
> you are trying to run all the tests.
>
> -phil.
>
> On 06/21/2018 12:08 PM, Shashidhara Veerabhadraiah wrote:
>
> Hi Phil, Here is the new Webrev. I chose 4 mins because I
> think it takes around 2 mins to add a new network printer,
> hence I felt 4 mins is a good time.
>
> The windows **PrinterChangeNotifications** calls are a
> blocking function calls hence I could not add the remote
> printers monitor to the existing thread. Hence there is a new
> thread being added to listen to remote printers status changes.
>
> http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.01/
> <http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.01/>
>
> Thanks and regards,
>
> Shashi
>
> *From:*Philip Race
> *Sent:* Thursday, June 21, 2018 5:38 AM
> *To:* 2d-dev <2d-dev at openjdk.java.net>
> <mailto:2d-dev at openjdk.java.net>; Shashidhara Veerabhadraiah
> <shashidhara.veerabhadraiah at oracle.com>
> <mailto:shashidhara.veerabhadraiah at oracle.com>
> *Subject:* Fwd: Re: <Swing Dev> [11] JDK-8153732: Windows
> remote printer changes do not reflect in lookupPrintServices()
>
> The main concern I have is we now have a busy thread burning
> CPU ..
> bad for laptops .. and if we add a delay we have less prompt
> notification
> of a new local printer.
>
> I think the compromise is that the existing thread maybe kept
> as is,
> and we add a new thread that pools every 10 minutes for a
> remote printer.
>
> If we can make the existing thread wake up from its wait and
> do that, even better.
>
> -phil.
>
> -------- Original Message --------
>
> *Subject: *
>
>
>
> Re: [11] JDK-8153732: Windows remote printer changes do not
> reflect in lookupPrintServices()
>
> *Date: *
>
>
>
> Wed, 20 Jun 2018 17:03:56 -0700
>
> *From: *
>
>
>
> Philip Race <philip.race at oracle.com>
> <mailto:philip.race at oracle.com>
>
> *Organization: *
>
>
>
> Oracle Corporation
>
> *To: *
>
>
>
> Shashidhara Veerabhadraiah
> <shashidhara.veerabhadraiah at oracle.com>
> <mailto:shashidhara.veerabhadraiah at oracle.com>
>
> *CC: *
>
>
>
> awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>,
> swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
>
>
>
> This is on the wrong lists. Not Swing. Not AWT. Should be 2d.
> I'll forward it there and continue there. Consider the
> AWT+Swing threads dead.
>
> -phil.
>
> On 6/20/18, 3:12 AM, Shashidhara Veerabhadraiah wrote:
>
> Hi All, Please review this code changes for the below
> enhancement.
>
> Enhancement: https://bugs.openjdk.java.net/browse/JDK-8153732
>
> Webrev:
> http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.00/
> <http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.00/>
>
> Details of the changes: Windows provides
> *PrinterChangeNotification* functions that provides
> information about printer status changes of the local
> printers(subset) but not network printers.
> Alternatively, Windows provides a way thro' which one can
> get the network printer status changes by using WMI,
> RegistryKeyChange combination, which is a slightly complex
> mechanism.
> The Windows WMI offers an async and sync method to read
> thro' registry via the WQL query. The async method is
> considered dangerous as it leaves open a channel until we
> close it. But the async method has the advantage of being
> notified of a change in registry by calling callback
> without polling for it. The sync method uses the polling
> mechanism to notify.
> RegistryValueChange cannot be used in combination with WMI
> to get registry value change notification because of an
> error that may be generated because the scope of the query
> would be too big to handle(at times).
> Hence an alternative mechanism is choosen via the
> EnumPrinters by polling for the count of printer status
> changes(add\remove) and based on it update the printers
> list(both local and remote printers - superset).
>
> Thanks and regards,
>
> Shashi
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180622/7605f5a8/attachment-0001.html>
More information about the 2d-dev
mailing list