[Presentation API] Media Router: queue PresentationConnectionStateChanged events fired before callb…

[Reverted] We adopt a temporary workaround to signal state change in browser side (MR)

[Presentation API] Media Router: queue PresentationConnectionStateChanged events fired before callback is registered.

Sometimes ConnectionStateChanged event ('connecting'->'connected') is fired before we listening to connection state change. Those events are lost and onconnect event handler won't be called. We will queue those events and fire them later when callback becomes available.

BUG=

[zhaobin, 2016-11-01 01:53:52]

Patchset #1 (id:1) has been deleted

[zhaobin, 2016-11-01 01:56:58]

zhaobin@chromium.org changed reviewers:
+ imcheng@chromium.org, mfoltz@chromium.org

[imcheng, 2016-11-01 20:09:19]

While I like the browser side queueing approach in theory since it is shared
with the Android implementation (though IIUC this has never been a problem on
Android), I have some concerns about managing the mapping browser-side. I am
wondering if they can be addressed here or if it's cleaner if we instead queue
on the extension side.

https://codereview.chromium.org/2463843003/diff/20001/chrome/browser/media/ro...
File chrome/browser/media/router/media_router_base.cc (right):

https://codereview.chromium.org/2463843003/diff/20001/chrome/browser/media/ro...
chrome/browser/media/router/media_router_base.cc:121:
queued_state_changes_.insert(std::make_pair(route_id, info));
If we never added a state change callback, would this end up staying in the map
indefinitely?

There are two issues we have to address here.

1) The extension can send us state change info with bogus route ids - in those
cases queued_state_changes_ will end up accumulating entries because we will
never add callbacks for them.

2) There are increased coupling between the behaviors of MR and Presentation API
implementation. Even for legit route IDs we expect a callback to be added at
some point in order for this to not be a (browser) memory leak.

I am wondering if there is a way to address these. Another approach I  think we
can consider is to have MR signal the extension that it is ready to receive
presentation connection state changes, similar to how it is done for messages.
This means queueing state changes in the extension and discarding them after
some condition is reached (for example, reaching a terminal state)

[zhaobin, 2016-11-02 00:18:20]

Description was changed from

==========
[Presentation API] Media Router: queue PresentationConnectionStateChanged events
fired before callback is registered.

Sometimes ConnectionStateChanged event ('connecting'->'connected') is fired
before we listening to connection state change. Those events are lost and
onconnect event handler won't be called. We will queue those events and fire
them later when callback becomes available.

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

to

==========
[Reverted] We adopt a temporary workaround to signal state change in browser
side (MR)

[Presentation API] Media Router: queue PresentationConnectionStateChanged events
fired before callback is registered.

Sometimes ConnectionStateChanged event ('connecting'->'connected') is fired
before we listening to connection state change. Those events are lost and
onconnect event handler won't be called. We will queue those events and fire
them later when callback becomes available.

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