Commit ce8a324e authored by imcheng's avatar imcheng Committed by Commit bot

[Media Router UI] Fix route-details/sink-list view flashing.

If the dialog is initially in route-details view, and the route is
closed before the 3s startup timer fires, then the dialog will flash
between the route-details and sink-list view before settling on
sink-list. See bug for details.

The fix is to only allow maybeShowRouteDetailsOnOpen to be invoked
once, on startup. It is removed from |rebuildRouteMaps_| and is now
called from setInitialData. Also removed |localRouteCount_| field from
media-router-container since it's no longer needed.

BUG=571463

Review URL: https://codereview.chromium.org/1538363003

Cr-Commit-Position: refs/heads/master@{#367031}
parent b3801704
...@@ -122,15 +122,6 @@ Polymer({ ...@@ -122,15 +122,6 @@ Polymer({
value: true, value: true,
}, },
/**
* The number of current local routes.
* @private {number}
*/
localRouteCount_: {
type: Number,
value: 0,
},
/** /**
* The list of current routes. * The list of current routes.
* @type {!Array<!media_router.Route>} * @type {!Array<!media_router.Route>}
...@@ -583,13 +574,24 @@ Polymer({ ...@@ -583,13 +574,24 @@ Polymer({
/** /**
* Updates |currentView_| if the dialog had just opened and there's * Updates |currentView_| if the dialog had just opened and there's
* only one local route. * only one local route.
*
* @param {?media_router.Route} route A local route.
* @private
*/ */
maybeShowRouteDetailsOnOpen_: function(route) { maybeShowRouteDetailsOnOpen: function() {
if (this.localRouteCount_ == 1 && this.justOpened_ && route) var localRoute = null;
this.showRouteDetails_(route); for (var i = 0; i < this.routeList.length; i++) {
var route = this.routeList[i];
if (!route.isLocal)
continue;
if (!localRoute) {
localRoute = route;
} else {
// Don't show route details if there are more than one local route.
localRoute = null;
break;
}
}
if (localRoute)
this.showRouteDetails_(localRoute);
}, },
/** /**
...@@ -711,12 +713,6 @@ Polymer({ ...@@ -711,12 +713,6 @@ Polymer({
*/ */
rebuildRouteMaps_: function() { rebuildRouteMaps_: function() {
this.routeMap_ = {}; this.routeMap_ = {};
this.localRouteCount_ = 0;
// Keeps track of the last local route we find in |routeList|. If
// |localRouteCount_| is eventually equal to one, |localRoute| would be the
// only current local route.
var localRoute = null;
// Rebuild |sinkToRouteMap_| with a temporary map to avoid firing the // Rebuild |sinkToRouteMap_| with a temporary map to avoid firing the
// computed functions prematurely. // computed functions prematurely.
...@@ -726,14 +722,6 @@ Polymer({ ...@@ -726,14 +722,6 @@ Polymer({
this.routeList.forEach(function(route) { this.routeList.forEach(function(route) {
this.routeMap_[route.id] = route; this.routeMap_[route.id] = route;
tempSinkToRouteMap[route.sinkId] = route; tempSinkToRouteMap[route.sinkId] = route;
if (route.isLocal) {
this.localRouteCount_++;
// It's OK if localRoute is updated multiple times; it is only used if
// |localRouteCount_| == 1, which implies it was only set once.
localRoute = route;
}
}, this); }, this);
// If |currentRoute_| is no longer active, clear |currentRoute_|. Also // If |currentRoute_| is no longer active, clear |currentRoute_|. Also
...@@ -747,7 +735,6 @@ Polymer({ ...@@ -747,7 +735,6 @@ Polymer({
} }
this.sinkToRouteMap_ = tempSinkToRouteMap; this.sinkToRouteMap_ = tempSinkToRouteMap;
this.maybeShowRouteDetailsOnOpen_(localRoute);
this.rebuildSinksToShow_(); this.rebuildSinksToShow_();
}, },
......
...@@ -65,6 +65,7 @@ cr.define('media_router.ui', function() { ...@@ -65,6 +65,7 @@ cr.define('media_router.ui', function() {
container.castModeList = data['castModes']; container.castModeList = data['castModes'];
container.allSinks = data['sinks']; container.allSinks = data['sinks'];
container.routeList = data['routes']; container.routeList = data['routes'];
container.maybeShowRouteDetailsOnOpen();
media_router.browserApi.onInitialDataReceived(); media_router.browserApi.onInitialDataReceived();
} }
......
...@@ -376,6 +376,7 @@ cr.define('media_router_container', function() { ...@@ -376,6 +376,7 @@ cr.define('media_router_container', function() {
test('initial view with one local route', function() { test('initial view with one local route', function() {
container.allSinks = fakeSinkList; container.allSinks = fakeSinkList;
container.routeList = fakeRouteList; container.routeList = fakeRouteList;
container.maybeShowRouteDetailsOnOpen();
checkCurrentView(media_router.MediaRouterView.ROUTE_DETAILS); checkCurrentView(media_router.MediaRouterView.ROUTE_DETAILS);
}); });
...@@ -403,6 +404,7 @@ cr.define('media_router_container', function() { ...@@ -403,6 +404,7 @@ cr.define('media_router_container', function() {
test('view after route is closed remotely', function() { test('view after route is closed remotely', function() {
container.allSinks = fakeSinkList; container.allSinks = fakeSinkList;
container.routeList = fakeRouteList; container.routeList = fakeRouteList;
container.maybeShowRouteDetailsOnOpen();
checkCurrentView(media_router.MediaRouterView.ROUTE_DETAILS); checkCurrentView(media_router.MediaRouterView.ROUTE_DETAILS);
container.routeList = []; container.routeList = [];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment