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

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/preferences/AndroidPreferencePage.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/preferences/AndroidPreferencePage.java
index 458f78e..90ad13a 100644 (file)
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/preferences/AndroidPreferencePage.java
+++ b/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/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/project/properties/AndroidPropertyPage.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/project/properties/AndroidPropertyPage.java
index a4c019f..9ab1154 100644 (file)
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/project/properties/AndroidPropertyPage.java
+++ b/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/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/wizards/newproject/NewProjectCreationPage.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/wizards/newproject/NewProjectCreationPage.java
index 61b1534..e62f82d 100644 (file)
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/wizards/newproject/NewProjectCreationPage.java
+++ b/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/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/SdkTargetSelector.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/SdkTargetSelector.java
index 5f9e9c2..37d69b8 100644 (file)
--- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/SdkTargetSelector.java
+++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/SdkTargetSelector.java
@@ -43,14 +43,13 @@ import java.util.ArrayList;
  * <p/>
  * 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 <em>not</em> 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 <em>not</em> 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}.
      * <p/>
-     * 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<IAndroidTarget> list = new ArrayList<IAndroidTarget>();
-        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 {
      * </ul>
      */
     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