[8] Review request for 7124213 and 7160627

[Anthony Petrov]

Thanks for the clarifications. I don't like how the code looks now, but 
the fix looks OK technically.

--
best regards,
Anthony

On 9/12/2012 6:08 PM, Sergey Bylokhov wrote:
> 12.09.2012 17:51, Anthony Petrov wrote:
>> On 9/12/2012 5:42 PM, Sergey Bylokhov wrote:
>>> 12.09.2012 16:50, Anthony Petrov wrote
>>>> src/macosx/classes/sun/lwawt/LWComponentPeer.java
>>>>>  880      * empty. In the XPeers or WPeers we use some magic 
>>>>> constants, but here we
>>>>>  881      * try to use something more useful,
>>>>
>>>> Why can't we use "some magic constants" here, and the constant 1 in 
>>>> particular? This choice may be relevant to components that display 
>>>> some text, but e.g. for a container component using text-based 
>>>> metrics doesn't look right. 
>>> Containers uses getP/getM methods from the LWCanvasPeer. Default 
>>> implementation in LWComponentPeer is applicable for usual components 
>>> only.
>>>> Also, I see that "w" was used previously, and you're changing this 
>>>> to "0". Perhaps "W" might work best?
>>> Just because it is used in the XToolkit and 
>>> Wtoolkit.(WTextAreaPeer/XTextAreaPeer).
>>
>> Should we extract this constant to a java.awt.peer.* interface then 
>> for consistency across all toolkits?
> Well, possibility to change this constant for different toolkits should be.
>>
>>>> src/macosx/classes/sun/lwawt/LWContainerPeer.java
>>>>>   43 abstract class LWContainerPeer<T extends Container, D extends 
>>>>> JComponent>
>>>>>   44     extends LWCanvasPeer<T, D>
>>>>
>>>> A Canvas peer implementation may be "heavier" since it can pull some 
>>>> graphics-related code which is unnecessary for simple containers.
>>> Most of the graphics code will be in the CGLGraphicsConfig.
>>
>> I see. But in the future we may want to modify the LWCanvasPeer, and 
>> this would also unintentionally affect the LWContainerPeer. I still 
>> suggest to extract all common code into a new abstract class.
> Nobody knows that will be in the future. But actually, if I'll move all 
> this code to the separate class our LWCanvasPeer becomes noop. Note that 
> in the WToolkit/XToolkit, container is subclass of X/W.CanvasPeer. From 
> this point of view this is unification across toolkits.
>>
>> -- 
>> best regards,
>> Anthony
>>
>>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 9/11/2012 9:38 PM, Sergey Bylokhov wrote:
>>>>>   Hi Everyone,
>>>>> Please review the fix for:
>>>>> 7124213 [macosx] pack() does ignore size of a component; doesn't on 
>>>>> the other platforms.
>>>>> 7160627 [macosx] TextArea has wrong initial size
>>>>>
>>>>> Description of main changes:
>>>>> All our components were split into 3 groups:
>>>>>  - Uses getPreferedSize()/getMinimumSise from swing delegetes.
>>>>>  - Uses its own size as a preferedSize/minimumSize.
>>>>>  - Uses its own implementation.
>>>>>
>>>>> Notes:
>>>>> LWContainerPeer is subclass of LWCanvasPeer now. We can share 
>>>>> buffers methods in LWCanvasPeer and LWWindowPeer.
>>>>> All magic/system constants were removed. Now we relies on default 
>>>>> look and feel as much as possible.
>>>>>
>>>>> Bugs: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7160627
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7124213
>>>>> ** Webrev can be found at: 
>>>>> http://cr.openjdk.java.net/~serb/7124213_7160627/webrev.00/
>>>>>
>>>>> -- 
>>>>> Best regards, Sergey.
>>>
>>>
> 
>