[Media Router WebUI] Move replace route responsibility to extension

[Media Router WebUI] Move replace route responsibility to extension

Currently the MR dialog controls the replace route functionality by
stopping the current route and starting a new one when that completes.
This doesn't allow the extension to optimize the change of source. This
change replaces the current UI behavior by simply calling CreateRoute
with the same sink and new source. This requires an extension change to
support accepting CreateRoute calls to a sink with an existing route.

BUG=614144
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/bbc838f3a5ed945462a29818591753e2cdb04f5c
Cr-Commit-Position: refs/heads/master@{#410607}

[btolsch, 2016-07-11 20:15:11]

Description was changed from

==========
[Media Router WebUI] Move replace route responsibility to extension

Currently the MR dialog controls the replace route functionality by
stopping the current route and starting a new one when that completes.
This doesn't allow the extension to optimize the change of source. This
change replaces the current UI behavior by simply calling CreateRoute
with the same sink and new source. This requires an extension change to
support accepting CreateRoute calls to a sink with an existing route.

BUG=614144
==========

to

==========
[Media Router WebUI] Move replace route responsibility to extension

Currently the MR dialog controls the replace route functionality by
stopping the current route and starting a new one when that completes.
This doesn't allow the extension to optimize the change of source. This
change replaces the current UI behavior by simply calling CreateRoute
with the same sink and new source. This requires an extension change to
support accepting CreateRoute calls to a sink with an existing route.

BUG=614144
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[btolsch, 2016-07-25 18:44:25]

btolsch@chromium.org changed reviewers:
+ imcheng@chromium.org

[btolsch, 2016-07-25 18:44:26]

It looks like there's no major objections to the slightly-modified CreateRoute
semantics so PTAL at the chromium side.  Thanks!

[imcheng, 2016-07-27 20:30:37]

https://codereview.chromium.org/2142613002/diff/1/chrome/browser/resources/me...
File
chrome/browser/resources/media_router/elements/route_details/route_details.js
(right):

https://codereview.chromium.org/2142613002/diff/1/chrome/browser/resources/me...
chrome/browser/resources/media_router/elements/route_details/route_details.js:139:
if (!sink) {
Is this case possible since sink has to be non-null for the option to show up in
the UI? What does returning 0 mean?

[btolsch, 2016-07-27 22:16:41]

https://codereview.chromium.org/2142613002/diff/1/chrome/browser/resources/me...
File
chrome/browser/resources/media_router/elements/route_details/route_details.js
(right):

https://codereview.chromium.org/2142613002/diff/1/chrome/browser/resources/me...
chrome/browser/resources/media_router/elements/route_details/route_details.js:139:
if (!sink) {
On 2016/07/27 20:30:37, imcheng wrote:
> Is this case possible since sink has to be non-null for the option to show up
in
> the UI? What does returning 0 mean?

0 means there is no cast mode selected/available to be selected and it affects
whether the replace route button can be used.  The sink can be null if there is
only one local route, so the dialog opens to route details, but the current cast
modes don't support the sink for that route.

I only expect this to happen for DIAL.  For example, cast a youtube video to a
DIAL tv, then go to a new tab and open the MR dialog.  The DIAL sink won't be in
the sink list at all because it isn't available for any of the mirroring urn's
(which will be the only observed sources).

[imcheng, 2016-08-01 17:36:31]

lgtm

https://codereview.chromium.org/2142613002/diff/1/chrome/browser/resources/me...
File
chrome/browser/resources/media_router/elements/route_details/route_details.js
(right):

https://codereview.chromium.org/2142613002/diff/1/chrome/browser/resources/me...
chrome/browser/resources/media_router/elements/route_details/route_details.js:139:
if (!sink) {
On 2016/07/27 22:16:41, btolsch wrote:
> On 2016/07/27 20:30:37, imcheng wrote:
> > Is this case possible since sink has to be non-null for the option to show
up
> in
> > the UI? What does returning 0 mean?
> 
> 0 means there is no cast mode selected/available to be selected and it affects
> whether the replace route button can be used.  The sink can be null if there
is
> only one local route, so the dialog opens to route details, but the current
cast
> modes don't support the sink for that route.
> 
> I only expect this to happen for DIAL.  For example, cast a youtube video to a
> DIAL tv, then go to a new tab and open the MR dialog.  The DIAL sink won't be
in
> the sink list at all because it isn't available for any of the mirroring urn's
> (which will be the only observed sources).

So firing the change-route-source-click event with selectedCastMode = 0 is not a
valid call. So maybe we should add a check for the returned value of
computeSelectedCastMode_ before firing the event in L161.

[btolsch, 2016-08-02 19:20:16]

Description was changed from

==========
[Media Router WebUI] Move replace route responsibility to extension

Currently the MR dialog controls the replace route functionality by
stopping the current route and starting a new one when that completes.
This doesn't allow the extension to optimize the change of source. This
change replaces the current UI behavior by simply calling CreateRoute
with the same sink and new source. This requires an extension change to
support accepting CreateRoute calls to a sink with an existing route.

BUG=614144
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router WebUI] Move replace route responsibility to extension

Currently the MR dialog controls the replace route functionality by
stopping the current route and starting a new one when that completes.
This doesn't allow the extension to optimize the change of source. This
change replaces the current UI behavior by simply calling CreateRoute
with the same sink and new source. This requires an extension change to
support accepting CreateRoute calls to a sink with an existing route.

BUG=614144
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

[btolsch, 2016-08-02 19:23:39]

Added some comments around the route details cast button/mode logic.

FYI, this needs the extension-side patch to be in first so I will submit this
after the extension patch is cherry picked and released.

https://codereview.chromium.org/2142613002/diff/1/chrome/browser/resources/me...
File
chrome/browser/resources/media_router/elements/route_details/route_details.js
(right):

https://codereview.chromium.org/2142613002/diff/1/chrome/browser/resources/me...
chrome/browser/resources/media_router/elements/route_details/route_details.js:139:
if (!sink) {
On 2016/08/01 17:36:31, imcheng wrote:
> On 2016/07/27 22:16:41, btolsch wrote:
> > On 2016/07/27 20:30:37, imcheng wrote:
> > > Is this case possible since sink has to be non-null for the option to show
> up
> > in
> > > the UI? What does returning 0 mean?
> > 
> > 0 means there is no cast mode selected/available to be selected and it
affects
> > whether the replace route button can be used.  The sink can be null if there
> is
> > only one local route, so the dialog opens to route details, but the current
> cast
> > modes don't support the sink for that route.
> > 
> > I only expect this to happen for DIAL.  For example, cast a youtube video to
a
> > DIAL tv, then go to a new tab and open the MR dialog.  The DIAL sink won't
be
> in
> > the sink list at all because it isn't available for any of the mirroring
urn's
> > (which will be the only observed sources).
> 
> So firing the change-route-source-click event with selectedCastMode = 0 is not
a
> valid call. So maybe we should add a check for the returned value of
> computeSelectedCastMode_ before firing the event in L161.

It's actually already impossible to fire the event with selectedCastMode == 0
because of the cast button's hidden property binding to
changeRouteSourceAvailable_.  I added a comment explaining this logic here
though.

[imcheng, 2016-08-02 19:32:33]

lgtm

[btolsch, 2016-08-09 01:55:27]

The CQ bit was checked by btolsch@chromium.org

[commit-bot: I haz the power, 2016-08-09 01:55:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 01:55:58]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-08-09 02:29:41]

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

[commit-bot: I haz the power, 2016-08-09 02:29:42]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[btolsch, 2016-08-09 03:40:58]

The CQ bit was checked by btolsch@chromium.org

[btolsch, 2016-08-09 03:40:59]

The patchset sent to the CQ was uploaded after l-g-t-m from imcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2142613002/#ps40001
(title: "Rename event name in route_details_tests.js")

[commit-bot: I haz the power, 2016-08-09 03:41:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 03:41:26]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-08-09 05:57:28]

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

[commit-bot: I haz the power, 2016-08-09 05:57:29]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[btolsch, 2016-08-09 06:37:27]

The CQ bit was checked by btolsch@chromium.org

[commit-bot: I haz the power, 2016-08-09 06:38:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 06:38:06]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-08-09 07:35:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-09 07:37:18]

Description was changed from

==========
[Media Router WebUI] Move replace route responsibility to extension

Currently the MR dialog controls the replace route functionality by
stopping the current route and starting a new one when that completes.
This doesn't allow the extension to optimize the change of source. This
change replaces the current UI behavior by simply calling CreateRoute
with the same sink and new source. This requires an extension change to
support accepting CreateRoute calls to a sink with an existing route.

BUG=614144
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router WebUI] Move replace route responsibility to extension

Currently the MR dialog controls the replace route functionality by
stopping the current route and starting a new one when that completes.
This doesn't allow the extension to optimize the change of source. This
change replaces the current UI behavior by simply calling CreateRoute
with the same sink and new source. This requires an extension change to
support accepting CreateRoute calls to a sink with an existing route.

BUG=614144
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/bbc838f3a5ed945462a29818591753e2cdb04f5c
Cr-Commit-Position: refs/heads/master@{#410607}
==========

[commit-bot: I haz the power, 2016-08-09 07:37:19]

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