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

[Anthony Petrov]

Hi Alex,

The fix looks good to me.

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.

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