Merge "ADT: fix spurious SDK refresh when opening layout editor." into idea133

12 files changed, 81 insertions, 22 deletions

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtPlugin.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtPlugin.java
index cbeb274..c53fdb0 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtPlugin.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/AdtPlugin.java
@@ -1810,6 +1810,12 @@ public class AdtPlugin extends AbstractUIPlugin implements ILogger {
      * and updates opened projects.
      * <p/>
      * The operation is asynchronous and happens in a background eclipse job.
+     * <p/>
+     * This operation is called in multiple places and should be reasonably
+     * cheap and conservative. The goal is to automatically refresh the SDK
+     * when it is obvious it has changed so when not sure the code should
+     * tend to not reload and avoid reloading too often (which is an expensive
+     * operation that has a lot of user impact.)
      */
     public void refreshSdk() {
         // SDK can't have changed if we haven't loaded it yet.
@@ -1822,8 +1828,29 @@ public class AdtPlugin extends AbstractUIPlugin implements ILogger {
             @Override
             protected IStatus run(IProgressMonitor monitor) {
                 // SDK has changed if its location path is different.
-                boolean changed = sdk.getSdkLocation() == null ||
-                                 !sdk.getSdkLocation().equals(AdtPrefs.getPrefs().getOsSdkFolder());
+                File location = sdk.getSdkFileLocation();
+                boolean changed = location == null || !location.isDirectory();
+
+                if (!changed) {
+                    assert location != null;
+                    File prefLocation = new File(AdtPrefs.getPrefs().getOsSdkFolder());
+                    changed = !location.equals(prefLocation);
+
+                    if (changed) {
+                        // Basic file path comparison indicates they are not the same.
+                        // Let's dig a bit deeper.
+                        try {
+                            location     = location.getCanonicalFile();
+                            prefLocation = prefLocation.getCanonicalFile();
+                            changed = !location.equals(prefLocation);
+                        } catch (IOException ignore) {
+                            // There's no real reason for the canonicalization to fail
+                            // if the paths map to actual directories. And if they don't
+                            // this should have been caught above.
+                        }
+                    }
+                }
+
                 if (!changed) {
                     // Check whether the target directories has potentially changed.
                     changed = sdk.haveTargetsChanged();
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AddSupportJarAction.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AddSupportJarAction.java
index 37559e9..2eeec5b 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AddSupportJarAction.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AddSupportJarAction.java
@@ -172,7 +172,7 @@ public class AddSupportJarAction implements IObjectActionDelegate {
             return null;
         }
 
-        String sdkLocation = sdk.getSdkLocation();
+        String sdkLocation = sdk.getSdkOsLocation();
         if (minimumRevision > 0) {
             File path = getSupportJarFile();
             if (path != null) {
@@ -229,7 +229,7 @@ public class AddSupportJarAction implements IObjectActionDelegate {
     public static int getInstalledRevision() {
         final Sdk sdk = Sdk.getCurrent();
         if (sdk != null) {
-            String sdkLocation = sdk.getSdkLocation();
+            String sdkLocation = sdk.getSdkOsLocation();
             SdkManager manager = SdkManager.createManager(sdkLocation, NullLogger.getLogger());
             Map<String, Integer> versions = manager.getExtrasVersions();
             Integer version = versions.get(VENDOR_ID + '/' + SUPPORT_ID);
@@ -362,7 +362,7 @@ public class AddSupportJarAction implements IObjectActionDelegate {
     private static File getSupportPackageDir() {
         final Sdk sdk = Sdk.getCurrent();
         if (sdk != null) {
-            String sdkLocation = sdk.getSdkLocation();
+            String sdkLocation = sdk.getSdkOsLocation();
             SdkManager manager = SdkManager.createManager(sdkLocation, NullLogger.getLogger());
             Map<String, Integer> versions = manager.getExtrasVersions();
             Integer version = versions.get(VENDOR_ID + '/' + SUPPORT_ID);
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AvdManagerAction.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AvdManagerAction.java
index 2597090..86f2d3c 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AvdManagerAction.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/AvdManagerAction.java
@@ -57,7 +57,7 @@ public class AvdManagerAction implements IWorkbenchWindowActionDelegate, IObject
             AvdManagerWindow window = new AvdManagerWindow(
                     AdtPlugin.getShell(),
                     new AdtConsoleSdkLog(),
-                    sdk.getSdkLocation(),
+                    sdk.getSdkOsLocation(),
                     AvdInvocationContext.IDE);
             window.open();
         } else {
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/SdkManagerAction.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/SdkManagerAction.java
index 9d33230..9a7e7a9 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/SdkManagerAction.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/actions/SdkManagerAction.java
@@ -147,7 +147,7 @@ public class SdkManagerAction implements IWorkbenchWindowActionDelegate, IObject
                     // Get the SDK locatiom from the current SDK or as fallback
                     // directly from the ADT preferences.
                     Sdk sdk = Sdk.getCurrent();
-                    String osSdkLocation = sdk == null ? null : sdk.getSdkLocation();
+                    String osSdkLocation = sdk == null ? null : sdk.getSdkOsLocation();
                     if (osSdkLocation == null || !new File(osSdkLocation).isDirectory()) {
                         osSdkLocation = AdtPrefs.getPrefs().getOsSdkFolder();
                     }
@@ -272,7 +272,7 @@ public class SdkManagerAction implements IWorkbenchWindowActionDelegate, IObject
                         // Do not show non-error/warning log in Eclipse.
                     };
                 },
-                sdk.getSdkLocation(),
+                sdk.getSdkOsLocation(),
                 SdkInvocationContext.IDE);
 
         ISdkChangeListener listener = new ISdkChangeListener() {
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java
index 8e831b7..c981e90 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java
@@ -581,9 +581,9 @@ public class BuildHelper {
         String[] envp = null;
         Map<String, String> envMap = new TreeMap<String, String>(System.getenv());
         if (!envMap.containsKey("PROGUARD_HOME")) {                                    //$NON-NLS-1$
-            envMap.put("PROGUARD_HOME",    Sdk.getCurrent().getSdkLocation() +         //$NON-NLS-1$
-                                            SdkConstants.FD_TOOLS + File.separator +
-                                            SdkConstants.FD_PROGUARD);
+            envMap.put("PROGUARD_HOME", Sdk.getCurrent().getSdkOsLocation() +          //$NON-NLS-1$
+                                        SdkConstants.FD_TOOLS + File.separator +
+                                        SdkConstants.FD_PROGUARD);
             envp = new String[envMap.size()];
             int i = 0;
             for (Map.Entry<String, String> entry : envMap.entrySet()) {
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/Hyperlinks.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/Hyperlinks.java
index 1df2e4d..95cec47 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/Hyperlinks.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/Hyperlinks.java
@@ -445,7 +445,7 @@ public class Hyperlinks {
      */
     private static URL getDocUrl(String relative) {
         // First try to find locally installed documentation
-        File sdkLocation = new File(Sdk.getCurrent().getSdkLocation());
+        File sdkLocation = new File(Sdk.getCurrent().getSdkOsLocation());
         File docs = new File(sdkLocation, FD_DOCS + File.separator + FD_DOCS_REFERENCE);
         try {
             if (docs.exists()) {
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/RenderService.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/RenderService.java
index 3da6565..ffcb258 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/RenderService.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/RenderService.java
@@ -168,7 +168,7 @@ public class RenderService {
         if (RenderSecurityManager.RESTRICT_READS) {
             projectPath = AdtUtils.getAbsolutePath(mProject).toFile().getPath();
             Sdk sdk = Sdk.getCurrent();
-            sdkPath = sdk != null ? sdk.getSdkLocation() : null;
+            sdkPath = sdk != null ? sdk.getSdkOsLocation() : null;
         }
         RenderSecurityManager securityManager = new RenderSecurityManager(sdkPath, projectPath);
         securityManager.setLogger(AdtPlugin.getDefault());
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/DeviceChooserDialog.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/DeviceChooserDialog.java
index 1b850c9..995ccdf 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/DeviceChooserDialog.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/DeviceChooserDialog.java
@@ -479,7 +479,7 @@ public class DeviceChooserDialog extends Dialog implements IDeviceChangeListener
         offsetComp.setLayout(layout);
 
         mPreferredAvdSelector = new AvdSelector(offsetComp,
-                mSdk.getSdkLocation(),
+                mSdk.getSdkOsLocation(),
                 mSdk.getAvdManager(),
                 new NonRunningAvdFilter(),
                 DisplayMode.SIMPLE_SELECTION,
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java
index 248cb7a..779dfa1 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java
@@ -233,7 +233,7 @@ public class EmulatorConfigTab extends AbstractLaunchConfigurationTab {
         // displayed to ensure we have the latest one (dialog is reused but SDK could have
         // been changed in between.
         mPreferredAvdSelector = new AvdSelector(avdOffsetComp,
-                Sdk.getCurrent().getSdkLocation(),
+                Sdk.getCurrent().getSdkOsLocation(),
                 null /* avd manager */,
                 DisplayMode.SIMPLE_CHECK,
                 new AdtConsoleSdkLog());
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/project/LibraryClasspathContainerInitializer.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/project/LibraryClasspathContainerInitializer.java
index 914e1de..525e2fd 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/project/LibraryClasspathContainerInitializer.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/project/LibraryClasspathContainerInitializer.java
@@ -326,7 +326,7 @@ public class LibraryClasspathContainerInitializer extends BaseClasspathContainer
 
             // annotations support for older version of android
             if (state.getTarget() != null && state.getTarget().getVersion().getApiLevel() <= 15) {
-                File annotationsJar = new File(Sdk.getCurrent().getSdkLocation(),
+                File annotationsJar = new File(Sdk.getCurrent().getSdkOsLocation(),
                         SdkConstants.FD_TOOLS + File.separator + SdkConstants.FD_SUPPORT +
                         File.separator + SdkConstants.FN_ANNOTATIONS_JAR);
 
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/sdk/Sdk.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/sdk/Sdk.java
index 1a0e049..8e40f1c 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/sdk/Sdk.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/sdk/Sdk.java
@@ -321,15 +321,47 @@ public final class Sdk  {
     }
 
     /**
-     * Returns the location (OS path) of the current SDK.
+     * Returns the location of the current SDK as an OS path string.
+     * Guaranteed to be terminated by a platform-specific path separator.
+     * <p/>
+     * Due to {@link File} canonicalization, this MAY differ from the string used to initialize
+     * the SDK path.
+     *
+     * @return The SDK OS path or null if no SDK is setup.
+     * @deprecated Consider using {@link #getSdkFileLocation()} instead.
+     * @see #getSdkFileLocation()
      */
-    public String getSdkLocation() {
-        return mManager.getLocation();
+    @Deprecated
+    @Nullable
+    public String getSdkOsLocation() {
+        String path = mManager == null ? null : mManager.getLocation();
+        if (path != null) {
+            // For backward compatibility make sure it ends with a separator.
+            // This used to be the case when the SDK Manager was created from a String path
+            // but now that a File is internally used the trailing dir separator is lost.
+            if (path.length() > 0 && !path.endsWith(File.separator)) {
+                path = path + File.separator;
+            }
+        }
+        return path;
+    }
+
+    /**
+     * Returns the location of the current SDK as a {@link File} or null.
+     *
+     * @return The SDK OS path or null if no SDK is setup.
+     */
+    @Nullable
+    public File getSdkFileLocation() {
+        if (mManager == null || mManager.getLocalSdk() == null) {
+            return null;
+        }
+        return mManager.getLocalSdk().getLocation();
     }
 
     /**
      * Returns a <em>new</em> {@link SdkManager} that can parse the SDK located
-     * at the current {@link #getSdkLocation()}.
+     * at the current {@link #getSdkOsLocation()}.
      * <p/>
      * Implementation detail: The {@link Sdk} has its own internal manager with
      * a custom logger which is not designed to be useful for outsiders. Callers
@@ -343,7 +375,7 @@ public final class Sdk  {
      * @return A new {@link SdkManager} parsing the same location.
      */
     public @Nullable SdkManager getNewSdkManager(@NonNull ILogger log) {
-        return SdkManager.createManager(getSdkLocation(), log);
+        return SdkManager.createManager(getSdkOsLocation(), log);
     }
 
     /**
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/exportgradle/BuildFileCreator.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/exportgradle/BuildFileCreator.java
index 9ee7e31..e749e71 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/exportgradle/BuildFileCreator.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/exportgradle/BuildFileCreator.java
@@ -110,7 +110,7 @@ public class BuildFileCreator {
             @NonNull Shell shell,
             @NonNull IProgressMonitor pm) {
 
-        File gradleLocation = new File(Sdk.getCurrent().getSdkLocation(), GRADLE_WRAPPER_LOCATION);
+        File gradleLocation = new File(Sdk.getCurrent().getSdkOsLocation(), GRADLE_WRAPPER_LOCATION);
         SubMonitor localmonitor = null;
 
         try {