Skip to content

Commit 4997409

Browse files
ptzieglerptziegler
committed
Encapsulate the Swing SpinnerModel using an ObservableValue
The SpinnerModel shown in the SpinnerModelDialog is currently updated in the SWT UI thread. On MacOS, this may lead to a deadlock, in case the SWT UI thread is waiting for the AWT event dispatch thread. Access to this model is now guarded by an ObservableValue, which requires proper synchronization. The dialog has been restructured to make sure modifications to this value are done exclusively in the event dispatch thread and that the dialog is only updated in the SWT UI thread. May close #1204
1 parent 601dd17 commit 4997409

9 files changed

Lines changed: 263 additions & 114 deletions

File tree

‎org.eclipse.wb.swing/src/org/eclipse/wb/internal/swing/model/property/editor/models/spinner/AbstractSpinnerComposite.java‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,10 @@
1717
import org.eclipse.swt.widgets.Composite;
1818
import org.eclipse.swt.widgets.Display;
1919

20-
import javax.swing.JSpinner;
21-
import javax.swing.SpinnerModel;
20+
import java.util.function.Supplier;
2221

2322
/**
24-
* Abstract editor for some {@link SpinnerModel} type.
23+
* Abstract editor for some {@link SpinnerModelValue} type.
2524
*
2625
* @author scheglov_ke
2726
* @coverage swing.property.editor
@@ -52,22 +51,29 @@ public AbstractSpinnerComposite(Composite parent, SpinnerModelDialog modelDialog
5251
public abstract String getTitle();
5352

5453
/**
55-
* Sets the {@link SpinnerModel} to display/edit.
54+
* Sets the {@link SpinnerModelValue} to display/edit. <b>Important:</b> This
55+
* method is called from the AWT event dispatch thread.
5656
*
57-
* @return <code>true</code> if this {@link AbstractSpinnerComposite} understands given model.
57+
* @return <code>true</code> if this {@link AbstractSpinnerComposite}
58+
* understands given model.
5859
*/
59-
public abstract boolean setModel(SpinnerModel model);
60+
public abstract boolean setModelValue(SpinnerModelValue modelValue);
6061

6162
/**
6263
* @return the error message, or <code>null</code> if model configured correctly.
6364
*/
6465
public abstract String validate();
6566

6667
/**
67-
* @return the {@link SpinnerModel} that corresponds to this {@link AbstractSpinnerComposite} and
68-
* configuration. This {@link SpinnerModel} is used later for preview in {@link JSpinner}.
68+
* This method is called from the SWT UI thread. But the returned
69+
* {@link Supplier} must be called from the AWT event dispatch thread.
70+
*
71+
* @return the {@link SpinnerModelValue} that corresponds to this
72+
* {@link AbstractSpinnerComposite} and configuration. This
73+
* {@link SpinnerModelValue} is used later for preview in
74+
* {@link javax.swing.JSpinner}.
6975
*/
70-
public abstract SpinnerModel getModel();
76+
public abstract Supplier<SpinnerModelValue> getModelValue();
7177

7278
/**
7379
* @return the source to apply.

‎org.eclipse.wb.swing/src/org/eclipse/wb/internal/swing/model/property/editor/models/spinner/DateSpinnerComposite.java‎

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
*******************************************************************************/
1313
package org.eclipse.wb.internal.swing.model.property.editor.models.spinner;
1414

15-
import org.eclipse.wb.core.controls.CSpinner;
1615
import org.eclipse.wb.internal.core.utils.reflect.ReflectionUtils;
1716
import org.eclipse.wb.internal.core.utils.ui.GridDataFactory;
1817
import org.eclipse.wb.internal.core.utils.ui.GridLayoutFactory;
@@ -33,12 +32,11 @@
3332

3433
import java.util.Calendar;
3534
import java.util.Date;
36-
37-
import javax.swing.SpinnerDateModel;
38-
import javax.swing.SpinnerModel;
35+
import java.util.function.Supplier;
3936

4037
/**
41-
* Implementation of {@link AbstractSpinnerComposite} for {@link SpinnerDateModel}.
38+
* Implementation of {@link AbstractSpinnerComposite} for
39+
* {@link javax.swing.SpinnerDateModel SpinnerDateModel}.
4240
*
4341
* @author scheglov_ke
4442
* @coverage swing.property.editor
@@ -199,33 +197,43 @@ public String getTitle() {
199197
}
200198

201199
@Override
202-
public boolean setModel(SpinnerModel model) {
203-
if (model instanceof SpinnerDateModel dateModel) {
204-
// values
205-
setValue(m_valueField, dateModel.getValue());
206-
setValue(m_minField, dateModel.getStart());
207-
setValue(m_maxField, dateModel.getEnd());
208-
// step
209-
for (int i = 0; i < CALENDAR_FIELDS.length; i++) {
210-
String field = CALENDAR_FIELDS[i];
211-
if (ReflectionUtils.getFieldInt(Calendar.class, field) == dateModel.getCalendarField()) {
212-
m_stepCombo.select(i);
213-
break;
200+
public boolean setModelValue(SpinnerModelValue modelValue) {
201+
if (modelValue.getValue() instanceof javax.swing.SpinnerDateModel dateModel) {
202+
Object value = dateModel.getValue();
203+
Comparable<Date> start = dateModel.getStart();
204+
Comparable<Date> end = dateModel.getEnd();
205+
int calendarField = dateModel.getCalendarField();
206+
getDisplay().asyncExec(() -> {
207+
// values
208+
setValue(m_valueField, value);
209+
setValue(m_minField, start);
210+
setValue(m_maxField, end);
211+
// step
212+
for (int i = 0; i < CALENDAR_FIELDS.length; i++) {
213+
String field = CALENDAR_FIELDS[i];
214+
if (ReflectionUtils.getFieldInt(Calendar.class, field) == calendarField) {
215+
m_stepCombo.select(i);
216+
break;
217+
}
214218
}
215-
}
216-
// enable/disable min/max fields
217-
updateCheckField(m_minButton, m_minField, dateModel.getStart() != null);
218-
updateCheckField(m_maxButton, m_maxField, dateModel.getEnd() != null);
219+
// enable/disable min/max fields
220+
updateCheckField(m_minButton, m_minField, start != null);
221+
updateCheckField(m_maxButton, m_maxField, end != null);
222+
m_modelDialog.validateAll();
223+
});
219224
// OK, this is our model
220225
return true;
221226
} else {
222-
// values
223-
m_valueField.setSelection(DEFAULT_DATE);
224-
m_minField.setSelection(DEFAULT_DATE);
225-
m_maxField.setSelection(DEFAULT_DATE);
226-
// disable min/max fields
227-
updateCheckField(m_minButton, m_minField, false);
228-
updateCheckField(m_maxButton, m_maxField, false);
227+
getDisplay().asyncExec(() -> {
228+
// values
229+
m_valueField.setSelection(DEFAULT_DATE);
230+
m_minField.setSelection(DEFAULT_DATE);
231+
m_maxField.setSelection(DEFAULT_DATE);
232+
// disable min/max fields
233+
updateCheckField(m_minButton, m_minField, false);
234+
updateCheckField(m_maxButton, m_maxField, false);
235+
m_modelDialog.validateAll();
236+
});
229237
// no, we don't know this model
230238
return false;
231239
}
@@ -251,13 +259,13 @@ public String validate() {
251259
}
252260

253261
@Override
254-
public SpinnerModel getModel() {
262+
public Supplier<SpinnerModelValue> getModelValue() {
255263
Date value = m_valueField.getSelection();
256264
Date minimum = m_minButton.getSelection() ? m_minField.getSelection() : null;
257265
Date maximum = m_maxButton.getSelection() ? m_maxField.getSelection() : null;
258266
String stepField = CALENDAR_FIELDS[m_stepCombo.getSelectionIndex()];
259267
int step = ReflectionUtils.getFieldInt(Calendar.class, stepField);
260-
return new SpinnerDateModel(value, minimum, maximum, step);
268+
return () -> new SpinnerModelValue(new javax.swing.SpinnerDateModel(value, minimum, maximum, step));
261269
}
262270

263271
@Override
@@ -298,7 +306,7 @@ private static void setValue(CDateTime field, Object value) {
298306
}
299307

300308
/**
301-
* @return the source for value from {@link CSpinner}.
309+
* @return the source for value from {@link CDateTime}.
302310
*/
303311
private String getValueSource(CDateTime field) {
304312
long milliseconds;

‎org.eclipse.wb.swing/src/org/eclipse/wb/internal/swing/model/property/editor/models/spinner/ListSpinnerComposite.java‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@
2626

2727
import org.apache.commons.lang3.StringUtils;
2828

29-
import javax.swing.SpinnerListModel;
30-
import javax.swing.SpinnerModel;
29+
import java.util.function.Supplier;
3130

3231
/**
33-
* Implementation of {@link AbstractSpinnerComposite} for {@link SpinnerListModel}.
32+
* Implementation of {@link AbstractSpinnerComposite} for
33+
* {@link javax.swing.SpinnerListModel SpinnerListModel}.
3434
*
3535
* @author scheglov_ke
3636
* @coverage swing.property.editor
@@ -75,10 +75,13 @@ public String getTitle() {
7575
}
7676

7777
@Override
78-
public boolean setModel(SpinnerModel model) {
79-
if (model instanceof SpinnerListModel listModel) {
78+
public boolean setModelValue(SpinnerModelValue modelValue) {
79+
if (modelValue.getValue() instanceof javax.swing.SpinnerListModel listModel) {
8080
String text = StringUtils.join(listModel.getList().iterator(), "\n");
81-
m_textWidget.setText(text);
81+
getDisplay().asyncExec(() -> {
82+
m_textWidget.setText(text);
83+
m_modelDialog.validateAll();
84+
});
8285
// OK, this is our model
8386
return true;
8487
}
@@ -95,9 +98,9 @@ public String validate() {
9598
}
9699

97100
@Override
98-
public SpinnerModel getModel() {
101+
public Supplier<SpinnerModelValue> getModelValue() {
99102
String[] items = getItems();
100-
return new SpinnerListModel(items);
103+
return () -> new SpinnerModelValue(new javax.swing.SpinnerListModel(items));
101104
}
102105

103106
@Override

‎org.eclipse.wb.swing/src/org/eclipse/wb/internal/swing/model/property/editor/models/spinner/NumberSpinnerComposite.java‎

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@
2727
import org.eclipse.swt.widgets.Listener;
2828
import org.eclipse.swt.widgets.Spinner;
2929

30-
import javax.swing.SpinnerModel;
31-
import javax.swing.SpinnerNumberModel;
30+
import java.util.function.Supplier;
3231

3332
/**
34-
* Implementation of {@link AbstractSpinnerComposite} for {@link SpinnerNumberModel}.
33+
* Implementation of {@link AbstractSpinnerComposite} for
34+
* {@link javax.swing.SpinnerNumberModel SpinnerNumberModel}.
3535
*
3636
* @author scheglov_ke
3737
* @coverage swing.property.editor
@@ -170,30 +170,40 @@ public String getTitle() {
170170
}
171171

172172
@Override
173-
public boolean setModel(SpinnerModel model) {
174-
if (model instanceof SpinnerNumberModel numberModel) {
175-
// type
176-
NumberTypeDescription[] values = NumberTypeDescription.values();
177-
for (int i = 0; i < values.length; i++) {
178-
NumberTypeDescription typeDescription = values[i];
179-
if (typeDescription.getType() == numberModel.getValue().getClass()) {
180-
m_typeCombo.select(i);
173+
public boolean setModelValue(SpinnerModelValue modelValue) {
174+
if (modelValue.getValue() instanceof javax.swing.SpinnerNumberModel numberModel) {
175+
Object value = numberModel.getValue();
176+
Comparable<?> minimum = numberModel.getMinimum();
177+
Comparable<?> maximum = numberModel.getMaximum();
178+
Number stepSize = numberModel.getStepSize();
179+
getDisplay().asyncExec(() -> {
180+
// type
181+
NumberTypeDescription[] values = NumberTypeDescription.values();
182+
for (int i = 0; i < values.length; i++) {
183+
NumberTypeDescription typeDescription = values[i];
184+
if (typeDescription.getType() == value.getClass()) {
185+
m_typeCombo.select(i);
186+
}
181187
}
182-
}
183-
// values
184-
setValue(m_valueField, numberModel.getValue());
185-
setValue(m_minField, numberModel.getMinimum());
186-
setValue(m_maxField, numberModel.getMaximum());
187-
setValue(m_stepField, numberModel.getStepSize());
188-
// enable/disable min/max fields
189-
updateCheckSpinner(m_minButton, m_minField, numberModel.getMinimum() != null);
190-
updateCheckSpinner(m_maxButton, m_maxField, numberModel.getMaximum() != null);
188+
// values
189+
setValue(m_valueField, value);
190+
setValue(m_minField, minimum);
191+
setValue(m_maxField, maximum);
192+
setValue(m_stepField, stepSize);
193+
// enable/disable min/max fields
194+
updateCheckSpinner(m_minButton, m_minField, minimum != null);
195+
updateCheckSpinner(m_maxButton, m_maxField, maximum != null);
196+
m_modelDialog.validateAll();
197+
});
191198
// OK, this is our model
192199
return true;
193200
} else {
194201
// disable min/max fields
195-
updateCheckSpinner(m_minButton, m_minField, false);
196-
updateCheckSpinner(m_maxButton, m_maxField, false);
202+
getDisplay().asyncExec(() -> {
203+
updateCheckSpinner(m_minButton, m_minField, false);
204+
updateCheckSpinner(m_maxButton, m_maxField, false);
205+
m_modelDialog.validateAll();
206+
});
197207
return false;
198208
}
199209
}
@@ -219,12 +229,12 @@ public String validate() {
219229
}
220230

221231
@Override
222-
public SpinnerModel getModel() {
232+
public Supplier<SpinnerModelValue> getModelValue() {
223233
int value = m_valueField.getSelection();
224234
Integer minimum = m_minButton.getSelection() ? m_minField.getSelection() : null;
225235
Integer maximum = m_maxButton.getSelection() ? m_maxField.getSelection() : null;
226236
Number step = m_stepField.getSelection();
227-
return new SpinnerNumberModel(value, minimum, maximum, step);
237+
return () -> new SpinnerModelValue(new javax.swing.SpinnerNumberModel(value, minimum, maximum, step));
228238
}
229239

230240
@Override
@@ -278,7 +288,7 @@ private String getValueSource(Spinner spinner) {
278288
//
279289
////////////////////////////////////////////////////////////////////////////
280290
/**
281-
* Description for {@link Number} in {@link SpinnerNumberModel}.
291+
* Description for {@link Number} in {@link javax.swing.SpinnerNumberModel}.
282292
*/
283293
private static enum NumberTypeDescription {
284294
BYTE(Byte.class) {

‎org.eclipse.wb.swing/src/org/eclipse/wb/internal/swing/model/property/editor/models/spinner/SpinnerModelCanvas.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2011, 2025 Google, Inc. and others.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License 2.0 which is available at
6+
* https://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* SPDX-License-Identifier: EPL-2.0
9+
*
10+
* Contributors:
11+
* Google, Inc. - initial API and implementation
12+
*******************************************************************************/
13+
package org.eclipse.wb.internal.swing.model.property.editor.models.spinner;
14+
15+
import org.eclipse.wb.internal.swing.utils.SwingUtils;
16+
17+
import org.eclipse.swt.widgets.Composite;
18+
19+
import java.util.function.Supplier;
20+
21+
import javax.swing.JComponent;
22+
import javax.swing.JSpinner;
23+
import javax.swing.SpinnerModel;
24+
25+
import swingintegration.example.EmbeddedSwingComposite;
26+
27+
/**
28+
* Preview of the currently selected SpinnerModel in the SpinnerModel dialog..
29+
*/
30+
public final class SpinnerModelCanvas extends EmbeddedSwingComposite {
31+
private JSpinner m_spinner;
32+
33+
public SpinnerModelCanvas(Composite parent, int style) {
34+
super(parent, style);
35+
}
36+
37+
@Override
38+
protected JComponent createSwingComponent() {
39+
m_spinner = new JSpinner();
40+
return m_spinner;
41+
}
42+
43+
/* package */ void disable() {
44+
SwingUtils.runLogLater(() -> {
45+
if (m_spinner != null) {
46+
m_spinner.setEnabled(false);
47+
}
48+
});
49+
}
50+
51+
/* package */ void setSpinnerModel(Supplier<SpinnerModelValue> supplier) {
52+
SwingUtils.runLogLater(() -> {
53+
if (m_spinner != null) {
54+
SpinnerModelValue modelValue = supplier.get();
55+
SpinnerModel model = modelValue.getValue();
56+
m_spinner.setModel(model);
57+
m_spinner.setEnabled(true);
58+
}
59+
});
60+
}
61+
}

0 commit comments

Comments
 (0)