diff --git a/android/app/src/main/java/app/organicmaps/location/LocationHelper.java b/android/app/src/main/java/app/organicmaps/location/LocationHelper.java index 612b64ac33..92ab5e13c1 100644 --- a/android/app/src/main/java/app/organicmaps/location/LocationHelper.java +++ b/android/app/src/main/java/app/organicmaps/location/LocationHelper.java @@ -18,6 +18,8 @@ import androidx.core.content.ContextCompat; import androidx.core.location.GnssStatusCompat; import androidx.core.location.LocationManagerCompat; +import org.chromium.base.ObserverList; + import app.organicmaps.Framework; import app.organicmaps.Map; import app.organicmaps.MwmApplication; @@ -30,9 +32,6 @@ import app.organicmaps.util.LocationUtils; import app.organicmaps.util.NetworkPolicy; import app.organicmaps.util.log.Logger; -import java.util.LinkedHashSet; -import java.util.Set; - public class LocationHelper implements BaseLocationProvider.Listener { private static final long INTERVAL_FOLLOW_MS = 0; @@ -47,8 +46,10 @@ public class LocationHelper implements BaseLocationProvider.Listener private final Context mContext; private static final String TAG = LocationState.LOCATION_TAG; - @NonNull - private final Set mListeners = new LinkedHashSet<>(); + + private final ObserverList mListeners = new ObserverList<>(); + private final ObserverList.RewindableIterator mListenersIterator = mListeners.rewindableIterator(); + @Nullable private Location mSavedLocation; private MapObject mMyPosition; @@ -156,8 +157,9 @@ public class LocationHelper implements BaseLocationProvider.Listener mHandler.removeCallbacks(mLocationTimeoutRunnable); mHandler.postDelayed(mLocationTimeoutRunnable, LOCATION_UPDATE_TIMEOUT_MS); // Reset the timeout. - for (LocationListener listener : mListeners) - listener.onLocationUpdated(mSavedLocation); + mListenersIterator.rewind(); + while (mListenersIterator.hasNext()) + mListenersIterator.next().onLocationUpdated(mSavedLocation); // If we are still in the first run mode, i.e. user is staying on the first run screens, // not on the map, we mustn't post location update to the core. Only this preserving allows us @@ -187,8 +189,9 @@ public class LocationHelper implements BaseLocationProvider.Listener } Logger.d(TAG); - for (LocationListener listener : mListeners) - listener.onLocationUpdateTimeout(); + mListenersIterator.rewind(); + while (mListenersIterator.hasNext()) + mListenersIterator.next().onLocationUpdateTimeout(); } @Override @@ -240,8 +243,9 @@ public class LocationHelper implements BaseLocationProvider.Listener stop(); LocationState.nativeOnLocationError(LocationState.ERROR_GPS_OFF); - for (LocationListener listener : mListeners) - listener.onLocationResolutionRequired(pendingIntent); + mListenersIterator.rewind(); + while (mListenersIterator.hasNext()) + mListenersIterator.next().onLocationResolutionRequired(pendingIntent); } // Used by GoogleFusedLocationProvider. @@ -281,8 +285,9 @@ public class LocationHelper implements BaseLocationProvider.Listener stop(); LocationState.nativeOnLocationError(LocationState.ERROR_GPS_OFF); - for (LocationListener listener : mListeners) - listener.onLocationDisabled(); + mListenersIterator.rewind(); + while (mListenersIterator.hasNext()) + mListenersIterator.next().onLocationDisabled(); } /** @@ -295,7 +300,7 @@ public class LocationHelper implements BaseLocationProvider.Listener { Logger.d(TAG, "listener: " + listener + " count was: " + mListeners.size()); - mListeners.add(listener); + mListeners.addObserver(listener); if (mSavedLocation != null) listener.onLocationUpdated(mSavedLocation); } @@ -308,7 +313,7 @@ public class LocationHelper implements BaseLocationProvider.Listener public void removeListener(@NonNull LocationListener listener) { Logger.d(TAG, "listener: " + listener + " count was: " + mListeners.size()); - mListeners.remove(listener); + mListeners.removeObserver(listener); } private long calcLocationUpdatesInterval() diff --git a/android/app/src/main/java/org/chromium/base/ObserverList.java b/android/app/src/main/java/org/chromium/base/ObserverList.java new file mode 100644 index 0000000000..16ae0d08bd --- /dev/null +++ b/android/app/src/main/java/org/chromium/base/ObserverList.java @@ -0,0 +1,251 @@ +// Copyright 2013 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.base; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; + +import javax.annotation.concurrent.NotThreadSafe; + +/** + * A container for a list of observers. + *

+ * This container can be modified during iteration without invalidating the iterator. + * So, it safely handles the case of an observer removing itself or other observers from the list + * while observers are being notified. + *

+ * The implementation (and the interface) is heavily influenced by the C++ ObserverList. + * Notable differences: + * - The iterator implements NOTIFY_EXISTING_ONLY. + * - The range-based for loop is left to the clients to implement in terms of iterator(). + *

+ * This class is not threadsafe. Observers MUST be added, removed and will be notified on the same + * thread this is created. + * + * @param The type of observers that this list should hold. + */ +@NotThreadSafe +public class ObserverList implements Iterable { + /** Extended iterator interface that provides rewind functionality. */ + public interface RewindableIterator extends Iterator { + /** + * Rewind the iterator back to the beginning. + * + * If we need to iterate multiple times, we can avoid iterator object reallocation by using + * this method. + */ + public void rewind(); + } + + public final List mObservers = new ArrayList(); + private int mIterationDepth; + private int mCount; + private boolean mNeedsCompact; + + /** + * Add an observer to the list. + *

+ * An observer should not be added to the same list more than once. If an iteration is already + * in progress, this observer will be not be visible during that iteration. + * + * @return true if the observer list changed as a result of the call. + */ + public boolean addObserver(@Nullable E obs) { + // Avoid adding null elements to the list as they may be removed on a compaction. + if (obs == null || mObservers.contains(obs)) { + return false; + } + + // Structurally modifying the underlying list here. This means we + // cannot use the underlying list's iterator to iterate over the list. + boolean result = mObservers.add(obs); + assert result; + + ++mCount; + return true; + } + + /** + * Remove an observer from the list if it is in the list. + * + * @return true if an element was removed as a result of this call. + */ + public boolean removeObserver(@Nullable E obs) { + if (obs == null) { + return false; + } + + int index = mObservers.indexOf(obs); + if (index == -1) { + return false; + } + + if (mIterationDepth == 0) { + // No one is iterating over the list. + mObservers.remove(index); + } else { + mNeedsCompact = true; + mObservers.set(index, null); + } + --mCount; + assert mCount >= 0; + + return true; + } + + public boolean hasObserver(@Nullable E obs) { + if (obs == null) { + return false; + } + + return mObservers.contains(obs); + } + + public void clear() { + mCount = 0; + + if (mIterationDepth == 0) { + mObservers.clear(); + return; + } + + int size = mObservers.size(); + mNeedsCompact |= size != 0; + for (int i = 0; i < size; i++) { + mObservers.set(i, null); + } + } + + @NonNull + @Override + public Iterator iterator() { + return new ObserverListIterator(); + } + + /** + * It's the same as {@link ObserverList#iterator()} but the return type is + * {@link RewindableIterator}. Use this iterator type if you need to use + * {@link RewindableIterator#rewind()}. + */ + public RewindableIterator rewindableIterator() { + return new ObserverListIterator(); + } + + /** + * Returns the number of observers currently registered in the ObserverList. + * This is equivalent to the number of non-empty spaces in |mObservers|. + */ + public int size() { + return mCount; + } + + /** Returns true if the ObserverList contains no observers. */ + public boolean isEmpty() { + return mCount == 0; + } + + /** + * Compact the underlying list be removing null elements. + *

+ * Should only be called when mIterationDepth is zero. + */ + private void compact() { + assert mIterationDepth == 0; + for (int i = mObservers.size() - 1; i >= 0; i--) { + if (mObservers.get(i) == null) { + mObservers.remove(i); + } + } + } + + private void incrementIterationDepth() { + mIterationDepth++; + } + + private void decrementIterationDepthAndCompactIfNeeded() { + mIterationDepth--; + assert mIterationDepth >= 0; + if (mIterationDepth > 0) return; + if (!mNeedsCompact) return; + mNeedsCompact = false; + compact(); + } + + /** + * Returns the size of the underlying storage of the ObserverList. + * It will take into account the empty spaces inside |mObservers|. + */ + private int capacity() { + return mObservers.size(); + } + + private E getObserverAt(int index) { + return mObservers.get(index); + } + + private class ObserverListIterator implements RewindableIterator { + private int mListEndMarker; + private int mIndex; + private boolean mIsExhausted; + + private ObserverListIterator() { + ObserverList.this.incrementIterationDepth(); + mListEndMarker = ObserverList.this.capacity(); + } + + @Override + public void rewind() { + compactListIfNeeded(); + ObserverList.this.incrementIterationDepth(); + mListEndMarker = ObserverList.this.capacity(); + mIsExhausted = false; + mIndex = 0; + } + + @Override + public boolean hasNext() { + int lookupIndex = mIndex; + while (lookupIndex < mListEndMarker + && ObserverList.this.getObserverAt(lookupIndex) == null) { + lookupIndex++; + } + if (lookupIndex < mListEndMarker) return true; + + // We have reached the end of the list, allow for compaction. + compactListIfNeeded(); + return false; + } + + @Override + public E next() { + // Advance if the current element is null. + while (mIndex < mListEndMarker && ObserverList.this.getObserverAt(mIndex) == null) { + mIndex++; + } + if (mIndex < mListEndMarker) return ObserverList.this.getObserverAt(mIndex++); + + // We have reached the end of the list, allow for compaction. + compactListIfNeeded(); + throw new NoSuchElementException(); + } + + @Override + public void remove() { + throw new UnsupportedOperationException(); + } + + private void compactListIfNeeded() { + if (!mIsExhausted) { + mIsExhausted = true; + ObserverList.this.decrementIterationDepthAndCompactIfNeeded(); + } + } + } +} diff --git a/android/app/src/test/java/org/chromium/base/ObserverListTest.java b/android/app/src/test/java/org/chromium/base/ObserverListTest.java new file mode 100644 index 0000000000..aa4ac917e6 --- /dev/null +++ b/android/app/src/test/java/org/chromium/base/ObserverListTest.java @@ -0,0 +1,313 @@ +// Copyright 2017 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.base; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.Collection; +import java.util.Iterator; +import java.util.NoSuchElementException; + +/** Tests for (@link ObserverList}. */ +public class ObserverListTest { + interface Observer { + void observe(int x); + } + + private static class Foo implements Observer { + private final int mScalar; + private int mTotal; + + Foo(int scalar) { + mScalar = scalar; + } + + @Override + public void observe(int x) { + mTotal += x * mScalar; + } + } + + /** An observer which add a given Observer object to the list when observe is called. */ + private static class FooAdder implements Observer { + private final ObserverList mList; + private final Observer mLucky; + + FooAdder(ObserverList list, Observer oblivious) { + mList = list; + mLucky = oblivious; + } + + @Override + public void observe(int x) { + mList.addObserver(mLucky); + } + } + + /** An observer which removes a given Observer object from the list when observe is called. */ + private static class FooRemover implements Observer { + private final ObserverList mList; + private final Observer mDoomed; + + FooRemover(ObserverList list, Observer innocent) { + mList = list; + mDoomed = innocent; + } + + @Override + public void observe(int x) { + mList.removeObserver(mDoomed); + } + } + + private static int getSizeOfIterable(Iterable iterable) { + if (iterable instanceof Collection) return ((Collection) iterable).size(); + int num = 0; + for (T el : iterable) num++; + return num; + } + + @Test + public void testRemoveWhileIteration() { + ObserverList observerList = new ObserverList(); + Foo a = new Foo(1); + Foo b = new Foo(-1); + Foo c = new Foo(1); + Foo d = new Foo(-1); + Foo e = new Foo(-1); + FooRemover evil = new FooRemover(observerList, c); + + observerList.addObserver(a); + observerList.addObserver(b); + + for (Observer obs : observerList) obs.observe(10); + + // Removing an observer not in the list should do nothing. + observerList.removeObserver(e); + + observerList.addObserver(evil); + observerList.addObserver(c); + observerList.addObserver(d); + + for (Observer obs : observerList) obs.observe(10); + + // observe should be called twice on a. + Assert.assertEquals(20, a.mTotal); + // observe should be called twice on b. + Assert.assertEquals(-20, b.mTotal); + // evil removed c from the observerList before it got any callbacks. + Assert.assertEquals(0, c.mTotal); + // observe should be called once on d. + Assert.assertEquals(-10, d.mTotal); + // e was never added to the list, observe should not be called. + Assert.assertEquals(0, e.mTotal); + } + + @Test + public void testAddWhileIteration() { + ObserverList observerList = new ObserverList(); + Foo a = new Foo(1); + Foo b = new Foo(-1); + Foo c = new Foo(1); + FooAdder evil = new FooAdder(observerList, c); + + observerList.addObserver(evil); + observerList.addObserver(a); + observerList.addObserver(b); + + for (Observer obs : observerList) obs.observe(10); + + Assert.assertTrue(observerList.hasObserver(c)); + Assert.assertEquals(10, a.mTotal); + Assert.assertEquals(-10, b.mTotal); + Assert.assertEquals(0, c.mTotal); + } + + @Test + public void testIterator() { + ObserverList observerList = new ObserverList(); + observerList.addObserver(5); + observerList.addObserver(10); + observerList.addObserver(15); + Assert.assertEquals(3, getSizeOfIterable(observerList)); + + observerList.removeObserver(10); + Assert.assertEquals(2, getSizeOfIterable(observerList)); + + Iterator it = observerList.iterator(); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(5, (int) it.next()); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(15, (int) it.next()); + Assert.assertFalse(it.hasNext()); + + boolean removeExceptionThrown = false; + try { + it.remove(); + Assert.fail("Expecting UnsupportedOperationException to be thrown here."); + } catch (UnsupportedOperationException e) { + removeExceptionThrown = true; + } + Assert.assertTrue(removeExceptionThrown); + Assert.assertEquals(2, getSizeOfIterable(observerList)); + + boolean noElementExceptionThrown = false; + try { + it.next(); + Assert.fail("Expecting NoSuchElementException to be thrown here."); + } catch (NoSuchElementException e) { + noElementExceptionThrown = true; + } + Assert.assertTrue(noElementExceptionThrown); + } + + @Test + public void testRewindableIterator() { + ObserverList observerList = new ObserverList(); + observerList.addObserver(5); + observerList.addObserver(10); + observerList.addObserver(15); + Assert.assertEquals(3, getSizeOfIterable(observerList)); + + ObserverList.RewindableIterator it = observerList.rewindableIterator(); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(5, (int) it.next()); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(10, (int) it.next()); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(15, (int) it.next()); + Assert.assertFalse(it.hasNext()); + + it.rewind(); + + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(5, (int) it.next()); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(10, (int) it.next()); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(15, (int) it.next()); + Assert.assertEquals(5, (int) observerList.mObservers.get(0)); + observerList.removeObserver(5); + Assert.assertEquals(null, observerList.mObservers.get(0)); + + it.rewind(); + + Assert.assertEquals(10, (int) observerList.mObservers.get(0)); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(10, (int) it.next()); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(15, (int) it.next()); + } + + @Test + public void testAddObserverReturnValue() { + ObserverList observerList = new ObserverList(); + + Object a = new Object(); + Assert.assertTrue(observerList.addObserver(a)); + Assert.assertFalse(observerList.addObserver(a)); + + Object b = new Object(); + Assert.assertTrue(observerList.addObserver(b)); + Assert.assertFalse(observerList.addObserver(null)); + } + + @Test + public void testRemoveObserverReturnValue() { + ObserverList observerList = new ObserverList(); + + Object a = new Object(); + Object b = new Object(); + observerList.addObserver(a); + observerList.addObserver(b); + + Assert.assertTrue(observerList.removeObserver(a)); + Assert.assertFalse(observerList.removeObserver(a)); + Assert.assertFalse(observerList.removeObserver(new Object())); + Assert.assertTrue(observerList.removeObserver(b)); + Assert.assertFalse(observerList.removeObserver(null)); + + // If we remove an object while iterating, it will be replaced by 'null'. + observerList.addObserver(a); + Assert.assertTrue(observerList.removeObserver(a)); + Assert.assertFalse(observerList.removeObserver(null)); + } + + @Test + public void testSize() { + ObserverList observerList = new ObserverList(); + + Assert.assertEquals(0, observerList.size()); + Assert.assertTrue(observerList.isEmpty()); + + observerList.addObserver(null); + Assert.assertEquals(0, observerList.size()); + Assert.assertTrue(observerList.isEmpty()); + + Object a = new Object(); + observerList.addObserver(a); + Assert.assertEquals(1, observerList.size()); + Assert.assertFalse(observerList.isEmpty()); + + observerList.addObserver(a); + Assert.assertEquals(1, observerList.size()); + Assert.assertFalse(observerList.isEmpty()); + + observerList.addObserver(null); + Assert.assertEquals(1, observerList.size()); + Assert.assertFalse(observerList.isEmpty()); + + Object b = new Object(); + observerList.addObserver(b); + Assert.assertEquals(2, observerList.size()); + Assert.assertFalse(observerList.isEmpty()); + + observerList.removeObserver(null); + Assert.assertEquals(2, observerList.size()); + Assert.assertFalse(observerList.isEmpty()); + + observerList.removeObserver(new Object()); + Assert.assertEquals(2, observerList.size()); + Assert.assertFalse(observerList.isEmpty()); + + observerList.removeObserver(b); + Assert.assertEquals(1, observerList.size()); + Assert.assertFalse(observerList.isEmpty()); + + observerList.removeObserver(b); + Assert.assertEquals(1, observerList.size()); + Assert.assertFalse(observerList.isEmpty()); + + observerList.removeObserver(a); + Assert.assertEquals(0, observerList.size()); + Assert.assertTrue(observerList.isEmpty()); + + observerList.removeObserver(a); + observerList.removeObserver(b); + observerList.removeObserver(null); + observerList.removeObserver(new Object()); + Assert.assertEquals(0, observerList.size()); + Assert.assertTrue(observerList.isEmpty()); + + observerList.addObserver(new Object()); + observerList.addObserver(new Object()); + observerList.addObserver(new Object()); + observerList.addObserver(a); + Assert.assertEquals(4, observerList.size()); + Assert.assertFalse(observerList.isEmpty()); + + observerList.clear(); + Assert.assertEquals(0, observerList.size()); + Assert.assertTrue(observerList.isEmpty()); + + observerList.removeObserver(a); + observerList.removeObserver(b); + observerList.removeObserver(null); + observerList.removeObserver(new Object()); + Assert.assertEquals(0, observerList.size()); + Assert.assertTrue(observerList.isEmpty()); + } +}