Fix IT2Me.

Fix IT2Me.

BUG=416540
Committed: https://crrev.com/54452e7b9480332f1e7740540eb7c8824a1071ab
Cr-Commit-Position: refs/heads/master@{#296092}

[Jamie, 2014-09-22 18:03:37]

jamiewalch@chromium.org changed reviewers:
+ sergeyu@chromium.org

[Jamie, 2014-09-22 18:03:38]

ptal

[Sergey Ulanov, 2014-09-22 18:11:51]

https://codereview.chromium.org/592163002/diff/1/remoting/webapp/session_conn...
File remoting/webapp/session_connector_impl.js (right):

https://codereview.chromium.org/592163002/diff/1/remoting/webapp/session_conn...
remoting/webapp/session_connector_impl.js:277: this.connectSignaling_();
connectSignaling_() connects signaling and then starts the client, but
connectIT2MeWithToken_ is still called below, so this would start the plugin
twice.

https://codereview.chromium.org/592163002/diff/1/remoting/webapp/session_conn...
remoting/webapp/session_connector_impl.js:433: this.createSession_();
I think you just need to replace this line with "this.connectSignaling_();"

(connectSignaling_() is not a good name - it also connects the client, so it
would be a good idea to rename it)

[Jamie, 2014-09-22 18:40:05]

ptal

https://codereview.chromium.org/592163002/diff/1/remoting/webapp/session_conn...
File remoting/webapp/session_connector_impl.js (right):

https://codereview.chromium.org/592163002/diff/1/remoting/webapp/session_conn...
remoting/webapp/session_connector_impl.js:277: this.connectSignaling_();
On 2014/09/22 18:11:51, Sergey Ulanov wrote:
> connectSignaling_() connects signaling and then starts the client, but
> connectIT2MeWithToken_ is still called below, so this would start the plugin
> twice.

Acknowledged.

https://codereview.chromium.org/592163002/diff/1/remoting/webapp/session_conn...
remoting/webapp/session_connector_impl.js:433: this.createSession_();
On 2014/09/22 18:11:51, Sergey Ulanov wrote:
> I think you just need to replace this line with "this.connectSignaling_();"


Done

> (connectSignaling_() is not a good name - it also connects the client, so it
> would be a good idea to rename it)

We have plenty of other examples of this sort of naming, where the function just
performs the next in a sequence of steps and is named according to that step.
It's a good example of where Promises can make things cleaner; each function now
just does the thing its name suggests and returns a Promise that the caller can
use to chain multiple such functions together. For now, I think this name is
good enough.

[Sergey Ulanov, 2014-09-22 22:52:49]

lgtm

[Sergey Ulanov, 2014-09-22 22:52:54]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-09-22 22:54:00]

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

[commit-bot: I haz the power, 2014-09-22 23:36:55]

Committed patchset #2 (id:20001) as b51844472633b5a18e1d70cbf2fa26c321d1cd26

[commit-bot: I haz the power, 2014-09-22 23:37:28]

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