RFR: 8288137: The set of available printers is not updated without application restart

Phil Race prr at openjdk.org
Fri Jul 8 20:07:13 UTC 2022


On Fri, 8 Jul 2022 15:07:46 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> This fixes the issue that the default printer or set of available printers is fixed from the first call to it in the lifetime of the application.
>> Now subsequent calls will check to see if there are changes.
>> A manual test is provided for verifying this - it requires you to add/remove printers at the system level to verify anything related to this fix.
>
> modules/javafx.graphics/src/main/java/com/sun/prism/j2d/PrismPrintPipeline.java line 138:
> 
>> 136:             }
>> 137:         }
>> 138:         lastTime = System.currentTimeMillis();
> 
> Is `lastTime` intended to be the last time you updated the printers or the last time the `getAllPrinters` method was called? If the former, then you would want to set `lastTime` right after `updatePrinters`. If the latter, then it's fine where it is.

The last time we updated.
If the number of printers has changed I will always rebuild the printer list (set) but if they are the same
what are the odds a printer was removed and another one added ? 
I figured if 2 minutes has passed since we last checked it is worth it, but I am not re-setting this every
time we called getAllPrinters() else if you kept calling it every 30 seconds .. then we'd never get past that check.

> modules/javafx.graphics/src/main/java/com/sun/prism/j2d/PrismPrintPipeline.java line 192:
> 
>> 190:         if (oldDefaultService != null && newDefaultService != null) {
>> 191:             if (!oldDefaultService.equals(newDefaultService)) {
>> 192:                 defaultPrinter = findDefaultPrinter(printerSet, newDefaultService);
> 
> Don't you also want to find the default printer in the case where the old `default` printer was null and the `newDefaultService` is not null?

Updating this.

> modules/javafx.graphics/src/main/java/javafx/print/Printer.java line 95:
> 
>> 93:         } else {
>> 94:         defaultPrinter.setValue(p);
>> 95:     }
> 
> Minor: indentation is off for these two lines

That's what you get for trying to use an IDE ... will fix ..

-------------

PR: https://git.openjdk.org/jfx/pull/817


More information about the openjfx-dev mailing list