Review request for MACOSX_PORT-539: Need a java.awt.EmbeddedFrame subclass

[Dmitry Cherepanov]

Hi Artem,

Artem Ananiev wrote:
> Hi, Dmitry,
>
> here are my comments:
>
> 1. CEmbeddedFrame and CPlatformEmbeddedFrame are probably not the best 
> names. I can't suggest anything better, though.
I can't come up with anything better too but hopefully these names make 
sense to some extent.

 - "CEmbeddedFrame" seems to conform to existing names - 
"WEmbeddedFrame" on Windows and "XEmbeddedFrame" on X11.
 - "CPlatformEmbeddedFrame" is consistent with "CPlatformWindow".
>
> 2. Is there a reason to explicitly call addNotify() in CEF 
> constructor? Will it be called automatically as a part of show()?
Done.
>
> 3. CEF.handle*Event(): please, add (responder != null) checks there to 
> protect these methods from calling on invisible frames.
My understanding is that there's no strong reason to write these checks 
for null values. If there's a key/mouse event coming from the Plugin in 
case of invisible embedded frames, I would say that such events are 
unexpected and shouldn't be triggered.
>
> 4. There are several (platformWindow instanceof CPlatformWindow) 
> checks added. Could you comment on this, please? Do you expect 
> anything else than CPlatformWindow there?
All these checks are needed to handle CPlatformEmbeddedFrame case:

src/macosx/classes/sun/lwawt/macosx/CDropTarget.java

    skips registering drop target on CPlatformEmbeddedFrame (Scott wrote 
that the Plugin doesn't deliver drag-and-drop events).

src/macosx/classes/sun/lwawt/macosx/CInputMethod.java

    skips notifying CInputMethod instances about focus/IME events (input 
methods aren't implemented yet in embedded env).

src/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java

    skip using embedded frames (which don't have underlying 
NSView/NSWindow) as owner windows.
>
> 5. What is the reason of exporting the getPeer() method in the 
> PlatformWindow interface?
It's done as a part of the changes in 
src/macosx/classes/sun/lwawt/macosx/CPlatformComponent.java

The change removes explicit reference to the CPlatformWindow class (to 
make it working for CPlatformEmbeddedFrame) but it still needs to call 
platformWindow.getPeer() (see the setBounds method). It could probably 
be refactored into a more simplified code (to remove the getPeer method 
from the PlatformWindow interface).

Thanks for reviewing this!

Dmitry
>
> Thanks,
>
> Artem
>
> On 12/29/2011 6:06 PM, Dmitry Cherepanov wrote:
>> Hello,
>>
>> Please review an initial implementation for
>> http://java.net/jira/browse/MACOSX_PORT-539 at
>>
>> http://cr.openjdk.java.net/~dcherepanov/7124335/webrev.0/
>>
>> Basically the fix provides a lightweight implementation of the
>> EmbeddedFrame class (isn't backed by a NSView/NSWindow). The
>> implementation creates an instance of CALayer and exposes it as
>> protected CEmbeddedFrame method.
>>
>> Please see the bug report http://java.net/jira/browse/MACOSX_PORT-539
>> for more details about the current version of the fix.
>>
>> Thanks,
>> Dmitry
>>