From a43533117cd122dd7245f44ce764335479fb9e68 Mon Sep 17 00:00:00 2001
From: Raphael Moll <>
Date: Thu, 16 Apr 2009 15:59:55 -0700
Subject: [PATCH] AI 146631: ADT #1793333: fix Widget disposed in
SdkTargetSelector. This happens when you open the Windows > Prefs > Android
panel while an SDK is initially loading or when you change the SDK in the
pref panel. The target change listener was not properly removed since the
field was not properly disposed. This also removed the multiple selection
handling in the SdkTargetSelector, which we never use. In the unlikely
event we want to use it later, it would be trivial to add it back.
BUG=1793333
Automated import of CL 146631
---
.../preferences/AndroidPreferencePage.java | 21 ++++-
.../properties/AndroidPropertyPage.java | 6 +-
.../newproject/NewProjectCreationPage.java | 4 +-
.../android/sdkuilib/SdkTargetSelector.java | 88 ++++++++-----------
4 files changed, 58 insertions(+), 61 deletions(-)
diff --git a/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/preferences/AndroidPreferencePage.java b/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/preferences/AndroidPreferencePage.java
index 458f78e49..90ad13a1c 100644
--- a/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/preferences/AndroidPreferencePage.java
+++ b/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/preferences/AndroidPreferencePage.java
@@ -50,6 +50,8 @@ import java.io.File;
public class AndroidPreferencePage extends FieldEditorPreferencePage implements
IWorkbenchPreferencePage {
+ private SdkDirectoryFieldEditor mDirectoryField;
+
public AndroidPreferencePage() {
super(GRID);
setPreferenceStore(AdtPlugin.getDefault().getPreferenceStore());
@@ -64,8 +66,10 @@ public class AndroidPreferencePage extends FieldEditorPreferencePage implements
@Override
public void createFieldEditors() {
- addField(new SdkDirectoryFieldEditor(AdtPlugin.PREFS_SDK_DIR,
- Messages.AndroidPreferencePage_SDK_Location_, getFieldEditorParent()));
+ mDirectoryField = new SdkDirectoryFieldEditor(AdtPlugin.PREFS_SDK_DIR,
+ Messages.AndroidPreferencePage_SDK_Location_, getFieldEditorParent());
+
+ addField(mDirectoryField);
}
/*
@@ -75,6 +79,16 @@ public class AndroidPreferencePage extends FieldEditorPreferencePage implements
*/
public void init(IWorkbench workbench) {
}
+
+ @Override
+ public void dispose() {
+ super.dispose();
+
+ if (mDirectoryField != null) {
+ mDirectoryField.dispose();
+ mDirectoryField = null;
+ }
+ }
/**
* Custom version of DirectoryFieldEditor which validates that the directory really
@@ -164,8 +178,7 @@ public class AndroidPreferencePage extends FieldEditorPreferencePage implements
mTargetSelector = new SdkTargetSelector(parent,
targets,
- false, /*allowSelection*/
- false /*multipleSelection*/);
+ false /*allowSelection*/);
gd = (GridData) mTargetSelector.getLayoutData();
gd.horizontalSpan = numColumns;
diff --git a/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/project/properties/AndroidPropertyPage.java b/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/project/properties/AndroidPropertyPage.java
index a4c019f55..9ab1154c9 100644
--- a/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/project/properties/AndroidPropertyPage.java
+++ b/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/project/properties/AndroidPropertyPage.java
@@ -70,7 +70,7 @@ public class AndroidPropertyPage extends PropertyPage implements IWorkbenchPrope
Label l = new Label(top, SWT.NONE);
l.setText("Project Target");
- mSelector = new SdkTargetSelector(top, targets, false /*allowMultipleSelection*/);
+ mSelector = new SdkTargetSelector(top, targets);
l = new Label(top, SWT.SEPARATOR | SWT.HORIZONTAL);
l.setLayoutData(new GridData(GridData.FILL_HORIZONTAL));
@@ -98,7 +98,7 @@ public class AndroidPropertyPage extends PropertyPage implements IWorkbenchPrope
@Override
public void widgetSelected(SelectionEvent e) {
// look for the selection and validate the page if there is a selection
- IAndroidTarget target = mSelector.getFirstSelected();
+ IAndroidTarget target = mSelector.getSelected();
setValid(target != null);
}
});
@@ -114,7 +114,7 @@ public class AndroidPropertyPage extends PropertyPage implements IWorkbenchPrope
public boolean performOk() {
Sdk currentSdk = Sdk.getCurrent();
if (currentSdk != null) {
- currentSdk.setProject(mProject, mSelector.getFirstSelected(),
+ currentSdk.setProject(mProject, mSelector.getSelected(),
mApkConfigWidget.getApkConfigs());
}
diff --git a/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/wizards/newproject/NewProjectCreationPage.java b/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/wizards/newproject/NewProjectCreationPage.java
index 61b15346e..e62f82d21 100644
--- a/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/wizards/newproject/NewProjectCreationPage.java
+++ b/tools/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/wizards/newproject/NewProjectCreationPage.java
@@ -219,7 +219,7 @@ public class NewProjectCreationPage extends WizardPage {
/** Returns the current sdk target or null if none has been selected yet. */
public IAndroidTarget getSdkTarget() {
- return mSdkTargetSelector == null ? null : mSdkTargetSelector.getFirstSelected();
+ return mSdkTargetSelector == null ? null : mSdkTargetSelector.getSelected();
}
/**
@@ -405,7 +405,7 @@ public class NewProjectCreationPage extends WizardPage {
group.setText("Target");
// The selector is created without targets. They are added below in the change listener.
- mSdkTargetSelector = new SdkTargetSelector(group, null, false /*multi-selection*/);
+ mSdkTargetSelector = new SdkTargetSelector(group, null);
mSdkTargetChangeListener = new ITargetChangeListener() {
public void onProjectTargetChange(IProject changedProject) {
diff --git a/tools/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/SdkTargetSelector.java b/tools/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/SdkTargetSelector.java
index 5f9e9c23a..37d69b8ba 100644
--- a/tools/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/SdkTargetSelector.java
+++ b/tools/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/SdkTargetSelector.java
@@ -43,14 +43,13 @@ import java.util.ArrayList;
*
* To use, create it using {@link #SdkTargetSelector(Composite, IAndroidTarget[], boolean)} then
* call {@link #setSelection(IAndroidTarget)}, {@link #setSelectionListener(SelectionListener)}
- * and finally use {@link #getFirstSelected()} or {@link #getAllSelected()} to retrieve the
+ * and finally use {@link #getSelected()} to retrieve the
* selection.
*/
public class SdkTargetSelector {
private IAndroidTarget[] mTargets;
private final boolean mAllowSelection;
- private final boolean mAllowMultipleSelection;
private SelectionListener mSelectionListener;
private Table mTable;
private Label mDescription;
@@ -62,12 +61,9 @@ public class SdkTargetSelector {
* @param parent The parent composite where the selector will be added.
* @param targets The list of targets. This is not copied, the caller must not modify.
* Targets can be null or an empty array, in which case the table is disabled.
- * @param allowMultipleSelection True if more than one SDK target can be selected at the same
- * time.
*/
- public SdkTargetSelector(Composite parent, IAndroidTarget[] targets,
- boolean allowMultipleSelection) {
- this(parent, targets, true /*allowSelection*/, allowMultipleSelection);
+ public SdkTargetSelector(Composite parent, IAndroidTarget[] targets) {
+ this(parent, targets, true /*allowSelection*/);
}
/**
@@ -77,12 +73,8 @@ public class SdkTargetSelector {
* @param targets The list of targets. This is not copied, the caller must not modify.
* Targets can be null or an empty array, in which case the table is disabled.
* @param allowSelection True if selection is enabled.
- * @param allowMultipleSelection True if more than one SDK target can be selected at the same
- * time. Used only if allowSelection is true.
*/
- public SdkTargetSelector(Composite parent, IAndroidTarget[] targets,
- boolean allowSelection,
- boolean allowMultipleSelection) {
+ public SdkTargetSelector(Composite parent, IAndroidTarget[] targets, boolean allowSelection) {
// Layout has 1 column
mInnerGroup = new Composite(parent, SWT.NONE);
mInnerGroup.setLayout(new GridLayout());
@@ -90,13 +82,9 @@ public class SdkTargetSelector {
mInnerGroup.setFont(parent.getFont());
mAllowSelection = allowSelection;
- mAllowMultipleSelection = allowMultipleSelection;
- int style = SWT.BORDER;
+ int style = SWT.BORDER | SWT.SINGLE | SWT.FULL_SELECTION;
if (allowSelection) {
- style |= SWT.CHECK | SWT.FULL_SELECTION;
- }
- if (!mAllowMultipleSelection) {
- style |= SWT.SINGLE;
+ style |= SWT.CHECK;
}
mTable = new Table(mInnerGroup, style);
mTable.setHeaderVisible(true);
@@ -127,7 +115,7 @@ public class SdkTargetSelector {
setTargets(targets);
setupTooltip(mTable);
}
-
+
/**
* Returns the layout data of the inner composite widget that contains the target selector.
* By default the layout data is set to a {@link GridData} with a {@link GridData#FILL_BOTH}
@@ -166,8 +154,7 @@ public class SdkTargetSelector {
* The event's item contains a {@link TableItem}.
* The {@link TableItem#getData()} contains an {@link IAndroidTarget}.
*
- * It is recommended that the caller uses the {@link #getFirstSelected()} and
- * {@link #getAllSelected()} methods instead.
+ * It is recommended that the caller uses the {@link #getSelected()} method instead.
*
* @param selectionListener The new listener or null to remove it.
*/
@@ -188,19 +175,22 @@ public class SdkTargetSelector {
if (!mAllowSelection) {
return false;
}
-
+
boolean found = false;
boolean modified = false;
- for (TableItem i : mTable.getItems()) {
- if ((IAndroidTarget) i.getData() == target) {
- found = true;
- if (!i.getChecked()) {
+
+ if (mTable != null && !mTable.isDisposed()) {
+ for (TableItem i : mTable.getItems()) {
+ if ((IAndroidTarget) i.getData() == target) {
+ found = true;
+ if (!i.getChecked()) {
+ modified = true;
+ i.setChecked(true);
+ }
+ } else if (i.getChecked()) {
modified = true;
- i.setChecked(true);
+ i.setChecked(false);
}
- } else if (i.getChecked()) {
- modified = true;
- i.setChecked(false);
}
}
@@ -212,30 +202,15 @@ public class SdkTargetSelector {
}
/**
- * Returns all selected items.
- * This is useful when the table is in multiple-selection mode.
+ * Returns the selected item.
*
- * @see #getFirstSelected()
- * @return An array of selected items. The list can be empty but not null.
+ * @return The selected item or null.
*/
- public IAndroidTarget[] getAllSelected() {
- ArrayList list = new ArrayList();
- for (TableItem i : mTable.getItems()) {
- if (i.getChecked()) {
- list.add((IAndroidTarget) i.getData());
- }
+ public IAndroidTarget getSelected() {
+ if (mTable == null || mTable.isDisposed()) {
+ return null;
}
- return list.toArray(new IAndroidTarget[list.size()]);
- }
- /**
- * Returns the first selected item.
- * This is useful when the table is in single-selection mode.
- *
- * @see #getAllSelected()
- * @return The first selected item or null.
- */
- public IAndroidTarget getFirstSelected() {
for (TableItem i : mTable.getItems()) {
if (i.getChecked()) {
return (IAndroidTarget) i.getData();
@@ -311,7 +286,7 @@ public class SdkTargetSelector {
* items when this one is selected.
*/
private void enforceSingleSelection(TableItem item) {
- if (!mAllowMultipleSelection && item.getChecked()) {
+ if (item.getChecked()) {
Table parentTable = item.getParent();
for (TableItem i2 : parentTable.getItems()) {
if (i2 != item && i2.getChecked()) {
@@ -335,7 +310,11 @@ public class SdkTargetSelector {
*
*/
private void fillTable(final Table table) {
-
+
+ if (table == null || table.isDisposed()) {
+ return;
+ }
+
table.removeAll();
if (mTargets != null && mTargets.length > 0) {
@@ -366,6 +345,11 @@ public class SdkTargetSelector {
* display the description in a label under the table.
*/
private void setupTooltip(final Table table) {
+
+ if (table == null || table.isDisposed()) {
+ return;
+ }
+
/*
* Reference:
* http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet125.java?view=markup