[Presentation API, Android] Implement starting a presentation via CreateRoute()

[Presentation API, Android] Implement starting a presentation via CreateRoute()

BUG=517093
Committed: https://crrev.com/edc3bce323739c05b499d234edf00ca36f4847b9
Cr-Commit-Position: refs/heads/master@{#344225}

[whywhat, 2015-08-18 14:01:03]

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

[whywhat, 2015-08-18 14:01:04]

PTaL

[whywhat, 2015-08-18 14:01:08]

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

[commit-bot: I haz the power, 2015-08-18 14:01:29]

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

[commit-bot: I haz the power, 2015-08-18 14:24:36]

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

[commit-bot: I haz the power, 2015-08-18 14:24:37]

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...)

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

lgtm with comments applied

https://codereview.chromium.org/1301773002/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/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouter.java:174:
nativeOnRouteCreationError(mNativeMediaRouterAndroid, "Not supported",
requestId);
return?

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouter.java:180:
// TODO(avayvod): handle application disconnect and report back to the native
side.
nit: bug number

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

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:95:
if (mState != STATE_IDLE) throwInvalidState();
nit: empty line after condition

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:95:
if (mState != STATE_IDLE) throwInvalidState();
nit: empty line after condition

What happens if start() is called twice in JS?

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:100:
mRoute = null;
nit: empty line before mRoute

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:108:
if (mRoute == null) {
nit: empty line between for() and if()

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:112:
mApiClient = createApiClient(mRoute, castApplicationListener,
applicationContext);
nit: empty line after if()

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:123:
// TODO(avayvod): figure out what needs to be reset at this point and reconnect
nit: bug number

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:126:
try {
nit: empty line before try{}

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:139:
// TODO(avayvod): what to suspend here to be reconnected in {@link
onConnected()}?
nit: bug number?

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:145:
Status status = result.getStatus();
nit: empty line after condition

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:146:
if (status.isSuccess()) {
nit:
if (!status.isSuccess()) {
    reportError();
    return;
}

// do the real thing

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:150:
// result.getApplicationMetadata();
shrug...

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:166:
if (mState != STATE_CONNECTING_TO_API) throwInvalidState();
nit: empty line after condition

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:168:
// try to resolve the problem (e.g. ask the user to update GPS).
nit: add bug number.

https://codereview.chromium.org/1301773002/diff/1/chrome/browser/media/androi...
File chrome/browser/media/android/router/media_router_android.cc (right):

https://codereview.chromium.org/1301773002/diff/1/chrome/browser/media/androi...
chrome/browser/media/android/router/media_router_android.cc:62: //
TODO(avayvod): unify presentation id generation code between platforms.
nit: add bug number.

https://codereview.chromium.org/1301773002/diff/1/chrome/browser/media/androi...
chrome/browser/media/android/router/media_router_android.cc:227: return;
DCHECK?

https://codereview.chromium.org/1301773002/diff/1/chrome/browser/media/androi...
chrome/browser/media/android/router/media_router_android.cc:251: return;
DCHECK?

[whywhat, 2015-08-18 20:06:33]

https://codereview.chromium.org/1301773002/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/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouter.java:174:
nativeOnRouteCreationError(mNativeMediaRouterAndroid, "Not supported",
requestId);
On 2015/08/18 at 16:04:54, Mounir Lamouri wrote:
> return?

Done.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouter.java:180:
// TODO(avayvod): handle application disconnect and report back to the native
side.
On 2015/08/18 at 16:04:54, Mounir Lamouri wrote:
> nit: bug number

Done. This will be needed for the route monitoring.

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

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:95:
if (mState != STATE_IDLE) throwInvalidState();
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> nit: empty line after condition
> 
> What happens if start() is called twice in JS?

Done.
PresentationServiceImpl will reject the consequent start while one request is in
process.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:100:
mRoute = null;
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> nit: empty line before mRoute

Done.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:108:
if (mRoute == null) {
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> nit: empty line between for() and if()

Done.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:112:
mApiClient = createApiClient(mRoute, castApplicationListener,
applicationContext);
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> nit: empty line after if()

Done.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:123:
// TODO(avayvod): figure out what needs to be reset at this point and reconnect
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> nit: bug number

Updated the comment. We don't need to have all this connection/reconnection
logic - it all should be handles by the Google Play Services now. Will look into
removing unnecessary code tomorrow.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:126:
try {
On 2015/08/18 at 16:04:54, Mounir Lamouri wrote:
> nit: empty line before try{}

Done.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:139:
// TODO(avayvod): what to suspend here to be reconnected in {@link
onConnected()}?
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> nit: bug number?

Ditto

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:145:
Status status = result.getStatus();
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> nit: empty line after condition

Done.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:146:
if (status.isSuccess()) {
On 2015/08/18 at 16:04:54, Mounir Lamouri wrote:
> nit:
> if (!status.isSuccess()) {
>     reportError();
>     return;
> }
> 
> // do the real thing

Done.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:150:
// result.getApplicationMetadata();
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> shrug...

Removed.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:166:
if (mState != STATE_CONNECTING_TO_API) throwInvalidState();
On 2015/08/18 at 16:04:54, Mounir Lamouri wrote:
> nit: empty line after condition

Done.

https://codereview.chromium.org/1301773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:168:
// try to resolve the problem (e.g. ask the user to update GPS).
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> nit: add bug number.

Ditto. Hopefully will remove altogether.

https://codereview.chromium.org/1301773002/diff/1/chrome/browser/media/androi...
File chrome/browser/media/android/router/media_router_android.cc (right):

https://codereview.chromium.org/1301773002/diff/1/chrome/browser/media/androi...
chrome/browser/media/android/router/media_router_android.cc:62: //
TODO(avayvod): unify presentation id generation code between platforms.
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> nit: add bug number.

Will send a patch.

https://codereview.chromium.org/1301773002/diff/1/chrome/browser/media/androi...
chrome/browser/media/android/router/media_router_android.cc:227: return;
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> DCHECK?

Done.

https://codereview.chromium.org/1301773002/diff/1/chrome/browser/media/androi...
chrome/browser/media/android/router/media_router_android.cc:251: return;
On 2015/08/18 at 16:04:55, Mounir Lamouri wrote:
> DCHECK?

Done.

[whywhat, 2015-08-18 20:06:40]

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:07:42]

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

[commit-bot: I haz the power, 2015-08-18 20:55:06]

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

[commit-bot: I haz the power, 2015-08-18 20:55:07]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[whywhat, 2015-08-19 13:54:46]

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:09]

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

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

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

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

Dry run: This issue passed the CQ dry run.

[whywhat, 2015-08-19 15:12:54]

The CQ bit was checked by avayvod@chromium.org

[whywhat, 2015-08-19 15:12:55]

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/1301773002/#ps60001
(title: "Added a bug for GoogleApiClient connection, unregister connection
callbacks.")

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

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

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

Committed patchset #4 (id:60001)

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

Patchset 4 (id:??) landed as
https://crrev.com/edc3bce323739c05b499d234edf00ca36f4847b9
Cr-Commit-Position: refs/heads/master@{#344225}