Hi Mario,
See comments below...
2012/4/16 Pavel Porvatov<pavel.porva...@oracle.com>:
Hello Pavel,
About the test, I'm not sure if you need something more specific, but
I was thinking to simply test if when isLightWeightPopupEnabled is set
we create an heavy or lightweight popup, any other ideas?
That's a good way I think
Here is the webrev:
http://cr.openjdk.java.net/~neugens/6800513/
Note that MEDIUM_WEIGHT is still used in some cases (PopupFactory.getPopup):
if (owner == null || invokerInHeavyWeightPopup(owner)) {
popupType = HEAVY_WEIGHT_POPUP;
}
else if (popupType == LIGHT_WEIGHT_POPUP&&
!(contents instanceof JToolTip)&&
!(contents instanceof JPopupMenu)) {
popupType = MEDIUM_WEIGHT_POPUP;
}
Yes, ToolTipManager uses MEDIUM_WEIGHT as well
I'm not sure if we want to replace this as well with full HW (I would
do it, in fact).
Actually we can't use HEAVY_WEIGHT always because there are some tricky
usages. E.g. you can create a frame with some transparency. After that
HEAVY_WEIGHT popups will be opaque, but LIGHT_WEIGHT will have the same
transparency...
Also I was thinking to have a very small optimization, if we are using
an HW popups, there's no need to do this check:
// Check if the parent component is an option pane. If so we need to
// force a heavy weight popup in order to have event dispatching work
// correctly.
Component c = owner;
while (c != null) {
if (c instanceof JComponent) {
if (((JComponent)c).getClientProperty(
PopupFactory_FORCE_HEAVYWEIGHT_POPUP) ==
Boolean.TRUE) {
popupType = HEAVY_WEIGHT_POPUP;
break;
}
} else if (c instanceof Window) {
Window w = (Window) c;
if (!w.isOpaque() || w.getOpacity()< 1 ||
w.getShape() != null) {
popupType = HEAVY_WEIGHT_POPUP;
break;
}
}
c = c.getParent();
}
We can save a loop and a couple of instanceof checks (although I
remember we proved with Jim and Phil that instanceof don't really
introduce any speed penalization).
What do you think?
I didn't catch details of your optimization. Note that we are not going
to remove lightweight popups because of the described above transparency
example. I see another optimization: because of the
PopupFactory#getPopupType only "increases" weight we can replace all
"popupType = HEAVY_WEIGHT_POPUP" by "return HEAVY_WEIGHT_POPUP". But I'd
preffer don't touch code that doesn't relative to the fix
About the test:
1. Now is 2012 :)
2. You must access to Swing components only from the EDT (see
clickOnComponent(final Component comp) and other methods)
3. Don't use long lines please
4. Could you please use simplest constractions when possible. E.g.
a.
frame.setMinimumSize(new Dimension(500, 500));
frame.pack();
can be replaced by
frame.setSize(...)
b.
loop
final Map<String, Boolean> tests = new HashMap<>();
tests.put("javax.swing.PopupFactory$HeavyWeightPopup", false);
tests.put("javax.swing.PopupFactory$LightWeightPopup", true);
for (final String test : tests.keySet()) {
can be replaced by two simple invocations
c. NoSuchFieldException, SecurityException, IllegalArgumentException,
IllegalAccessException can be replaced by Exception
d.
robot.delay(50);
robot.mousePress(InputEvent.BUTTON1_MASK);
robot.delay(50);
Just use Robot#setAutoDelay
etc.
5. latch must be volitile. After test rewriting I think this variable
can be removed at all
Note that tests should be readable and simplest as far as possible
Regards, Pavel
