<AWT Dev> [PATCH] Cleanup AWT peer interfaces

[Andrei V. Dmitriev]

Hi Roman,

here are some comments on the patch.

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.

The same for the following classes:
src/share/classes/java/awt/peer/ChoicePeer.java file.
src/share/classes/java/awt/peer/FileDialogPeer.java
src/share/classes/java/awt/peer/FramePeer.java
src/share/classes/java/awt/peer/LabelPeer.java
src/share/classes/java/awt/peer/MenuComponentPeer.java
src/share/classes/java/awt/peer/MenuItemPeer.java
src/share/classes/java/awt/peer/MenuPeer.java
src/share/classes/java/awt/peer/PopupMenuPeer.java
src/share/classes/java/awt/peer/ScrollPanePeer.java
src/share/classes/java/awt/peer/SystemTrayPeer.java
src/share/classes/java/awt/peer/TextAreaPeer.java
src/share/classes/java/awt/peer/TextComponentPeer.java
src/share/classes/java/awt/peer/TextFieldPeer.java
src/share/classes/java/awt/peer/TrayIconPeer.java


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.


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


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.


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


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".


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. We 
probably need to introduce another peer for that, right?


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);

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.

Thanks,
   Andrei







Roman Kennke wrote:
> Hi,
> 
> 
>> I found that reasonable since the very first time evaluated the cacio 
>> code. Though this patch probably requires more time to get reviewed...
> 
> So? We do it in a separate review&push or we put both (the peer cleanup
> and the documentation) together?
> 
> /Roman
> 
>> Thanks,
>>    Andrei
>>
>> Roman Kennke wrote:
>>> Hi Andrei,
>>>
>>>>> Yeah sure. I reviewed all the existing implementations and they should
>>>>> not break, but a lot of stuff could be removed.
>>>> Just appeared to public access:
>>>> http://bugs.sun.com/view_bug.do?bug_id=6749920
>>> Cool. BTW, I have another patch for the peer interfaces in the Cacio
>>> repo, that one adds API docs to all the interfaces. Nothing serious,
>>> only roughly what it does (this might be obvious by the method names
>>> anyway) and pointers to where it is used in AWT (I found this important
>>> for implementing the peers, so that people can look up the nitty gritty
>>> details themselves). In the projects I was involved in so far we had
>>> policies to separate code changes from doc and formatting changes, but I
>>> don't know how this is handled in OpenJDK. Maybe we want to push these
>>> together. Find this doc patch attached (this follows up on the
>>> cleanpeers patch, will not apply on clean OpenJDK!)
>>>
>>> Cheers, Roman
>>>
>>>