Fixing issue #650 - locked thread between location and user sync (#935)

* Fixing issue #650 - locked thread between location and user sync
* SyncUserState was moving on and causing a thread lock with the complete() LocationGMS work
* BlockingQueue used to make sure that the complete() callback is completely finished before moving on

* Unit test broke because of null point being passed
* Fixed by changing BlockArrayQueue to use a `Object`

* Another unit test broke after fixing previous test
* Added null check for `mGoogleApiClient` to prevent `NullPointerException`

Author: mikechoch

OneSignalSDK/onesignal/src/main/java/com/onesignal/FocusTimeController.java

@@ -66,7 +66,7 @@ void doBlockingBackgroundSyncOfUnsentTime() {
       if (OneSignal.isForeground())
          return;
 
-      for(FocusTimeProcessorBase focusTimeProcessor : focusTimeProcessors)
+      for (FocusTimeProcessorBase focusTimeProcessor : focusTimeProcessors)
          focusTimeProcessor.syncUnsentTimeFromSyncJob();
    }
 
@@ -75,7 +75,7 @@ private boolean giveProcessorsValidFocusTime(@NonNull OSSessionManager.SessionRe
       if (timeElapsed == null)
         return false;
 
-      for(FocusTimeProcessorBase focusTimeProcessor : focusTimeProcessors)
+      for (FocusTimeProcessorBase focusTimeProcessor : focusTimeProcessors)
          focusTimeProcessor.addTime(timeElapsed, lastSessionResult.session, focusType);
       return true;
    }

OneSignalSDK/onesignal/src/main/java/com/onesignal/LocationGMS.java

@@ -311,6 +311,9 @@ public void onConnected(Bundle bundle) {
          synchronized (syncLock) {
             PermissionsActivity.answered = false;
 
+            if (mGoogleApiClient == null || mGoogleApiClient.realInstance() == null)
+               return;
+
             if (mLastLocation == null) {
                mLastLocation = FusedLocationApiWrapper.getLastLocation(mGoogleApiClient.realInstance());
                if (mLastLocation != null)

OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignalSyncServiceUtils.java

@@ -43,6 +43,9 @@
 
 import com.onesignal.AndroidSupportV4Compat.ContextCompat;
 
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+
 /**
  This schedules a job to fire later in the background preform player REST calls to make a(n);
    - on_focus
@@ -209,25 +212,40 @@ public final void run() {
          OneSignal.appId = OneSignal.getSavedAppId();
          OneSignalStateSynchronizer.initUserState();
 
-         LocationGMS.LocationHandler locationHandler = new LocationGMS.LocationHandler() {
-            @Override
-            public LocationGMS.CALLBACK_TYPE getType() {
-               return LocationGMS.CALLBACK_TYPE.SYNC_SERVICE;
-            }
-
-            @Override
-            public void complete(LocationGMS.LocationPoint point) {
-               if (point != null)
-                  OneSignalStateSynchronizer.updateLocation(point);
-
-               // Both these calls are synchronous.
-               //   Thread is blocked until network calls are made or their retry limits are reached
-               OneSignalStateSynchronizer.syncUserState(true);
-               FocusTimeController.getInstance().doBlockingBackgroundSyncOfUnsentTime();
-               stopSync();
-            }
-         };
-         LocationGMS.getLocation(OneSignal.appContext, false, locationHandler);
+         // BlockingQueue used to make sure that the complete() callback is completely finished before moving on
+         // SyncUserState was moving on and causing a thread lock with the complete() LocationGMS work
+         // Issue #650
+         // https://github.com/OneSignal/OneSignal-Android-SDK/issues/650
+         try {
+            final BlockingQueue<Object> queue = new ArrayBlockingQueue<>(1);
+            LocationGMS.LocationHandler locationHandler = new LocationGMS.LocationHandler() {
+               @Override
+               public LocationGMS.CALLBACK_TYPE getType() {
+                  return LocationGMS.CALLBACK_TYPE.SYNC_SERVICE;
+               }
+
+               @Override
+               public void complete(LocationGMS.LocationPoint point) {
+                  Object object = point != null ?  point : new Object();
+                  queue.offer(object);
+               }
+            };
+            LocationGMS.getLocation(OneSignal.appContext, false, locationHandler);
+
+            // The take() will return the offered point once the callback for the locationHandler is completed
+            Object point = queue.take();
+            if (point instanceof LocationGMS.LocationPoint)
+               OneSignalStateSynchronizer.updateLocation((LocationGMS.LocationPoint) point);
+
+         } catch (InterruptedException e) {
+            e.printStackTrace();
+         }
+
+         // Both these calls are synchronous
+         // Once the queue calls take the code will continue and move on to the syncUserState
+         OneSignalStateSynchronizer.syncUserState(true);
+         FocusTimeController.getInstance().doBlockingBackgroundSyncOfUnsentTime();
+         stopSync();
       }
 
       protected abstract void stopSync();