[android] - optimise loaded style for location component, expose isFullyLoaded

7 files changed, 150 insertions, 18 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationCameraController.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationCameraController.java
index 7a20ca8177..a50b510073 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationCameraController.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationCameraController.java
@@ -6,7 +6,6 @@ import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
 import android.view.MotionEvent;
-
 import com.mapbox.android.gestures.AndroidGesturesManager;
 import com.mapbox.android.gestures.MoveGestureDetector;
 import com.mapbox.android.gestures.RotateGestureDetector;
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponent.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponent.java
index 2391d6a86e..59ce2a900e 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponent.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationComponent.java
@@ -854,7 +854,8 @@ public final class LocationComponent {
    */
   public void onFinishLoadingStyle() {
     if (isComponentInitialized) {
-      locationLayerController.initializeComponents(options);
+      style = mapboxMap.getStyle();
+      locationLayerController.initializeComponents(style, options);
       locationCameraController.initializeOptions(options);
       onLocationLayerStart();
     }
@@ -913,7 +914,10 @@ public final class LocationComponent {
     if (isComponentInitialized) {
       return;
     }
-    isComponentInitialized = true;
+
+    if (!style.isFullyLoaded()) {
+      throw new IllegalStateException("Style hasn't fully loaded yet, can't initialize LocationComponent.");
+    }
 
     this.style = style;
     this.options = options;
@@ -946,6 +950,8 @@ public final class LocationComponent {
     setRenderMode(RenderMode.NORMAL);
     setCameraMode(CameraMode.NONE);
 
+    isComponentInitialized = true;
+
     onLocationLayerStart();
   }
 
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java
index ed15e52a08..b67ce3da88 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationLayerController.java
@@ -62,7 +62,7 @@ final class LocationLayerController implements MapboxAnimator.OnLayerAnimationsV
   private int renderMode;
 
   private final MapboxMap mapboxMap;
-  private final Style style;
+  private Style style;
   private final LayerSourceProvider layerSourceProvider;
   private final LayerBitmapProvider bitmapProvider;
   private LocationComponentOptions options;
@@ -81,11 +81,12 @@ final class LocationLayerController implements MapboxAnimator.OnLayerAnimationsV
     this.layerSourceProvider = layerSourceProvider;
     this.bitmapProvider = bitmapProvider;
     this.locationFeature = featureProvider.generateLocationFeature(locationFeature, options);
-    initializeComponents(options);
+    initializeComponents(style, options);
     setRenderMode(RenderMode.NORMAL);
   }
 
-  void initializeComponents(LocationComponentOptions options) {
+  void initializeComponents(Style style, LocationComponentOptions options) {
+    this.style = style;
     addLocationSource();
     addLayers(options.layerBelow());
     applyStyle(options);
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java
index 03fbdab3bd..d7a61af7eb 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java
@@ -101,7 +101,7 @@ public final class MapboxMap {
    * Get the Style of the map asynchronously.
    */
   public void getStyle(@NonNull Style.OnStyleLoaded onStyleLoaded) {
-    if (style != null && style.isStyleLoaded()) {
+    if (style != null && style.isFullyLoaded()) {
       onStyleLoaded.onStyleLoaded(style);
     } else {
       styleLoadedCallbacks.add(onStyleLoaded);
@@ -118,7 +118,7 @@ public final class MapboxMap {
    */
   @Nullable
   public Style getStyle() {
-    if (style == null || !style.isStyleLoaded()) {
+    if (style == null || !style.isFullyLoaded()) {
       return null;
     } else {
       return style;
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java
index 72de0b24c5..c803042242 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java
@@ -34,7 +34,7 @@ public class Style {
   private final HashMap<String, Layer> layers = new HashMap<>();
   private final HashMap<String, Bitmap> images = new HashMap<>();
   private final Builder builder;
-  private boolean styleLoaded;
+  private boolean fullyLoaded;
 
   /**
    * Private constructor to build a style object.
@@ -54,6 +54,11 @@ public class Style {
    */
   @NonNull
   public String getUrl() {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return "";
+    }
+
     return nativeMapView.getStyleUrl();
   }
 
@@ -64,6 +69,11 @@ public class Style {
    */
   @NonNull
   public String getJson() {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return "";
+    }
+
     return nativeMapView.getStyleJson();
   }
 
@@ -78,6 +88,11 @@ public class Style {
    */
   @NonNull
   public List<Source> getSources() {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return new ArrayList<>();
+    }
+
     return nativeMapView.getSources();
   }
 
@@ -87,6 +102,11 @@ public class Style {
    * @param source the source to add
    */
   public void addSource(@NonNull Source source) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return;
+    }
+
     sources.put(source.getId(), source);
     nativeMapView.addSource(source);
   }
@@ -99,6 +119,11 @@ public class Style {
    */
   @Nullable
   public Source getSource(String id) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return null;
+    }
+
     Source source = sources.get(id);
     if (source == null) {
       source = nativeMapView.getSource(id);
@@ -115,6 +140,11 @@ public class Style {
    */
   @Nullable
   public <T extends Source> T getSourceAs(@NonNull String sourceId) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return null;
+    }
+
     // noinspection unchecked
     if (sources.containsKey(sourceId)) {
       return (T) sources.get(sourceId);
@@ -128,8 +158,12 @@ public class Style {
    * @param sourceId the source to remove
    * @return the source handle or null if the source was not present
    */
-  @Nullable
   public boolean removeSource(@NonNull String sourceId) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return false;
+    }
+
     sources.remove(sourceId);
     return nativeMapView.removeSource(sourceId);
   }
@@ -140,8 +174,12 @@ public class Style {
    * @param source the source to remove
    * @return the source
    */
-  @Nullable
   public boolean removeSource(@NonNull Source source) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return false;
+    }
+
     sources.remove(source.getId());
     return nativeMapView.removeSource(source);
   }
@@ -156,6 +194,11 @@ public class Style {
    * @param layer the layer to add
    */
   public void addLayer(@NonNull Layer layer) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return;
+    }
+
     layers.put(layer.getId(), layer);
     nativeMapView.addLayer(layer);
   }
@@ -167,6 +210,11 @@ public class Style {
    * @param below the layer id to add this layer before
    */
   public void addLayerBelow(@NonNull Layer layer, @NonNull String below) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return;
+    }
+
     layers.put(layer.getId(), layer);
     nativeMapView.addLayerBelow(layer, below);
   }
@@ -178,6 +226,11 @@ public class Style {
    * @param above the layer id to add this layer above
    */
   public void addLayerAbove(@NonNull Layer layer, @NonNull String above) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return;
+    }
+
     layers.put(layer.getId(), layer);
     nativeMapView.addLayerAbove(layer, above);
   }
@@ -190,6 +243,11 @@ public class Style {
    * @param index the index to insert the layer at
    */
   public void addLayerAt(@NonNull Layer layer, @IntRange(from = 0) int index) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return;
+    }
+
     layers.put(layer.getId(), layer);
     nativeMapView.addLayerAt(layer, index);
   }
@@ -202,6 +260,11 @@ public class Style {
    */
   @Nullable
   public Layer getLayer(@NonNull String id) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return null;
+    }
+
     Layer layer = layers.get(id);
     if (layer == null) {
       layer = nativeMapView.getLayer(id);
@@ -218,6 +281,11 @@ public class Style {
    */
   @Nullable
   public <T extends Layer> T getLayerAs(@NonNull String layerId) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return null;
+    }
+
     // noinspection unchecked
     return (T) nativeMapView.getLayer(layerId);
   }
@@ -229,6 +297,11 @@ public class Style {
    */
   @NonNull
   public List<Layer> getLayers() {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return new ArrayList<>();
+    }
+
     return nativeMapView.getLayers();
   }
 
@@ -239,6 +312,10 @@ public class Style {
    * @return the removed layer or null if not found
    */
   public boolean removeLayer(@NonNull String layerId) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return false;
+    }
     layers.remove(layerId);
     return nativeMapView.removeLayer(layerId);
   }
@@ -250,6 +327,10 @@ public class Style {
    * @return the layer
    */
   public boolean removeLayer(@NonNull Layer layer) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return false;
+    }
     layers.remove(layer.getId());
     return nativeMapView.removeLayer(layer);
   }
@@ -286,6 +367,11 @@ public class Style {
    * @param sdf   the flag indicating image is an SDF or template image
    */
   public void addImage(@NonNull String name, @NonNull Bitmap image, boolean sdf) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return;
+    }
+
     nativeMapView.addImage(name, image, sdf);
   }
 
@@ -293,6 +379,11 @@ public class Style {
    * Adds an images to be used in the map's style.
    */
   public void addImages(@NonNull HashMap<String, Bitmap> images) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return;
+    }
+
     nativeMapView.addImages(images);
   }
 
@@ -302,6 +393,11 @@ public class Style {
    * @param name the name of the image to remove
    */
   public void removeImage(@NonNull String name) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return;
+    }
+
     nativeMapView.removeImage(name);
   }
 
@@ -313,6 +409,11 @@ public class Style {
    */
   @Nullable
   public Bitmap getImage(@NonNull String id) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return null;
+    }
+
     return nativeMapView.getImage(id);
   }
 
@@ -329,6 +430,11 @@ public class Style {
    * @param transitionOptions the transition options
    */
   public void setTransition(@NonNull TransitionOptions transitionOptions) {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return;
+    }
+
     nativeMapView.setTransitionDuration(transitionOptions.getDuration());
     nativeMapView.setTransitionDelay(transitionOptions.getDelay());
   }
@@ -343,6 +449,11 @@ public class Style {
    */
   @NonNull
   public TransitionOptions getTransition() {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return new TransitionOptions(0, 0);
+    }
+
     return new TransitionOptions(nativeMapView.getTransitionDuration(), nativeMapView.getTransitionDelay());
   }
 
@@ -357,6 +468,11 @@ public class Style {
    */
   @Nullable
   public Light getLight() {
+    if (!fullyLoaded) {
+      // we are loading a new style
+      return null;
+    }
+
     return nativeMapView.getLight();
   }
 
@@ -365,7 +481,6 @@ public class Style {
    * by setting the java sources and layers in a detached state and removing them from core.
    */
   void onWillStartLoadingMap() {
-    styleLoaded = false;
     for (Source source : sources.values()) {
       if (source != null) {
         source.setDetached();
@@ -388,6 +503,8 @@ public class Style {
     sources.clear();
     layers.clear();
     images.clear();
+
+    fullyLoaded = false;
   }
 
   /**
@@ -395,8 +512,8 @@ public class Style {
    * This method will add all components added to the builder that were defined with the 'with' prefix.
    */
   void onDidFinishLoadingStyle() {
-    if (!styleLoaded) {
-      styleLoaded = true;
+    if (!fullyLoaded) {
+      fullyLoaded = true;
       for (Source source : builder.sources) {
         addSource(source);
       }
@@ -424,8 +541,8 @@ public class Style {
     }
   }
 
-  boolean isStyleLoaded() {
-    return styleLoaded;
+  public boolean isFullyLoaded() {
+    return fullyLoaded;
   }
 
   //
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/LocationComponentTest.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/LocationComponentTest.kt
index 8362095042..94e1f079f3 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/LocationComponentTest.kt
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/LocationComponentTest.kt
@@ -153,7 +153,7 @@ class LocationComponentTest : BaseActivityTest() {
     executeComponentTest(componentAction)
   }
 
-  @Test
+  @Test(expected = IllegalStateException::class)
   fun settingMapStyleImmediatelyBeforeLoadingComponent_doesStillLoadLayersProperly() {
     validateTestSetup()
     val componentAction = object : LocationComponentAction.OnPerformLocationComponentAction {
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/utils/MapboxTestingUtils.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/utils/MapboxTestingUtils.kt
index 0a9aa48503..20ef5efded 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/utils/MapboxTestingUtils.kt
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/location/utils/MapboxTestingUtils.kt
@@ -48,6 +48,15 @@ fun MapboxMap.waitForLayer(uiController: UiController, location: Location, layer
   }
 }
 
+fun MapboxMap.waitForStyle(uiController: UiController, mapboxMap: MapboxMap) {
+  var counter = 0
+  val delay = MapboxTestingUtils.MAP_RENDER_DELAY
+  while ((mapboxMap.style == null && !mapboxMap.style?.isFullyLoaded!!) && delay * counter < MapboxTestingUtils.RENDER_TIMEOUT) {
+    uiController.loopMainThreadForAtLeast(delay)
+    counter++
+  }
+}
+
 inline fun waitForRenderResult(uiController: UiController, checkFunction: () -> Boolean, expectedResult: Boolean) {
   var counter = 0
   val delay = MapboxTestingUtils.MAP_RENDER_DELAY
@@ -62,7 +71,7 @@ class MapboxTestingUtils {
 
     const val MAP_RENDER_DELAY = 250L
     const val MAP_CONNECTION_DELAY = 1000L
-    const val RENDER_TIMEOUT = 2_500L
+    const val RENDER_TIMEOUT = 5_500L
 
     /**
      * Used to increase style load time for stress testing.