My bad Prashanta. Here is the updated Webrev:
http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.07/
<http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.07/>
Thanks and regards,
Shashi
*From:*Prasanta Sadhukhan
*Sent:* Friday, June 22, 2018 2:46 PM
*To:* Shashidhara Veerabhadraiah
<shashidhara.veerabhadra...@oracle.com>; Philip Race
<philip.r...@oracle.com>; 2d-dev <2d-dev@openjdk.java.net>
*Subject:* Re: [OpenJDK 2D-Dev] <Swing Dev> [11] JDK-8153732: Windows
remote printer changes do not reflect in lookupPrintServices()
you are using in java
409 prevRemotePrinters = GetRemotePrintersNames();
431 String[] currentRemotePrinters =
GetRemotePrintersNames();
while you are at it, please change
269 if (info4->Attributes & 0x00000010) {
to
if (info4->Attributes & PRINTER_ATTRIBUTE_NETWORK) {
as the hardcoded value might change in future.
Regards
Prasanta
On 6/22/2018 2:32 PM, Shashidhara Veerabhadraiah wrote:
Hi Prasanta, Here is the new webrev for that change:
http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.06/
<http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.06/>
Thanks and regards,
Shashi
*From:*Prasanta Sadhukhan
*Sent:* Friday, June 22, 2018 2:00 PM
*To:* Shashidhara Veerabhadraiah
<shashidhara.veerabhadra...@oracle.com>
<mailto:shashidhara.veerabhadra...@oracle.com>; Philip Race
<philip.r...@oracle.com> <mailto:philip.r...@oracle.com>; 2d-dev
<2d-dev@openjdk.java.net> <mailto:2d-dev@openjdk.java.net>
*Subject:* Re: [OpenJDK 2D-Dev] <Swing Dev> [11] JDK-8153732:
Windows remote printer changes do not reflect in lookupPrintServices()
btw,
GetRemotePrintersNames
violates camelcase style. I suggest to use getRemotePrintersNames
Regards
Prasanta
On 6/22/2018 1:56 PM, Shashidhara Veerabhadraiah wrote:
Thank you Prasanta for the review.
Thanks and regards,
Shashi
*From:*Prasanta Sadhukhan
*Sent:* Friday, June 22, 2018 1:42 PM
*To:* Shashidhara Veerabhadraiah
<shashidhara.veerabhadra...@oracle.com>
<mailto:shashidhara.veerabhadra...@oracle.com>; Philip Race
<philip.r...@oracle.com> <mailto:philip.r...@oracle.com>;
2d-dev <2d-dev@openjdk.java.net> <mailto:2d-dev@openjdk.java.net>
*Subject:* Re: [OpenJDK 2D-Dev] <Swing Dev> [11] JDK-8153732:
Windows remote printer changes do not reflect in
lookupPrintServices()
looks good. One more thing, you could probably use
if (info4->Attributes & PRINTER_ATTRIBUTE_NETWORK) {
instead of hardcoding
if (info4->Attributes & 0x00000010) {
Also,
getAllPrinterNames() shares more than 80% code with your recently
addedGetRemotePrintersNames(). Probably we could optimise
getAllPrinterNames to use yours
by having a parameter(all/remote) but it's upto you.
Regards
Prasanta
On 6/22/2018 12:53 PM, Shashidhara Veerabhadraiah wrote:
Hi Prasanta, Thank you for the review. Here is the new Webrev:
http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.05/
<http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.05/>
For the EnumPrinters case, I think the cReturned is
sufficient as it is set to zero every time we start.
Thanks and regards,
Shashi
*From:*Prasanta Sadhukhan
*Sent:* Friday, June 22, 2018 10:41 AM
*To:* Shashidhara Veerabhadraiah
<shashidhara.veerabhadra...@oracle.com>
<mailto:shashidhara.veerabhadra...@oracle.com>; Philip
Race <philip.r...@oracle.com>
<mailto:philip.r...@oracle.com>; 2d-dev
<2d-dev@openjdk.java.net> <mailto:2d-dev@openjdk.java.net>
*Subject:* Re: [OpenJDK 2D-Dev] <Swing Dev> [11]
JDK-8153732: Windows remote printer changes do not reflect
in lookupPrintServices()
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.veerabhadra...@oracle.com>
<mailto:shashidhara.veerabhadra...@oracle.com>; 2d-dev
<2d-dev@openjdk.java.net> <mailto:2d-dev@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.veerabhadra...@oracle.com>
<mailto:shashidhara.veerabhadra...@oracle.com>;
2d-dev <2d-dev@openjdk.java.net>
<mailto:2d-dev@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@openjdk.java.net>
<mailto:2d-dev@openjdk.java.net>; Shashidhara
Veerabhadraiah
<shashidhara.veerabhadra...@oracle.com>
<mailto:shashidhara.veerabhadra...@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.r...@oracle.com>
<mailto:philip.r...@oracle.com>
*Organization: *
Oracle Corporation
*To: *
Shashidhara Veerabhadraiah
<shashidhara.veerabhadra...@oracle.com>
<mailto:shashidhara.veerabhadra...@oracle.com>
*CC: *
awt-...@openjdk.java.net
<mailto:awt-...@openjdk.java.net>,
swing-...@openjdk.java.net
<mailto:swing-...@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
