Move ProtocolExtensionManager from SessionConnector into its own class

Move ProtocolExtensionManager from SessionConnector into its own class

This CL moves protocolExtensionManager from SessionConnector into its own class.
The ProtcolExtensionManager will be exposed via the ClientPlugin.

The higher level goal is to remove functionality from the SessionConnecctor
so that it can be removed altogether in a future CL.

BUG=474766
Committed: https://crrev.com/25f89e58a6c54e39d2b67cec3859c15f3c9f9661
Cr-Commit-Position: refs/heads/master@{#324167}

[kelvinp, 2015-04-07 21:59:51]

kelvinp@chromium.org changed reviewers:
+ garykac@chromium.org

[kelvinp, 2015-04-07 22:00:36]

PTAL

[garykac, 2015-04-07 22:14:16]

lgtm once client_plugin_impl is updated.

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/base/js/pro...
File remoting/webapp/base/js/protocol_extension_manager_unittest.js (right):

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/base/js/pro...
remoting/webapp/base/js/protocol_extension_manager_unittest.js:85:
QUnit.test('should not register extensions of the same type', function(assert) {
I assume that there's no way to detect that an error was reported here.

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/crd/js/clie...
File remoting/webapp/crd/js/client_plugin_impl.js (right):

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/crd/js/clie...
remoting/webapp/crd/js/client_plugin_impl.js:234: // TODO(kelvinp): Refactor the
ClientSession.State into its own file as
Or have an onConnected, onDisconnected,... message instead of just
onConnectionStatus.

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/crd/js/clie...
remoting/webapp/crd/js/client_plugin_impl.js:257:
this.extensions_.onProtocolExtensionMessage(extMsgType, extMsgData);
Since this no longer requires a connectionEventHandler, it should be moved out
of this if().

[kelvinp, 2015-04-08 00:25:52]

Patchset #2 (id:20001) has been deleted

[kelvinp, 2015-04-08 00:26:02]

FYI

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/base/js/pro...
File remoting/webapp/base/js/protocol_extension_manager_unittest.js (right):

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/base/js/pro...
remoting/webapp/base/js/protocol_extension_manager_unittest.js:85:
QUnit.test('should not register extensions of the same type', function(assert) {
On 2015/04/07 22:14:16, garykac wrote:
> I assume that there's no way to detect that an error was reported here.

I modified register to return true when an extension is registered successfully
and updated the test accordingly.

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/crd/js/clie...
File remoting/webapp/crd/js/client_plugin_impl.js (right):

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/crd/js/clie...
remoting/webapp/crd/js/client_plugin_impl.js:234: // TODO(kelvinp): Refactor the
ClientSession.State into its own file as
On 2015/04/07 22:14:16, garykac wrote:
> Or have an onConnected, onDisconnected,... message instead of just
> onConnectionStatus.

Even if we break onConnectionStatus into OnConnected, etc, we still need to
compare the status against some predefined enum string. 
The problem here is that the enum string is defined in ClientSession, which is a
much concept that belongs to a higher layer than the plugin.

https://codereview.chromium.org/1067133002/diff/1/remoting/webapp/crd/js/clie...
remoting/webapp/crd/js/client_plugin_impl.js:257:
this.extensions_.onProtocolExtensionMessage(extMsgType, extMsgData);
On 2015/04/07 22:14:16, garykac wrote:
> Since this no longer requires a connectionEventHandler, it should be moved out
> of this if().

Done.

[kelvinp, 2015-04-08 00:26:42]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2015-04-08 00:26:42]

The patchset sent to the CQ was uploaded after l-g-t-m from garykac@chromium.org
Link to the patchset: https://codereview.chromium.org/1067133002/#ps40001
(title: "Rebase and  Reviewer's feedback")

[commit-bot: I haz the power, 2015-04-08 00:27:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1067133002/40001

[commit-bot: I haz the power, 2015-04-08 01:21:11]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-04-08 01:22:08]

Patchset 2 (id:??) landed as
https://crrev.com/25f89e58a6c54e39d2b67cec3859c15f3c9f9661
Cr-Commit-Position: refs/heads/master@{#324167}