Commit 3dacc09d authored by Simeon Anfinrud's avatar Simeon Anfinrud Committed by Commit Bot

[Chromecast] Break dependency of Controller on ObserverList.

The special properties of ObserverList were not needed because
of how Controller uses a Sequencer to avoid concurrent
modification. This also avoids heap allocation of iterators and
the use of the O(n) memory Itertools.reverse() when closing
scopes.

Bug: None
Test: cast_base_junit_tests
Change-Id: I91ef516caf07de8b9a899c17b015f2e2838fee35
Reviewed-on: https://chromium-review.googlesource.com/1191087Reviewed-by: default avatarLuke Halliwell <halliwell@chromium.org>
Commit-Queue: Simeon Anfinrud <sanfin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586378}
parent 52531c3d
......@@ -4,10 +4,10 @@
package org.chromium.chromecast.base;
import org.chromium.base.ObserverList;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
/**
......@@ -21,19 +21,19 @@ import java.util.Map;
*/
public class Controller<T> extends Observable<T> {
private final Sequencer mSequencer = new Sequencer();
private final ObserverList<Observer<? super T>> mEnterObservers = new ObserverList<>();
private final List<Observer<? super T>> mObservers = new ArrayList<>();
private final Map<Observer<? super T>, Scope> mScopeMap = new HashMap<>();
private T mData = null;
@Override
public Scope watch(Observer<? super T> observer) {
mSequencer.sequence(() -> {
mEnterObservers.addObserver(observer);
mObservers.add(observer);
if (mData != null) notifyEnter(observer);
});
return () -> mSequencer.sequence(() -> {
if (mData != null) notifyExit(observer);
mEnterObservers.removeObserver(observer);
mObservers.remove(observer);
});
}
......@@ -65,8 +65,8 @@ public class Controller<T> extends Observable<T> {
}
mData = data;
for (Observer<? super T> observer : mEnterObservers) {
notifyEnter(observer);
for (int i = 0; i < mObservers.size(); i++) {
notifyEnter(mObservers.get(i));
}
});
}
......@@ -88,8 +88,8 @@ public class Controller<T> extends Observable<T> {
assert mSequencer.inSequence();
if (mData == null) return;
mData = null;
for (Observer<? super T> observer : Itertools.reverse(mEnterObservers)) {
notifyExit(observer);
for (int i = mObservers.size() - 1; i >= 0; i--) {
notifyExit(mObservers.get(i));
}
}
......
......@@ -5,48 +5,11 @@
package org.chromium.chromecast.base;
import java.util.Iterator;
import java.util.Stack;
/**
* Utility methods for working with Iterables and Iterators.
*/
public class Itertools {
/**
* Iterate the given Iterable in reverse.
*
* This is inefficient, as the entire given Iterable needs to be iterated before the result can
* be iterated, and O(n) memory needs to be allocated out-of-place to store the stack. However,
* for small, simple tasks, like iterating an ObserverList in reverse, this is a useful way to
* concisely iterate a container in reverse order in a for-each loop.
*
* Example:
*
* List<String> lines = ...;
* // Prints lines in reverse order that they were added to the list.
* for (String line : Itertools.reverse(lines)) {
* Log.i(TAG, line);
* }
*/
public static <T> Iterable<T> reverse(Iterable<? extends T> iterable) {
return (() -> {
// Lazily iterate `iterable` only after reverse()'s `iterator()` method is called.
Stack<T> stack = new Stack<T>();
for (T item : iterable) {
stack.push(item);
}
return new Iterator<T>() {
@Override
public boolean hasNext() {
return !stack.empty();
}
@Override
public T next() {
return stack.pop();
}
};
});
}
/**
* Create an Iterable from an Iterator.
*
......
......@@ -4,7 +4,6 @@
package org.chromium.chromecast.base;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.emptyIterable;
import static org.junit.Assert.assertThat;
......@@ -12,9 +11,6 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.BlockJUnit4ClassRunner;
import org.chromium.chromecast.base.Inheritance.Base;
import org.chromium.chromecast.base.Inheritance.Derived;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
......@@ -24,26 +20,6 @@ import java.util.List;
*/
@RunWith(BlockJUnit4ClassRunner.class)
public class ItertoolsTest {
@Test
public void testReverse() {
List<String> emptyList = new ArrayList<String>();
assertThat(Itertools.reverse(emptyList), emptyIterable());
List<String> singleItem = new ArrayList<>();
singleItem.add("a");
assertThat(Itertools.reverse(singleItem), contains("a"));
List<String> threeItems = new ArrayList<>();
threeItems.add("a");
threeItems.add("b");
threeItems.add("c");
assertThat(Itertools.reverse(threeItems), contains("c", "b", "a"));
}
@Test
public void testAssignReversedToIterableOfSuperclass() {
// Compile error if the generics are wrong.
Iterable<Base> reversed = Itertools.reverse(new ArrayList<Derived>());
}
@Test
public void testForEachLoopWithIterator() {
Iterator<String> emptyIterator = new Iterator<String>() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment