8363889: Update sun.print.PrintJob2D to use Disposer #26432
Conversation
|
👋 Welcome back prr! A progress list of the required criteria for merging this PR into |
|
@prrace This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 141 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
openjdk
bot
changed the title
Webrevs
|
| * graphics from the queue when the app asks for a graphics. | ||
| */ | ||
| graphicsToBeDrawn.append( (Graphics2D) graphics); | ||
| private static class PrintJobDisposerRecord implements DisposerRecord { |
There was a problem hiding this comment.
should we also make it a final class?
There was a problem hiding this comment.
I don't see that it matters.
| @@ -28,47 +28,14 @@ | |||
| import java.awt.Dimension; | |||
| import java.awt.Frame; | |||
| import java.awt.Graphics; | |||
| import java.awt.Graphics2D; | |||
| import java.awt.PrintJob; | |||
| import java.awt.JobAttributes; | |||
| import java.awt.JobAttributes.*; | |||
| import java.awt.PageAttributes; | |||
| import java.awt.PageAttributes.*; | |||
There was a problem hiding this comment.
wildcard..probably it should be removed as it is used in Delegate class
There was a problem hiding this comment.
yes, they aren't used in this class. I'll remove them.
| @@ -0,0 +1,1233 @@ | |||
| /* | |||
| * Copyright (c) 2000, 2025, Oracle and/or its affiliates. All rights reserved. | |||
There was a problem hiding this comment.
should it be 2000? its a new class but its content is from 2000!!
There was a problem hiding this comment.
Not sure what you are saying.
You seem to be saying it should say 2000 .. but it does .. and yes it should ...
The filename doesn't matter the material is what matters.
| import java.awt.JobAttributes; | ||
| import java.awt.JobAttributes.*; | ||
| import java.awt.PageAttributes; | ||
| import java.awt.PageAttributes.*; |
There was a problem hiding this comment.
Here I'm not 'cleaning up' anything. I just moved it.
| /** | ||
| * Returns true if the last page will be printed first. | ||
| */ | ||
| public boolean lastPageFirst() { |
There was a problem hiding this comment.
because the method just returns false the delegate wasn't needed.
I decided to make it call the delegate anyway for consistency so now it is used.
| /* The caller wants a Graphics instance but we do | ||
| * not want them to make 2D calls. We can't hand | ||
| * back a Graphics2D. The returned Graphics also | ||
| * needs to implement PrintGraphics, so we wrap | ||
| * the Graphics2D instance. The PrintJob API has | ||
| * the application dispose of the Graphics so | ||
| * we create a copy of the one returned by PrinterJob. | ||
| */ | ||
| //printGraphics = new ProxyPrintGraphics(currentGraphics.create(), this); | ||
| printGraphics = currentGraphics.create(); |
There was a problem hiding this comment.
Do we need the comment on line 823?
The returned Graphics also to implement PrintGraphics, so we wrap the Graphics2D instance.
That doesn't seem true anymore.
There was a problem hiding this comment.
Moving part of this comment back over to PrintJob2D.
|
/integrate |
|
Going to push as commit d1e362e.
Your commit was automatically rebased without conflicts. |
openjdk
bot
removed
ready
As laid out in the JBS issue, this updates the java.awt.PrintJob implementation to use a delegate which can be disposed instead of relying on finalize().
PrintJob.finalize() is not being removed, and won't be removed until there is a more definite plan for completely removing the finalize() mechanism.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26432/head:pull/26432$ git checkout pull/26432Update a local copy of the PR:
$ git checkout pull/26432$ git pull https://git.openjdk.org/jdk.git pull/26432/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 26432View PR using the GUI difftool:
$ git pr show -t 26432Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26432.diff
Using Webrev
Link to Webrev Comment