[Chromecast] Controllers drop duplicate set() calls.

This makes Controller<Unit> behavior a bit less surprising and
is probably a better fit for some applications of
Observable#unique().

Bug: None
Test: cast_base_junit_tests
Change-Id: Ic66544b5db6970df1bada2fd0925b5bfe4ab869b
Reviewed-on: https://chromium-review.googlesource.com/1114203Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Commit-Queue: Simeon Anfinrud <sanfin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570208}

chromecast/base/java/src/org/chromium/chromecast/base/Controller.java

@@ -57,8 +57,13 @@ public class Controller<T> extends Observable<T> {
             }
             // If this Controller was already set(), call reset() so observing Scopes can clean up.
             if (mData != null) {
+                // If this Controller was already set() with this data, no-op.
+                if (mData.equals(data)) {
+                    return;
+                }
                 resetInternal();
             }
+
             mData = data;
             for (ScopeFactory<? super T> observer : mEnterObservers) {
                 notifyEnter(observer);

chromecast/base/java/test/org/chromium/chromecast/base/ObservableAndControllerTest.java

@@ -694,6 +694,25 @@ public class ObservableAndControllerTest {
         assertThat(result, contains("it worked!"));
     }
 
+    @Test
+    public void testSetWithDuplicateValueIsNoOp() {
+        Controller<String> controller = new Controller<>();
+        ReactiveRecorder recorder = ReactiveRecorder.record(controller);
+        controller.set("stop copying me");
+        controller.set("stop copying me");
+        recorder.verify().entered("stop copying me").end();
+    }
+
+    @Test
+    public void testSetUnitControllerInActivatedStateIsNoOp() {
+        Controller<Unit> controller = new Controller<>();
+        ReactiveRecorder recorder = ReactiveRecorder.record(controller);
+        controller.set(Unit.unit());
+        recorder.verify().entered(Unit.unit()).end();
+        controller.set(Unit.unit());
+        recorder.verify().end();
+    }
+
     // Any Scope's constructor whose parameters match the scope can be used as a method reference.
     private static class TransitionLogger implements Scope {
         public static final List<String> sResult = new ArrayList<>();

chromecast/base/reactive_java.md

@@ -300,8 +300,10 @@ Here are some guarantees that `Controller`s provide:
 *   If in the activated state, `reset()` and `set(null)` enter the *deactivated*
     state.
 *   If already in the deactivated state, `reset()` and `set(null)` do nothing.
-*   If in the activated state, `set()` implicitly deactivates and reactivates
-    with the new data.
+*   If in the activated state with data `data1`, `set(data2)` no-ops if
+    `data1.equals(data2)`.
+*   If in the activated state, and the new data is not `equal()` to the current
+    data, `set()` implicitly deactivates and reactivates with the new data.
 
 As corollaries, any registered `ScopeFactory` objects will:
 
@@ -369,6 +371,10 @@ Since `Unit` means "no data," and there's only one way to get a `Unit` instance
 (through the `Unit.unit()` method), this maps correctly to the concept of a
 mutable boolean value.
 
+Note that because the instance of `Unit` equals itself, calling `set()` on a
+`Controller<Unit>` when it is already activated will no-op, making the behavior
+of `set(Unit.unit())` and `reset()` symmetric.
+
 Example:
 
 ```java
@@ -379,7 +385,9 @@ Example:
         return () -> Log.d(TAG, "off");
     });
     onOrOff.set(Unit.unit()); // Turns on.
+    onOrOff.set(Unit.unit()); // Does nothing because it's already on.
     onOrOff.reset(); // Turns off.
+    onOrOff.reset(); // Does nothing because it's already off.
 }
 ```