[Media Router] Handle route creation where resolved route is not for display.

[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not marked for display. This keeps track of the route id and checks if the route is updated to be ready for display.

In the case where the route is never ready for display, route creation times out and fails. The MRP will send an issue to the Media Router if this happens.

Updated reporting for relevant UMA metric.

BUG=579138
Committed: https://crrev.com/3647a2e3c8476adfc7aba16eb12b7f3e13648ec6
Cr-Commit-Position: refs/heads/master@{#371385}

[apacible, 2016-01-19 20:57:13]

Description was changed from

==========
resolved route not for display

BUG=
==========

to

==========
[Work In Progress] resolved route not for display

BUG=579138
==========

[apacible, 2016-01-19 22:51:52]

Patchset #1 (id:1) has been deleted

[apacible, 2016-01-20 00:24:35]

Patchset #2 (id:40001) has been deleted

[apacible, 2016-01-20 00:24:39]

Patchset #1 (id:20001) has been deleted

[apacible, 2016-01-20 01:12:17]

Description was changed from

==========
[Work In Progress] resolved route not for display

BUG=579138
==========

to

==========
[Media Router] Handle route creation where resolved route is not for display

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails. If the route is also removed from the |routeList|, the route creation
had also failed.

Updated reporting for relevant UMA metric.

BUG=579138
==========

[apacible, 2016-01-20 01:12:17]

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

[apacible, 2016-01-20 01:13:29]

PTAL, thanks!

Tests not updated yet. I've only manually tested on app casting since mirroring
is currently failing.

[apacible, 2016-01-20 01:32:54]

Patchset #1 (id:60001) has been deleted

[apacible, 2016-01-20 01:33:08]

Patchset #1 (id:80001) has been deleted

[apacible, 2016-01-20 01:33:51]

Description was changed from

==========
[Media Router] Handle route creation where resolved route is not for display

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails. If the route is also removed from the |routeList|, the route creation
had also failed.

Updated reporting for relevant UMA metric.

BUG=579138
==========

to

==========
[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails. If the route is also removed from the |routeList|, the route creation
had also failed.

Updated reporting for relevant UMA metric.

BUG=579138
==========

[apacible, 2016-01-20 03:53:33]

Description was changed from

==========
[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails. If the route is also removed from the |routeList|, the route creation
had also failed.

Updated reporting for relevant UMA metric.

BUG=579138
==========

to

==========
[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails.

Updated reporting for relevant UMA metric.

BUG=579138
==========

[apacible, 2016-01-20 04:26:35]

Patchset #1 (id:100001) has been deleted

[apacible, 2016-01-20 04:26:40]

Patchset #1 (id:120001) has been deleted

[imcheng, 2016-01-20 19:35:47]

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
(right):

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js:865:
* @param {?string} routeId The ID of the newly created route for the sink if
{string} since it is non-null.

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js:940:
if (this.currentLaunchingSinkId_ != '') {
should we also need to check this.pendingCreatedRouteId_ != '' here? Seems like
you don't have to check the route map if you haven't gotten a successful route
response yet.

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js:1022:
resetRouteCreationPropertiesState_: function(creationSuccess) {
naming: how about just resetRouteCreationProperties_ ?

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
(right):

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/ui/webu...
chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc:564:
void MediaRouterWebUIMessageHandler::OnReportRouteCreation(
I wonder if this is needed. MRUI already knows if a route creation was
successful or not it looks like this roundtrip is unnecessary. Can we just call
UMA in OnCreateRouteResponseReceived / NotifyRouteCreationTimeout?

[apacible, 2016-01-20 19:51:27]

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
(right):

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js:865:
* @param {?string} routeId The ID of the newly created route for the sink if
On 2016/01/20 19:35:46, imcheng1 wrote:
> {string} since it is non-null.

Done.

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js:940:
if (this.currentLaunchingSinkId_ != '') {
On 2016/01/20 19:35:46, imcheng1 wrote:
> should we also need to check this.pendingCreatedRouteId_ != '' here? Seems
like
> you don't have to check the route map if you haven't gotten a successful route
> response yet.

Done.

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js:1022:
resetRouteCreationPropertiesState_: function(creationSuccess) {
On 2016/01/20 19:35:46, imcheng1 wrote:
> naming: how about just resetRouteCreationProperties_ ?

Done.

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
(right):

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/ui/webu...
chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc:564:
void MediaRouterWebUIMessageHandler::OnReportRouteCreation(
On 2016/01/20 19:35:46, imcheng1 wrote:
> I wonder if this is needed. MRUI already knows if a route creation was
> successful or not it looks like this roundtrip is unnecessary. Can we just
call
> UMA in OnCreateRouteResponseReceived / NotifyRouteCreationTimeout?

OnCreateRouteResponseReceived returns the route that isn't for display, but its
follow up "for display" route indicates the route was 'completely' created.

There's still a chance the route will fail to successfully create after
OnCreateRouteResponseReceived is called. In that case, the MRP sends an error to
the MRUI.

[imcheng, 2016-01-25 19:27:32]

lgtm + 1 comment

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
(right):

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/ui/webu...
chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc:564:
void MediaRouterWebUIMessageHandler::OnReportRouteCreation(
On 2016/01/20 19:51:26, apacible wrote:
> On 2016/01/20 19:35:46, imcheng1 wrote:
> > I wonder if this is needed. MRUI already knows if a route creation was
> > successful or not it looks like this roundtrip is unnecessary. Can we just
> call
> > UMA in OnCreateRouteResponseReceived / NotifyRouteCreationTimeout?
> 
> OnCreateRouteResponseReceived returns the route that isn't for display, but
its
> follow up "for display" route indicates the route was 'completely' created.
> 
> There's still a chance the route will fail to successfully create after
> OnCreateRouteResponseReceived is called. In that case, the MRP sends an error
to
> the MRUI.

In this case, will the MRP send an issue? We should think of a mechanism to
report this type of route creation failure.

[apacible, 2016-01-25 22:54:04]

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
(right):

https://codereview.chromium.org/1605133002/diff/170001/chrome/browser/ui/webu...
chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc:564:
void MediaRouterWebUIMessageHandler::OnReportRouteCreation(
On 2016/01/25 19:27:32, imcheng1 wrote:
> On 2016/01/20 19:51:26, apacible wrote:
> > On 2016/01/20 19:35:46, imcheng1 wrote:
> > > I wonder if this is needed. MRUI already knows if a route creation was
> > > successful or not it looks like this roundtrip is unnecessary. Can we just
> > call
> > > UMA in OnCreateRouteResponseReceived / NotifyRouteCreationTimeout?
> > 
> > OnCreateRouteResponseReceived returns the route that isn't for display, but
> its
> > follow up "for display" route indicates the route was 'completely' created.
> > 
> > There's still a chance the route will fail to successfully create after
> > OnCreateRouteResponseReceived is called. In that case, the MRP sends an
error
> to
> > the MRUI.
> 
> In this case, will the MRP send an issue? We should think of a mechanism to
> report this type of route creation failure.

Correct, the MRP will send an issue. I confirmed this with vadimgo@ when
discussing the behavior we should expect on the MR side.

[apacible, 2016-01-25 22:54:43]

Description was changed from

==========
[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails.

Updated reporting for relevant UMA metric.

BUG=579138
==========

to

==========
[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails. The MRP will send an issue to the Media Router if this happens.

Updated reporting for relevant UMA metric.

BUG=579138
==========

[apacible, 2016-01-25 23:10:28]

The CQ bit was checked by apacible@chromium.org

[commit-bot: I haz the power, 2016-01-25 23:11:40]

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

[commit-bot: I haz the power, 2016-01-26 00:44:58]

Description was changed from

==========
[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails. The MRP will send an issue to the Media Router if this happens.

Updated reporting for relevant UMA metric.

BUG=579138
==========

to

==========
[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails. The MRP will send an issue to the Media Router if this happens.

Updated reporting for relevant UMA metric.

BUG=579138
==========

[commit-bot: I haz the power, 2016-01-26 00:44:59]

Committed patchset #4 (id:190001)

[commit-bot: I haz the power, 2016-01-26 00:46:34]

Description was changed from

==========
[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails. The MRP will send an issue to the Media Router if this happens.

Updated reporting for relevant UMA metric.

BUG=579138
==========

to

==========
[Media Router] Handle route creation where resolved route is not for display.

This change handles route creation where the initially resolved route is not
marked for display. This keeps track of the route id and checks if the route is
updated to be ready for display.

In the case where the route is never ready for display, route creation times out
and fails. The MRP will send an issue to the Media Router if this happens.

Updated reporting for relevant UMA metric.

BUG=579138

Committed: https://crrev.com/3647a2e3c8476adfc7aba16eb12b7f3e13648ec6
Cr-Commit-Position: refs/heads/master@{#371385}
==========

[commit-bot: I haz the power, 2016-01-26 00:46:34]

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