OSDN Git Service

First time install: multiple platforms, fix shell disposed.
authorRaphael <raphael@google.com>
Sat, 24 Sep 2011 04:27:14 +0000 (21:27 -0700)
committerRaphael <raphael@google.com>
Sat, 24 Sep 2011 04:32:03 +0000 (21:32 -0700)
Fixes for the first time install support:

- Support installing more than one platform.
  The custom install task was tailored before to one
  install a single package and return one status with
  on archive install path. This has been made more
  generic.

- Fix a couple occurences of the dreaded "SWT Shell disposed"
  error that happen when updating the progress bar and closing
  the window at the same time. The issue is that I was checking
  isDisposed before starting a syncExec but the window can be
  closed between the moment the syncExec is schedule and when it
  happens so code must check again for isDisposed inside the
  syncExec.

Change-Id: I413cfacd63406febc7953effad5ece2ef2854360

eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/welcome/WelcomeWizard.java
sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/UpdaterData.java
sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/AdtUpdateDialog.java
sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackageLoader.java
sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/tasks/ProgressView.java
sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/ui/SwtBaseDialog.java

index 9aa5ead..2bcc63c 100644 (file)
@@ -21,11 +21,12 @@ import com.android.ide.eclipse.adt.internal.preferences.AdtPrefs;
 import com.android.ide.eclipse.adt.internal.sdk.AdtConsoleSdkLog;
 import com.android.sdkstats.DdmsPreferenceStore;
 import com.android.sdkuilib.internal.repository.sdkman2.AdtUpdateDialog;
-import com.android.util.Pair;
 
 import org.eclipse.jface.wizard.Wizard;
 
 import java.io.File;
+import java.util.HashSet;
+import java.util.Set;
 
 /**
  * Wizard shown on first start for new users: configure SDK location, accept or
@@ -82,12 +83,14 @@ public class WelcomeWizard extends Wizard {
             public void run() {
                 if (createNew) {
                     try {
+                        Set<Integer> apiLevels = new HashSet<Integer>();
                         if (installCommon) {
-                            installSdk(path, 7);
+                            apiLevels.add(7);
                         }
                         if (installLatest) {
-                            installSdk(path, AdtUpdateDialog.USE_MAX_REMOTE_API_LEVEL);
+                            apiLevels.add(AdtUpdateDialog.USE_MAX_REMOTE_API_LEVEL);
                         }
+                        installSdk(path, apiLevels);
                     } catch (Exception e) {
                         AdtPlugin.logAndPrintError(e, "ADT Welcome Wizard", "Installation failed");
                     }
@@ -107,7 +110,7 @@ public class WelcomeWizard extends Wizard {
      * a confirmation window showing which packages are selected for install
      * and display a progress dialog during installation.
      */
-    private boolean installSdk(File path, int apiLevel) {
+    private boolean installSdk(File path, Set<Integer> apiLevels) {
         if (!path.isDirectory()) {
             if (!path.mkdirs()) {
                 AdtPlugin.logAndPrintError(null, "ADT Welcome Wizard",
@@ -123,9 +126,9 @@ public class WelcomeWizard extends Wizard {
                 path.getAbsolutePath());
         // Note: we don't have to specify tools & platform-tools since they
         // are required dependencies of any platform.
-        Pair<Boolean, File> result = updater.installNewSdk(apiLevel);
+        boolean result = updater.installNewSdk(apiLevels);
 
-        if (!result.getFirst().booleanValue()) {
+        if (!result) {
             AdtPlugin.printErrorToConsole("Failed to install Android SDK.");
             return false;
         }
index b317f71..0b494a7 100755 (executable)
@@ -55,7 +55,6 @@ import com.android.sdkuilib.internal.repository.sdkman1.SdkUpdaterWindowImpl1;
 import com.android.sdkuilib.repository.ISdkChangeListener;\r
 \r
 import org.eclipse.jface.dialogs.MessageDialog;\r
-import org.eclipse.swt.widgets.Display;\r
 import org.eclipse.swt.widgets.Shell;\r
 \r
 import java.io.ByteArrayOutputStream;\r
@@ -628,16 +627,19 @@ public class UpdaterData implements IUpdaterData {
 \r
         if (getWindowShell() != null && getSettingsController().getAskBeforeAdbRestart()) {\r
             // need to ask for permission first\r
-            Display display = getWindowShell().getDisplay();\r
-\r
-            display.syncExec(new Runnable() {\r
-                public void run() {\r
-                    canRestart[0] = MessageDialog.openQuestion(getWindowShell(),\r
-                            "ADB Restart",\r
-                            "A package that depends on ADB has been updated. \n" +\r
-                            "Do you want to restart ADB now?");\r
-                }\r
-            });\r
+            final Shell shell = getWindowShell();\r
+            if (shell != null && !shell.isDisposed()) {\r
+                shell.getDisplay().syncExec(new Runnable() {\r
+                    public void run() {\r
+                        if (!shell.isDisposed()) {\r
+                            canRestart[0] = MessageDialog.openQuestion(shell,\r
+                                    "ADB Restart",\r
+                                    "A package that depends on ADB has been updated. \n" +\r
+                                    "Do you want to restart ADB now?");\r
+                        }\r
+                    }\r
+                });\r
+            }\r
         }\r
 \r
         if (canRestart[0]) {\r
@@ -648,23 +650,23 @@ public class UpdaterData implements IUpdaterData {
     }\r
 \r
     private void notifyToolsNeedsToBeRestarted() {\r
-        if (getWindowShell() == null) {\r
-            // We don't need to print anything if this is a standalone console update.\r
-            return;\r
-        }\r
+        // We don't need to print anything if this is a standalone console update with no shell.\r
 \r
-        Display display = getWindowShell().getDisplay();\r
-\r
-        display.syncExec(new Runnable() {\r
-            public void run() {\r
-                MessageDialog.openInformation(getWindowShell(),\r
-                        "Android Tools Updated",\r
-                        "The Android SDK and AVD Manager that you are currently using has been updated. " +\r
-                        "It is recommended that you now close the manager window and re-open it. " +\r
-                        "If you started this window from Eclipse, please check if the Android " +\r
-                        "plug-in needs to be updated.");\r
-            }\r
-        });\r
+        final Shell shell = getWindowShell();\r
+        if (shell != null && !shell.isDisposed()) {\r
+            shell.getDisplay().syncExec(new Runnable() {\r
+                public void run() {\r
+                    if (!shell.isDisposed()) {\r
+                        MessageDialog.openInformation(getWindowShell(),\r
+                                "Android Tools Updated",\r
+                                "The Android SDK and AVD Manager that you are currently using has been updated. " +\r
+                                "It is recommended that you now close the manager window and re-open it. " +\r
+                                "If you started this window from Eclipse, please check if the Android " +\r
+                                "plug-in needs to be updated.");\r
+                    }\r
+                }\r
+            });\r
+        }\r
     }\r
 \r
 \r
index 924dc12..069102e 100755 (executable)
@@ -22,7 +22,9 @@ import com.android.sdklib.ISdkLog;
 import com.android.sdklib.internal.repository.ExtraPackage;\r
 import com.android.sdklib.internal.repository.Package;\r
 import com.android.sdklib.internal.repository.PlatformPackage;\r
+import com.android.sdklib.internal.repository.PlatformToolPackage;\r
 import com.android.sdklib.internal.repository.SdkSource;\r
+import com.android.sdklib.internal.repository.ToolPackage;\r
 import com.android.sdkuilib.internal.repository.SettingsController;\r
 import com.android.sdkuilib.internal.repository.UpdaterData;\r
 import com.android.sdkuilib.internal.repository.sdkman2.PackageLoader.IAutoInstallTask;\r
@@ -42,6 +44,9 @@ import org.eclipse.swt.widgets.ProgressBar;
 import org.eclipse.swt.widgets.Shell;\r
 \r
 import java.io.File;\r
+import java.util.Map;\r
+import java.util.Set;\r
+import java.util.Map.Entry;\r
 \r
 /**\r
  * This is a private implementation of UpdateWindow for ADT,\r
@@ -68,7 +73,7 @@ public class AdtUpdateDialog extends SwtBaseDialog {
     private final UpdaterData mUpdaterData;\r
 \r
     private Boolean mResultCode = Boolean.FALSE;\r
-    private File    mResultPath = null;\r
+    private Map<Package, File> mResultPaths = null;\r
     private SettingsController mSettingsController;\r
     private PackageFilter mPackageFilter;\r
     private PackageLoader mPackageMananger;\r
@@ -110,7 +115,18 @@ public class AdtUpdateDialog extends SwtBaseDialog {
     public Pair<Boolean, File> installExtraPackage(String vendor, String path) {\r
         mPackageFilter = createExtraFilter(vendor, path);\r
         open();\r
-        return Pair.of(mResultCode, mResultPath);\r
+\r
+        File installPath = null;\r
+        if (mResultPaths != null) {\r
+            for (Entry<Package, File> entry : mResultPaths.entrySet()) {\r
+                if (entry.getKey() instanceof ExtraPackage) {\r
+                    installPath = entry.getValue();\r
+                    break;\r
+                }\r
+            }\r
+        }\r
+\r
+        return Pair.of(mResultCode, installPath);\r
     }\r
 \r
     /**\r
@@ -128,30 +144,39 @@ public class AdtUpdateDialog extends SwtBaseDialog {
      *   is false, otherwise it should point to an existing valid folder.\r
      */\r
     public Pair<Boolean, File> installPlatformPackage(int apiLevel) {\r
-        mPackageFilter = createPlatformFilter(apiLevel, false /*forceRemote*/);\r
+        mPackageFilter = createPlatformFilter(apiLevel);\r
         open();\r
-        return Pair.of(mResultCode, mResultPath);\r
+\r
+        File installPath = null;\r
+        if (mResultPaths != null) {\r
+            for (Entry<Package, File> entry : mResultPaths.entrySet()) {\r
+                if (entry.getKey() instanceof PlatformPackage) {\r
+                    installPath = entry.getValue();\r
+                    break;\r
+                }\r
+            }\r
+        }\r
+\r
+        return Pair.of(mResultCode, installPath);\r
     }\r
 \r
     /**\r
      * Displays the update dialog and triggers installation of a new SDK. This works by\r
-     * requesting a remote platform package with the specified API level.\r
-     * By dependency, any missing tools or platform-tools packages will also be installed.\r
+     * requesting a remote platform package with the specified API levels as well as\r
+     * the first tools or platform-tools packages available.\r
      * <p/>\r
      * Callers must not try to reuse this dialog after this call.\r
      *\r
-     * @param apiLevel The platform API level to match.\r
+     * @param apiLevels A set of platform API levels to match.\r
      *  The special value {@link #USE_MAX_REMOTE_API_LEVEL} means to use\r
      *  the highest API level available in the repository.\r
-     * @return A boolean indicating whether the installation was successful (meaning the package\r
-     *   was either already present, or got installed or updated properly) and a {@link File}\r
-     *   with the path to the root folder of the package. The file is null when the boolean\r
-     *   is false, otherwise it should point to an existing valid folder.\r
+     * @return A boolean indicating whether the installation was successful (meaning the packages\r
+     *   were either already present, or got installed or updated properly).\r
      */\r
-    public Pair<Boolean, File> installNewSdk(int apiLevel) {\r
-        mPackageFilter = createPlatformFilter(apiLevel, true /*forceRemote*/);\r
+    public boolean installNewSdk(Set<Integer> apiLevels) {\r
+        mPackageFilter = createNewSdkFilter(apiLevels);\r
         open();\r
-        return Pair.of(mResultCode, mResultPath);\r
+        return mResultCode.booleanValue();\r
     }\r
 \r
     @Override\r
@@ -214,10 +239,10 @@ public class AdtUpdateDialog extends SwtBaseDialog {
                 return mPackageFilter.accept(pkg);\r
             }\r
 \r
-            public void setResult(Package pkg, boolean success, File installPath) {\r
+            public void setResult(boolean success, Map<Package, File> installPaths) {\r
                 // Capture the result from the installation.\r
                 mResultCode = Boolean.valueOf(success);\r
-                mResultPath = installPath;\r
+                mResultPaths = installPaths;\r
             }\r
 \r
             public void taskCompleted() {\r
@@ -288,9 +313,7 @@ public class AdtUpdateDialog extends SwtBaseDialog {
         };\r
     }\r
 \r
-    public static PackageFilter createPlatformFilter(\r
-            final int apiLevel,\r
-            final boolean forceRemote) {\r
+    public static PackageFilter createPlatformFilter(final int apiLevel) {\r
         return new PackageFilter() {\r
             int mApiLevel = apiLevel;\r
             boolean mFindMaxApi = apiLevel == USE_MAX_REMOTE_API_LEVEL;\r
@@ -298,10 +321,6 @@ public class AdtUpdateDialog extends SwtBaseDialog {
             @Override\r
             boolean accept(Package pkg) {\r
                 if (pkg instanceof PlatformPackage) {\r
-                    if (forceRemote && pkg.isLocal()) {\r
-                        // We only want remote packages, to force a fresh install.\r
-                        return false;\r
-                    }\r
                     PlatformPackage pp = (PlatformPackage) pkg;\r
                     AndroidVersion v = pp.getVersion();\r
                     return !v.isPreview() && v.getApiLevel() == mApiLevel;\r
@@ -328,6 +347,60 @@ public class AdtUpdateDialog extends SwtBaseDialog {
         };\r
     }\r
 \r
+    public static PackageFilter createNewSdkFilter(final Set<Integer> apiLevels) {\r
+        return new PackageFilter() {\r
+            int mMaxApiLevel;\r
+            boolean mFindMaxApi = apiLevels.contains(USE_MAX_REMOTE_API_LEVEL);\r
+            boolean mNeedTools = true;\r
+            boolean mNeedPlatformTools = true;\r
+\r
+            @Override\r
+            boolean accept(Package pkg) {\r
+                if (!pkg.isLocal()) {\r
+                    if (pkg instanceof PlatformPackage) {\r
+                        PlatformPackage pp = (PlatformPackage) pkg;\r
+                        AndroidVersion v = pp.getVersion();\r
+                        if (!v.isPreview()) {\r
+                            int level = v.getApiLevel();\r
+                            if ((mFindMaxApi && level == mMaxApiLevel) ||\r
+                                    (level > 0 && apiLevels.contains(level))) {\r
+                                return true;\r
+                            }\r
+                        }\r
+                    } else if (mNeedTools && pkg instanceof ToolPackage) {\r
+                        // We want a tool package. There should be only one,\r
+                        // but in case of error just take the first one.\r
+                        mNeedTools = false;\r
+                        return true;\r
+                    } else if (mNeedPlatformTools && pkg instanceof PlatformToolPackage) {\r
+                        // We want a platform-tool package. There should be only one,\r
+                        // but in case of error just take the first one.\r
+                        mNeedPlatformTools = false;\r
+                        return true;\r
+                    }\r
+                }\r
+                return false;\r
+            }\r
+\r
+            @Override\r
+            void visit(Package pkg) {\r
+                // Try to find the max API in all remote packages\r
+                if (mFindMaxApi &&\r
+                        pkg instanceof PlatformPackage &&\r
+                        !pkg.isLocal()) {\r
+                    PlatformPackage pp = (PlatformPackage) pkg;\r
+                    AndroidVersion v = pp.getVersion();\r
+                    if (!v.isPreview()) {\r
+                        int api = v.getApiLevel();\r
+                        if (api > mMaxApiLevel) {\r
+                            mMaxApiLevel = api;\r
+                        }\r
+                    }\r
+                }\r
+            }\r
+        };\r
+    }\r
+\r
 \r
 \r
     // End of hiding from SWT Designer\r
index 1ed44bc..f296cb0 100755 (executable)
@@ -30,7 +30,9 @@ import org.eclipse.swt.widgets.Shell;
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Loads packages fetched from the remote SDK Repository and keeps track
@@ -99,8 +101,7 @@ class PackageLoader {
         /**
          * Called by the install task for every package available (new ones, updates as well
          * as existing ones that don't have a potential update.)
-         * The method should return true if this is the package that should be installed,
-         * at which point the packager loader will stop processing the next packages and sources.
+         * The method should return true if this is a package that should be installed.
          * <p/>
          * <em>Important</em>: This method is called from a sub-thread, so clients who try
          * to access any UI widgets must wrap their calls into {@link Display#syncExec(Runnable)}
@@ -111,13 +112,13 @@ class PackageLoader {
         /**
          * Called when the accepted package has been installed, successfully or not.
          * If an already installed (aka existing) package has been accepted, this will
-         * be called with a 'true' success and the actual install path.
+         * be called with a 'true' success and the actual install paths.
          * <p/>
          * <em>Important</em>: This method is called from a sub-thread, so clients who try
          * to access any UI widgets must wrap their calls into {@link Display#syncExec(Runnable)}
          * or {@link Display#asyncExec(Runnable)}.
          */
-        public void setResult(Package pkg, boolean success, File installPath);
+        public void setResult(boolean success, Map<Package, File> installPaths);
 
         /**
          * Called when the task is done iterating and completed.
@@ -231,6 +232,9 @@ class PackageLoader {
     public void loadPackagesWithInstallTask(final IAutoInstallTask installTask) {
 
         loadPackages(new ISourceLoadedCallback() {
+            List<Archive> mArchivesToInstall = new ArrayList<Archive>();
+            Map<Package, File> mInstallPaths = new HashMap<Package, File>();
+
             public boolean onUpdateSource(SdkSource source, Package[] packages) {
                 packages = installTask.filterLoadedSource(source, packages);
                 if (packages == null || packages.length == 0) {
@@ -247,30 +251,20 @@ class PackageLoader {
                             Archive[] a = pkg.getArchives();
                             // an installed package should have one local compatible archive
                             if (a.length == 1 && a[0].isCompatible()) {
-                                installTask.setResult(
-                                        pkg,
-                                        true /*success*/,
-                                        new File(a[0].getLocalOsPath()));
+                                mInstallPaths.put(pkg, new File(a[0].getLocalOsPath()));
                             }
-                            // return false to tell loadPackages() that we don't
-                            // need to continue processing any more sources.
-                            return false;
                         }
 
                     } else {
                         // This is a remote package
                         if (installTask.acceptPackage(pkg)) {
-                            // The caller is accepting this remote package. Let's try to install it.
-
+                            // The caller is accepting this remote package. We'll install it.
                             for (Archive archive : pkg.getArchives()) {
                                 if (archive.isCompatible()) {
-                                    installArchive(archive);
+                                    mArchivesToInstall.add(archive);
                                     break;
                                 }
                             }
-                            // return false to tell loadPackages() that we don't
-                            // need to continue processing any more sources.
-                            return false;
                         }
                     }
                 }
@@ -279,70 +273,75 @@ class PackageLoader {
                 return true;
             }
 
+            public void onLoadCompleted() {
+                if (!mArchivesToInstall.isEmpty()) {
+                    installArchives(mArchivesToInstall);
+                }
+                if (mInstallPaths == null) {
+                    installTask.setResult(false, null);
+                } else {
+                    installTask.setResult(true, mInstallPaths);
+                }
+
+                installTask.taskCompleted();
+            }
+
             /**
              * Shows the UI of the install selector.
              * If the package is then actually installed, refresh the local list and
              * notify the install task of the installation path.
              *
-             * @param archiveToInstall The archive to install.
+             * @param archivesToInstall The archives to install.
              */
-            private void installArchive(Archive archiveToInstall) {
-                // What we want to install
-                final ArrayList<Archive> archivesToInstall = new ArrayList<Archive>();
-                archivesToInstall.add(archiveToInstall);
-
-                Package packageToInstall = archiveToInstall.getParentPackage();
+            private void installArchives(final List<Archive> archivesToInstall) {
+                // Actually install the new archives that we just found.
+                // This will display some UI so we need a shell's sync exec.
 
-                // What we'll end up installing
-                final Archive[] installedArchive = new Archive[] { null };
+                final List<Archive> installedArchives = new ArrayList<Archive>();
 
-                // Actually install the new archive that we just found.
-                // This will display some UI so we need a shell's sync exec.
+                Shell shell = mUpdaterData.getWindowShell();
+                if (shell != null && !shell.isDisposed()) {
+                    shell.getDisplay().syncExec(new Runnable() {;
+                        public void run() {
+                            List<Archive> archives =
+                                mUpdaterData.updateOrInstallAll_WithGUI(
+                                    archivesToInstall,
+                                    true /* includeObsoletes */);
 
-                mUpdaterData.getWindowShell().getDisplay().syncExec(new Runnable() {
-                    public void run() {
-                        List<Archive> archives =
-                            mUpdaterData.updateOrInstallAll_WithGUI(
-                                archivesToInstall,
-                                true /* includeObsoletes */);
-
-                        if (archives != null && !archives.isEmpty()) {
-                            // We expect that at most one archive has been installed.
-                            assert archives.size() == 1;
-                            installedArchive[0] = archives.get(0);
+                            if (archives != null) {
+                                installedArchives.addAll(archives);
+                            }
                         }
-                    }
-                });
+                    });
+                }
+
+                if (installedArchives.isEmpty()) {
+                    // We failed to install anything.
+                    mInstallPaths = null;
+                    return;
+                }
 
-                // If the desired package has been installed...
-                if (installedArchive[0] == archiveToInstall) {
+                // The local package list has changed, make sure to refresh it
+                mUpdaterData.getSdkManager().reloadSdk(mUpdaterData.getSdkLog());
+                mUpdaterData.getLocalSdkParser().clearPackages();
+                final Package[] localPkgs = mUpdaterData.getInstalledPackages(
+                        new NullTaskMonitor(mUpdaterData.getSdkLog()));
 
-                    // The local package list has changed, make sure to refresh it
-                    mUpdaterData.getLocalSdkParser().clearPackages();
-                    final Package[] localPkgs = mUpdaterData.getInstalledPackages(
-                            new NullTaskMonitor(mUpdaterData.getSdkLog()));
+                // Scan the installed package list to find the install paths.
+                for (Archive installedArchive : installedArchives) {
+                    Package pkg = installedArchive.getParentPackage();
 
-                    // Try to locate the installed package in the new package list
                     for (Package localPkg : localPkgs) {
-                        if (localPkg.canBeUpdatedBy(packageToInstall) == UpdateInfo.NOT_UPDATE) {
+                        if (localPkg.canBeUpdatedBy(pkg) == UpdateInfo.NOT_UPDATE) {
                             Archive[] localArchive = localPkg.getArchives();
                             if (localArchive.length == 1 && localArchive[0].isCompatible()) {
-                                installTask.setResult(
-                                        localPkg,
-                                        true /*success*/,
-                                        new File(localArchive[0].getLocalOsPath()));
-                                return;
+                                mInstallPaths.put(
+                                    localPkg,
+                                    new File(localArchive[0].getLocalOsPath()));
                             }
                         }
                     }
                 }
-
-                // We failed to install the package.
-                installTask.setResult(packageToInstall, false /*success*/, null);
-            }
-
-            public void onLoadCompleted() {
-                installTask.taskCompleted();
             }
         });
     }
index fb8376e..d4eb362 100755 (executable)
@@ -178,7 +178,15 @@ public final class ProgressView implements IProgressUiProvider {
 \r
     private void syncExec(final Widget widget, final Runnable runnable) {\r
         if (widget != null && !widget.isDisposed()) {\r
-            widget.getDisplay().syncExec(runnable);\r
+            widget.getDisplay().syncExec(new Runnable() {\r
+                public void run() {\r
+                    // Check again whether the widget got disposed between the time where\r
+                    // we requested the syncExec and the time it actually happened.\r
+                    if (!widget.isDisposed()) {\r
+                        runnable.run();\r
+                    }\r
+                }\r
+            });\r
         }\r
     }\r
 \r
index addb856..6bb0cdd 100755 (executable)
@@ -193,7 +193,7 @@ public abstract class SwtBaseDialog extends Dialog {
      * in which case the dialog will close as soon as possible.
      */
     protected void close() {
-        if (mShell != null) {
+        if (mShell != null && !mShell.isDisposed()) {
             saveSize();
             getShell().close();
         }