Review request for MACOSX_PORT-539: Need a java.awt.EmbeddedFrame subclass (original) (raw)
Dmitry Cherepanov dmitry.cherepanov at oracle.com
Wed Jan 11 05:31:58 PST 2012
- Previous message: Review request for MACOSX_PORT-539: Need a java.awt.EmbeddedFrame subclass
- Next message: Review request for MACOSX_PORT-539: Need a java.awt.EmbeddedFrame subclass
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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/MACOSXPORT-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/MACOSXPORT-539 for more details about the current version of the fix. Thanks, Dmitry
- Previous message: Review request for MACOSX_PORT-539: Need a java.awt.EmbeddedFrame subclass
- Next message: Review request for MACOSX_PORT-539: Need a java.awt.EmbeddedFrame subclass
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]