<AWT Dev> [PATCH] Cleanup AWT peer interfaces

[Roman Kennke]

Hi Anrei,

Thanks for reviewing! I comment on your comments below:

> 1) src/share/classes/java/awt/peer/CheckboxMenuItemPeer.java
> 
> +import java.awt.CheckboxMenuItem;
> if that's really needed? I thought that {@link ...} doesn't require such 
> stuff.

Hmm, this was most likely added by Eclipse. No, {@link...} doesn't
require this. I'll remove it.

> 2) src/share/classes/java/awt/peer/ContainerPeer.java
> There is a typo in the second word:
> -     * Indicates availabiltity of restacking operation in this container.
> +     * Indicates availability of restacking operation in this container.

I'll fix this.


> 3) Common thing.
> <code>true</code> have a shorter synonym since JDK 5: {@code true}

Oh cool. Didn't know that. I'll fix this too.


> 4) src/share/classes/java/awt/peer/RobotPeer.java
> I noticed that you prefer not to leave "public" modifier in an 
> interface. But here you are leaving all of them.

Oh hmm. Don't know what happened here. I'll remove the mods.


> 5) src/share/classes/java/awt/peer/RobotPeer.java
> public int getNumberOfButtons(); has left w/o any comments.

Oops. Needs fixing.


> 6) src/share/classes/java/awt/peer/ComponentPeer.java
> +    /**
> +     * Called by {@link EventQueue#coalescePaintEvent} to let the component
> +     * peer coalesce paint events.
> +     *
> +     * @param e the paint event to consider to coalesce
> +     *
> +     * @see EventQueue#coalescePaintEvent
> +     */
> +    void coalescePaintEvent(PaintEvent e);
> 
> I'd say it's not "to let .... coalesce paint events", but "to coalesce 
> paint events".

Ok, sounds better.

> 7) at the same class.
> You wrote:
> +    // TODO: Maybe change this to force Graphics2D, since many things will
> +    // break with plain Graphics nowadays.
> +    Graphics getGraphics();
> 
> Do you know a scenario to show what's exactly might be broken.

Many modern applications do stuff like this:

public void paint(Graphics g) {
    Graphics2D g2d = (Graphics2D) g;
    g.doFancyStuff();
}

This will break when we let peers return plain Graphics objects. I don't
have specific examples, but from my time as GNU Classpath developer I
know that quite alot of applications do the above thing, without
checking type. Even the Java2D/Swing tutorials from Sun advertise this.

The thing is that the official API in java.awt.Component only says
java.awt.Graphics, and that has probably been left that way for eternal
backwards compatibility ;-) We can't change this, but we can change the
internal peer interface.

>  We 
> probably need to introduce another peer for that, right?

No, why?


> 8) Similar to 7):
> +    // TODO: Maybe make that return a BufferedImage, because some stuff 
> will
> +    // break if a different kind of image is returned.
> +    Image createImage(int width, int height);
> 

Yeah, the answer is similar to 7 too: there are many apps out that
expect exactly this behaviour, so we should probably give them what they
want. Can't change the public API, but we can change the peers.

> The rest is just fine!
> 
> Finally I've applied the patch and reviewed the webrev. It was just more 
> convenient for me.
> There was an exception for ComponentPeer.java - it caused some conflicts 
> and I didn't succeeded to resolve them fast enough so just looked on the 
> patch itself for that file.
> At last the JDK7 workspace was successfully built.

Cool. I'll do the suggested changes and post another webrev for final
review.

/Roman

-- 
Dipl.-Inform. (FH) Roman Kennke, Software Engineer, http://kennke.org
aicas Allerton Interworks Computer Automated Systems GmbH
Haid-und-Neu-Straße 18 * D-76131 Karlsruhe * Germany
http://www.aicas.com   * Tel: +49-721-663 968-48
USt-Id: DE216375633, Handelsregister HRB 109481, AG Karlsruhe
Geschäftsführer: Dr. James J. Hunt