[Presentation API, Android] Implement closing the route

[Presentation API, Android] Implement closing the route

BUG=517095
Committed: https://crrev.com/6122247173a92bd4cdc7f0c6bbeea6ad2f6d63f2
Cr-Commit-Position: refs/heads/master@{#344229}

[whywhat, 2015-08-18 16:21:28]

avayvod@chromium.org changed reviewers:
+ mlamouri@chromium.org

[whywhat, 2015-08-18 16:21:29]

PTaL

[whywhat, 2015-08-18 16:22:03]

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

[commit-bot: I haz the power, 2015-08-18 16:22:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1292853003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1292853003/1

[mlamouri (slow - plz ping), 2015-08-18 16:46:04]

lgtm with comments addressed.

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouter.java
(right):

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouter.java:193:
if (mAndroidMediaRouter != null) {
Is it expected to have it set to null? IOW, should we assert?

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/SessionWrapper.java
(right):

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/SessionWrapper.java:16:
private String mSessionId;
nit: maybe mId? up to you

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/SessionWrapper.java:33:
Cast.CastApi.stopApplication(apiClient, sessionId);
What happens if you call stopApplication() with a null mApiClient/mSessionId or
with a not connected/connecting apiClient?

IOW, could we let the Cast API take care of these checks if it already does
[some of] them?

[commit-bot: I haz the power, 2015-08-18 16:58:55]

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

[commit-bot: I haz the power, 2015-08-18 16:58:56]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[whywhat, 2015-08-18 20:55:32]

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouter.java
(right):

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouter.java:193:
if (mAndroidMediaRouter != null) {
On 2015/08/18 at 16:46:04, Mounir Lamouri wrote:
> Is it expected to have it set to null? IOW, should we assert?

Yes, it's null when we are on a bad version of JB (MR0?) where MediaRouter
compat library doesn't work.
There's no way to report this to the Blink side today (ideally we would disable
the API completely).
So I just make everything a no-op if it's null.

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/SessionWrapper.java
(right):

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/SessionWrapper.java:16:
private String mSessionId;
On 2015/08/18 at 16:46:04, Mounir Lamouri wrote:
> nit: maybe mId? up to you

It would make sense if I would use it outside of the wrapper (e.g.
sessionWrapper.getId() or sessionWrapper.mId), but it's used within the class
and Cast.CastApi.stopApplication(apiClient, mSessionId) looks better to me than
Cast.CastApi.stopApplication(apiClient, mId).

https://codereview.chromium.org/1292853003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/SessionWrapper.java:33:
Cast.CastApi.stopApplication(apiClient, sessionId);
On 2015/08/18 at 16:46:04, Mounir Lamouri wrote:
> What happens if you call stopApplication() with a null mApiClient/mSessionId
or with a not connected/connecting apiClient?
> 
> IOW, could we let the Cast API take care of these checks if it already does
[some of] them?

I think it does check. Removed.

[whywhat, 2015-08-18 20:56:35]

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

[commit-bot: I haz the power, 2015-08-18 20:57:31]

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

[commit-bot: I haz the power, 2015-08-18 22:14:58]

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

[commit-bot: I haz the power, 2015-08-18 22:14:58]

Dry run: This issue passed the CQ dry run.

[whywhat, 2015-08-19 13:43:41]

merge

[whywhat, 2015-08-19 13:55:03]

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

[commit-bot: I haz the power, 2015-08-19 13:55:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1292853003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1292853003/40001

[commit-bot: I haz the power, 2015-08-19 14:37:35]

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

[commit-bot: I haz the power, 2015-08-19 14:37:36]

Dry run: This issue passed the CQ dry run.

[whywhat, 2015-08-19 15:19:05]

The CQ bit was checked by avayvod@chromium.org

[whywhat, 2015-08-19 15:19:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1292853003/#ps40001
(title: "merge")

[commit-bot: I haz the power, 2015-08-19 15:19:35]

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

[commit-bot: I haz the power, 2015-08-19 15:24:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-08-19 15:25:20]

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