Add (Robolectric) unit tests for media.remote.TransportControl

Add (Robolectric) unit tests for media.remote.TransportControl

Also some minor bug fixes.

BUG=472067
Committed: https://crrev.com/5cc50b558cfa7ee06de13e3449c3d029c63e003f
Cr-Commit-Position: refs/heads/master@{#326089}

[aberent, 2015-04-15 17:40:44]

aberent@chromium.org changed reviewers:
+ avayvod@chromium.org, dgn@chromium.org, nyquist@chromium.org

[aberent, 2015-04-15 17:40:44]

avayod@ - full review please
nyquist@ - OWNER review for tests please

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/media/remote/TransportControl.java
(left):

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/media/remote/TransportControl.java:35:
private final Set<Listener> mListeners = new CopyOnWriteArraySet<Listener>();
Made lazy for testability, since Mockito doesn't call mocked class constructors.

[whywhat, 2015-04-20 10:32:43]

lgtm

[aberent, 2015-04-20 11:05:14]

The CQ bit was checked by aberent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-20 11:05:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1052403005/20001

[commit-bot: I haz the power, 2015-04-20 11:38:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-20 11:38:04]

Dry run: This issue passed the CQ dry run.

[nyquist, 2015-04-21 17:18:21]

lgtm

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/media/remote/TransportControl.java
(right):

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/media/remote/TransportControl.java:35:
private Set<Listener> mListeners;
Nit: Could you add a comment that this should always be accessed through
getListeners()?

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/junit/sr...
File
chrome/android/junit/src/org/chromium/chrome/browser/media/remote/TransportControlTest.java
(right):

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/media/remote/TransportControlTest.java:71:
// Check that onScreenNameChanged is called precisely once
onErrorChanged

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/media/remote/TransportControlTest.java:123:
assertThat("getVideoInfo should return the last screen name set",
last videoInfo set

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/media/remote/TransportControlTest.java:138:
Color.RED };
Nit: could this be on the previous line?

[aberent, 2015-04-21 18:15:33]

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/media/remote/TransportControl.java
(right):

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/media/remote/TransportControl.java:35:
private Set<Listener> mListeners;
On 2015/04/21 17:18:21, nyquist wrote:
> Nit: Could you add a comment that this should always be accessed through
> getListeners()?

Done.

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/junit/sr...
File
chrome/android/junit/src/org/chromium/chrome/browser/media/remote/TransportControlTest.java
(right):

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/media/remote/TransportControlTest.java:71:
// Check that onScreenNameChanged is called precisely once
On 2015/04/21 17:18:21, nyquist wrote:
> onErrorChanged

Done.

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/media/remote/TransportControlTest.java:123:
assertThat("getVideoInfo should return the last screen name set",
On 2015/04/21 17:18:21, nyquist wrote:
> last videoInfo set

Done.

https://codereview.chromium.org/1052403005/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/media/remote/TransportControlTest.java:138:
Color.RED };
On 2015/04/21 17:18:21, nyquist wrote:
> Nit: could this be on the previous line?

Done.

[aberent, 2015-04-21 18:15:43]

The CQ bit was checked by aberent@chromium.org

[aberent, 2015-04-21 18:15:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
nyquist@chromium.org, avayvod@chromium.org
Link to the patchset: https://codereview.chromium.org/1052403005/#ps40001
(title: "Fix nits")

[commit-bot: I haz the power, 2015-04-21 18:17:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1052403005/40001

[commit-bot: I haz the power, 2015-04-21 19:07:44]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-21 19:10:03]

Patchset 3 (id:??) landed as
https://crrev.com/5cc50b558cfa7ee06de13e3449c3d029c63e003f
Cr-Commit-Position: refs/heads/master@{#326089}