[7u6] request for review: 7124328: [macosx] javax.swing.JDesktopPane.getAllFramesInLayer returns unexpected value

[Anthony Petrov]

OK then. The fix is fine anyway.

--
best regards,
Anthony

On 4/17/2012 5:30 PM, Alexander Potochkin wrote:
> Hello Anthony
>> Hi Alex,
>>
>> The fix looks good to me.
> thanks
>>
>> Though if I were you, I would add an argument to the new static method 
>> to allow for inclusion of internal frame from a specified layer only, 
>> or all layers if the argument is equal to e.g. MAX_INT. This way we 
>> could avoid adding and then removing some of the found frames to/from 
>> the collection.
> 
> MAX_INT is a valid parameter for JLayeredPane.putLayer(JComponent, int),
> negative values are also ok, for example 
> JLayeredPane.FRAME_CONTENT_LAYER is defined as -30000
> 
> so there is no "reserved" integer to serve as a special flag for frames 
> in all layers
> 
> alexp
> 
>>
>> -- 
>> best regards,
>> Anthony
>>
>> On 4/16/2012 10:30 PM, Alexander Potochkin wrote:
>>> Hello Anthony
>>>
>>> Thanks for reviewing it, here is the new version:
>>>
>>> http://cr.openjdk.java.net/~alexp/7124328/webrev.01/
>>>
>>>> Hi Alex,
>>>>
>>>> It seems like the new private getAllFrames(Contianer) method may be 
>>>> declared as static. Also I suggest to insert spaces between "if"s 
>>>> and "("s at lines 275, 277, and 282. Otherwise the changes look good.
>>>
>>> fixed
>>>
>>>>
>>>> BTW, do we have any regression tests for this method? Do they pass? 
>>>
>>> It's a pretty good question
>>> :-)
>>>
>>> There is no regression tests for JDesktopPane.getAllFrames()
>>> but I've found a regression test for the old bug 4132993 which fixes 
>>> the problem with
>>> JDesktopPane.getAllFramesInLayer().
>>>
>>> This method also should return iconified frames in a specified layer,
>>> it didn't work for Aqua so I fixed it as well
>>>> What about the performance regression caused by iterating over the 
>>>> inner containers?
>>>
>>> No observable performance regression.
>>>
>>> Thanks
>>> alexp
>>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 4/10/2012 9:25 PM, Alexander Potochkin wrote:
>>>>> Hello
>>>>>
>>>>> Please review the following fix:
>>>>> http://cr.openjdk.java.net/~alexp/7124328/webrev.00/
>>>>>
>>>>> for this bug:
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7124328
>>>>>
>>>>> the spec for the method
>>>>> javax.swing.JDesktopPane.getAllFrames()
>>>>>
>>>>> says
>>>>>
>>>>> * Returns all <code>JInternalFrames</code> currently displayed in the
>>>>> * desktop. Returns iconified frames as well as expanded frames.
>>>>>
>>>>> but the AquaLookAndFeel on MacOS uses a special container for the 
>>>>> minimized internal frames
>>>>> and that's why the current implementation doesn't see them
>>>>>
>>>>> the proposed fix recursively looks for the internal frames
>>>>> among the JDesktopPane's children
>>>>>
>>>>> the test can be found here:
>>>>> http://java.net/jira/browse/MACOSX_PORT-557
>>>>>
>>>>> Thanks
>>>>> alexp
>>>
>