From: Raphael Date: Sat, 24 Sep 2011 04:27:14 +0000 (-0700) Subject: First time install: multiple platforms, fix shell disposed. X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=949dc34d6202ec5e318c30992bc11b61e0c64808;p=android-x86%2Fsdk.git First time install: multiple platforms, fix shell disposed. 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 --- diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/welcome/WelcomeWizard.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/welcome/WelcomeWizard.java index 9aa5ead2a..2bcc63cd9 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/welcome/WelcomeWizard.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/welcome/WelcomeWizard.java @@ -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 apiLevels = new HashSet(); 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 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 result = updater.installNewSdk(apiLevel); + boolean result = updater.installNewSdk(apiLevels); - if (!result.getFirst().booleanValue()) { + if (!result) { AdtPlugin.printErrorToConsole("Failed to install Android SDK."); return false; } diff --git a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/UpdaterData.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/UpdaterData.java index b317f7101..0b494a7a6 100755 --- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/UpdaterData.java +++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/UpdaterData.java @@ -55,7 +55,6 @@ import com.android.sdkuilib.internal.repository.sdkman1.SdkUpdaterWindowImpl1; import com.android.sdkuilib.repository.ISdkChangeListener; import org.eclipse.jface.dialogs.MessageDialog; -import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Shell; import java.io.ByteArrayOutputStream; @@ -628,16 +627,19 @@ public class UpdaterData implements IUpdaterData { if (getWindowShell() != null && getSettingsController().getAskBeforeAdbRestart()) { // need to ask for permission first - Display display = getWindowShell().getDisplay(); - - display.syncExec(new Runnable() { - public void run() { - canRestart[0] = MessageDialog.openQuestion(getWindowShell(), - "ADB Restart", - "A package that depends on ADB has been updated. \n" + - "Do you want to restart ADB now?"); - } - }); + final Shell shell = getWindowShell(); + if (shell != null && !shell.isDisposed()) { + shell.getDisplay().syncExec(new Runnable() { + public void run() { + if (!shell.isDisposed()) { + canRestart[0] = MessageDialog.openQuestion(shell, + "ADB Restart", + "A package that depends on ADB has been updated. \n" + + "Do you want to restart ADB now?"); + } + } + }); + } } if (canRestart[0]) { @@ -648,23 +650,23 @@ public class UpdaterData implements IUpdaterData { } private void notifyToolsNeedsToBeRestarted() { - if (getWindowShell() == null) { - // We don't need to print anything if this is a standalone console update. - return; - } + // We don't need to print anything if this is a standalone console update with no shell. - Display display = getWindowShell().getDisplay(); - - display.syncExec(new Runnable() { - public void run() { - MessageDialog.openInformation(getWindowShell(), - "Android Tools Updated", - "The Android SDK and AVD Manager that you are currently using has been updated. " + - "It is recommended that you now close the manager window and re-open it. " + - "If you started this window from Eclipse, please check if the Android " + - "plug-in needs to be updated."); - } - }); + final Shell shell = getWindowShell(); + if (shell != null && !shell.isDisposed()) { + shell.getDisplay().syncExec(new Runnable() { + public void run() { + if (!shell.isDisposed()) { + MessageDialog.openInformation(getWindowShell(), + "Android Tools Updated", + "The Android SDK and AVD Manager that you are currently using has been updated. " + + "It is recommended that you now close the manager window and re-open it. " + + "If you started this window from Eclipse, please check if the Android " + + "plug-in needs to be updated."); + } + } + }); + } } diff --git a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/AdtUpdateDialog.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/AdtUpdateDialog.java index 924dc121e..069102e5f 100755 --- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/AdtUpdateDialog.java +++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/AdtUpdateDialog.java @@ -22,7 +22,9 @@ import com.android.sdklib.ISdkLog; import com.android.sdklib.internal.repository.ExtraPackage; import com.android.sdklib.internal.repository.Package; import com.android.sdklib.internal.repository.PlatformPackage; +import com.android.sdklib.internal.repository.PlatformToolPackage; import com.android.sdklib.internal.repository.SdkSource; +import com.android.sdklib.internal.repository.ToolPackage; import com.android.sdkuilib.internal.repository.SettingsController; import com.android.sdkuilib.internal.repository.UpdaterData; import com.android.sdkuilib.internal.repository.sdkman2.PackageLoader.IAutoInstallTask; @@ -42,6 +44,9 @@ import org.eclipse.swt.widgets.ProgressBar; import org.eclipse.swt.widgets.Shell; import java.io.File; +import java.util.Map; +import java.util.Set; +import java.util.Map.Entry; /** * This is a private implementation of UpdateWindow for ADT, @@ -68,7 +73,7 @@ public class AdtUpdateDialog extends SwtBaseDialog { private final UpdaterData mUpdaterData; private Boolean mResultCode = Boolean.FALSE; - private File mResultPath = null; + private Map mResultPaths = null; private SettingsController mSettingsController; private PackageFilter mPackageFilter; private PackageLoader mPackageMananger; @@ -110,7 +115,18 @@ public class AdtUpdateDialog extends SwtBaseDialog { public Pair installExtraPackage(String vendor, String path) { mPackageFilter = createExtraFilter(vendor, path); open(); - return Pair.of(mResultCode, mResultPath); + + File installPath = null; + if (mResultPaths != null) { + for (Entry entry : mResultPaths.entrySet()) { + if (entry.getKey() instanceof ExtraPackage) { + installPath = entry.getValue(); + break; + } + } + } + + return Pair.of(mResultCode, installPath); } /** @@ -128,30 +144,39 @@ public class AdtUpdateDialog extends SwtBaseDialog { * is false, otherwise it should point to an existing valid folder. */ public Pair installPlatformPackage(int apiLevel) { - mPackageFilter = createPlatformFilter(apiLevel, false /*forceRemote*/); + mPackageFilter = createPlatformFilter(apiLevel); open(); - return Pair.of(mResultCode, mResultPath); + + File installPath = null; + if (mResultPaths != null) { + for (Entry entry : mResultPaths.entrySet()) { + if (entry.getKey() instanceof PlatformPackage) { + installPath = entry.getValue(); + break; + } + } + } + + return Pair.of(mResultCode, installPath); } /** * Displays the update dialog and triggers installation of a new SDK. This works by - * requesting a remote platform package with the specified API level. - * By dependency, any missing tools or platform-tools packages will also be installed. + * requesting a remote platform package with the specified API levels as well as + * the first tools or platform-tools packages available. *

* Callers must not try to reuse this dialog after this call. * - * @param apiLevel The platform API level to match. + * @param apiLevels A set of platform API levels to match. * The special value {@link #USE_MAX_REMOTE_API_LEVEL} means to use * the highest API level available in the repository. - * @return A boolean indicating whether the installation was successful (meaning the package - * was either already present, or got installed or updated properly) and a {@link File} - * with the path to the root folder of the package. The file is null when the boolean - * is false, otherwise it should point to an existing valid folder. + * @return A boolean indicating whether the installation was successful (meaning the packages + * were either already present, or got installed or updated properly). */ - public Pair installNewSdk(int apiLevel) { - mPackageFilter = createPlatformFilter(apiLevel, true /*forceRemote*/); + public boolean installNewSdk(Set apiLevels) { + mPackageFilter = createNewSdkFilter(apiLevels); open(); - return Pair.of(mResultCode, mResultPath); + return mResultCode.booleanValue(); } @Override @@ -214,10 +239,10 @@ public class AdtUpdateDialog extends SwtBaseDialog { return mPackageFilter.accept(pkg); } - public void setResult(Package pkg, boolean success, File installPath) { + public void setResult(boolean success, Map installPaths) { // Capture the result from the installation. mResultCode = Boolean.valueOf(success); - mResultPath = installPath; + mResultPaths = installPaths; } public void taskCompleted() { @@ -288,9 +313,7 @@ public class AdtUpdateDialog extends SwtBaseDialog { }; } - public static PackageFilter createPlatformFilter( - final int apiLevel, - final boolean forceRemote) { + public static PackageFilter createPlatformFilter(final int apiLevel) { return new PackageFilter() { int mApiLevel = apiLevel; boolean mFindMaxApi = apiLevel == USE_MAX_REMOTE_API_LEVEL; @@ -298,10 +321,6 @@ public class AdtUpdateDialog extends SwtBaseDialog { @Override boolean accept(Package pkg) { if (pkg instanceof PlatformPackage) { - if (forceRemote && pkg.isLocal()) { - // We only want remote packages, to force a fresh install. - return false; - } PlatformPackage pp = (PlatformPackage) pkg; AndroidVersion v = pp.getVersion(); return !v.isPreview() && v.getApiLevel() == mApiLevel; @@ -328,6 +347,60 @@ public class AdtUpdateDialog extends SwtBaseDialog { }; } + public static PackageFilter createNewSdkFilter(final Set apiLevels) { + return new PackageFilter() { + int mMaxApiLevel; + boolean mFindMaxApi = apiLevels.contains(USE_MAX_REMOTE_API_LEVEL); + boolean mNeedTools = true; + boolean mNeedPlatformTools = true; + + @Override + boolean accept(Package pkg) { + if (!pkg.isLocal()) { + if (pkg instanceof PlatformPackage) { + PlatformPackage pp = (PlatformPackage) pkg; + AndroidVersion v = pp.getVersion(); + if (!v.isPreview()) { + int level = v.getApiLevel(); + if ((mFindMaxApi && level == mMaxApiLevel) || + (level > 0 && apiLevels.contains(level))) { + return true; + } + } + } else if (mNeedTools && pkg instanceof ToolPackage) { + // We want a tool package. There should be only one, + // but in case of error just take the first one. + mNeedTools = false; + return true; + } else if (mNeedPlatformTools && pkg instanceof PlatformToolPackage) { + // We want a platform-tool package. There should be only one, + // but in case of error just take the first one. + mNeedPlatformTools = false; + return true; + } + } + return false; + } + + @Override + void visit(Package pkg) { + // Try to find the max API in all remote packages + if (mFindMaxApi && + pkg instanceof PlatformPackage && + !pkg.isLocal()) { + PlatformPackage pp = (PlatformPackage) pkg; + AndroidVersion v = pp.getVersion(); + if (!v.isPreview()) { + int api = v.getApiLevel(); + if (api > mMaxApiLevel) { + mMaxApiLevel = api; + } + } + } + } + }; + } + // End of hiding from SWT Designer diff --git a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackageLoader.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackageLoader.java index 1ed44bcfe..f296cb096 100755 --- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackageLoader.java +++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/sdkman2/PackageLoader.java @@ -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. *

* Important: 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. *

* Important: 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 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 mArchivesToInstall = new ArrayList(); + Map mInstallPaths = new HashMap(); + 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 archivesToInstall = new ArrayList(); - archivesToInstall.add(archiveToInstall); - - Package packageToInstall = archiveToInstall.getParentPackage(); + private void installArchives(final List 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 installedArchives = new ArrayList(); - // 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 archives = + mUpdaterData.updateOrInstallAll_WithGUI( + archivesToInstall, + true /* includeObsoletes */); - mUpdaterData.getWindowShell().getDisplay().syncExec(new Runnable() { - public void run() { - List 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(); } }); } diff --git a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/tasks/ProgressView.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/tasks/ProgressView.java index fb8376ed4..d4eb362c4 100755 --- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/tasks/ProgressView.java +++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/tasks/ProgressView.java @@ -178,7 +178,15 @@ public final class ProgressView implements IProgressUiProvider { private void syncExec(final Widget widget, final Runnable runnable) { if (widget != null && !widget.isDisposed()) { - widget.getDisplay().syncExec(runnable); + widget.getDisplay().syncExec(new Runnable() { + public void run() { + // Check again whether the widget got disposed between the time where + // we requested the syncExec and the time it actually happened. + if (!widget.isDisposed()) { + runnable.run(); + } + } + }); } } diff --git a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/ui/SwtBaseDialog.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/ui/SwtBaseDialog.java index addb85691..6bb0cdddd 100755 --- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/ui/SwtBaseDialog.java +++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/ui/SwtBaseDialog.java @@ -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(); }