[Webapp Refactor] Remove It2Me connection code from SessionConnectorImpl.

[Webapp Refactor] Remove It2Me connection code from SessionConnectorImpl.

The SessionConnector is intended be a core component that is shared between
app remoting and desktop remoting. It2Me is desktop remoting specific and
should not be there.

This CL pulls out all the It2Me connection logic into It2MeConnectFlow.

BUG=464614
Committed: https://crrev.com/a78cad4e6288048d9a74eadf3dca66247f31fa62
Cr-Commit-Position: refs/heads/master@{#319552}

[kelvinp, 2015-03-06 01:57:09]

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

[kelvinp, 2015-03-06 01:58:20]

PTAL

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/sessi...
File remoting/webapp/crd/js/session_connector_impl.js (left):

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/sessi...
remoting/webapp/crd/js/session_connector_impl.js:250:
remoting.SessionConnectorImpl.prototype.connectIT2Me = function(accessCode) {
Moved to It2MeConnectFlow()

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/sessi...
remoting/webapp/crd/js/session_connector_impl.js:387:
remoting.SessionConnectorImpl.prototype.connectIT2MeWithToken_ =
Moved to It2MeConnectFlow()

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/sessi...
remoting/webapp/crd/js/session_connector_impl.js:407:
remoting.SessionConnectorImpl.prototype.onIT2MeHostInfo_ =
Moved to It2MeConnectFlow()

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/sessi...
remoting/webapp/crd/js/session_connector_impl.js:530:
remoting.SessionConnectorImpl.prototype.translateSupportHostsError_ =
Moved to It2MeConnectFlow()

[kelvinp, 2015-03-06 02:59:09]

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

[garykac, 2015-03-06 22:27:27]

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/crd_c...
File remoting/webapp/crd/js/crd_connect.js (right):

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/crd_c...
remoting/webapp/crd/js/crd_connect.js:23: function(reason){
formatting nit: What do you think about starting this line with 'catch' to make
it more obvious that this is error-handling code?

remoting.It2MeConnectFlow.start(connector, accessCode).
    catch(function(reason){
      var errorDiv = document.getElementById('connect-error-message');
      l10n.localizeElementFromTag(errorDiv, /** @type {string} */ (reason));
      remoting.setMode(remoting.AppMode.CLIENT_CONNECT_FAILED_IT2ME);
    });

(frankly, I would prefer the line to start with '.catch', but Chrome style wants
'.'s at the end of the line :-( )

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/it2me...
File remoting/webapp/crd/js/it2me_connect_flow.js (right):

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/it2me...
remoting/webapp/crd/js/it2me_connect_flow.js:12: var SUPPORT_ID_LENGTH = 7;
Could you add a comment here describing what these are and the units.

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/it2me...
remoting/webapp/crd/js/it2me_connect_flow.js:61: });
If this rejects() is will pass a remoting.Error, but I don't see that annotated
anywhere (I had to dig through the code).

Should we .catch the reject here and then re-reject it so that we can better
document the error handling? Are there good closure-compiler annotations for
that to that it will complain if we reject without a remoting.Error?

[kelvinp, 2015-03-06 23:22:00]

Patchset #2 (id:20001) has been deleted

[kelvinp, 2015-03-06 23:33:51]

PTAL

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/crd_c...
File remoting/webapp/crd/js/crd_connect.js (right):

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/crd_c...
remoting/webapp/crd/js/crd_connect.js:23: function(reason){
On 2015/03/06 22:27:27, garykac wrote:
> formatting nit: What do you think about starting this line with 'catch' to
make
> it more obvious that this is error-handling code?
> 
> remoting.It2MeConnectFlow.start(connector, accessCode).
>     catch(function(reason){
>       var errorDiv = document.getElementById('connect-error-message');
>       l10n.localizeElementFromTag(errorDiv, /** @type {string} */ (reason));
>       remoting.setMode(remoting.AppMode.CLIENT_CONNECT_FAILED_IT2ME);
>     });
> 
> (frankly, I would prefer the line to start with '.catch', but Chrome style
wants
> '.'s at the end of the line :-( )

Done.

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/it2me...
File remoting/webapp/crd/js/it2me_connect_flow.js (right):

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/it2me...
remoting/webapp/crd/js/it2me_connect_flow.js:12: var SUPPORT_ID_LENGTH = 7;
On 2015/03/06 22:27:27, garykac wrote:
> Could you add a comment here describing what these are and the units.

Done.

https://codereview.chromium.org/984793003/diff/1/remoting/webapp/crd/js/it2me...
remoting/webapp/crd/js/it2me_connect_flow.js:61: });
On 2015/03/06 22:27:27, garykac wrote:
> If this rejects() is will pass a remoting.Error, but I don't see that
annotated
> anywhere (I had to dig through the code).
> 
> Should we .catch the reject here and then re-reject it so that we can better
> document the error handling? Are there good closure-compiler annotations for
> that to that it will complain if we reject without a remoting.Error?

Done.

[garykac, 2015-03-06 23:53:02]

lgtm

https://codereview.chromium.org/984793003/diff/60001/remoting/webapp/crd/js/i...
File remoting/webapp/crd/js/it2me_connect_flow.js (right):

https://codereview.chromium.org/984793003/diff/60001/remoting/webapp/crd/js/i...
remoting/webapp/crd/js/it2me_connect_flow.js:64: }, function(error){
nit: I find that:
  }).then(function() {
  }).catch(function(error) {
  });

is easier to read than:
  }).then(function() {
  }, function(error) {
  });

Or rather, I find that the former makes the error-handling code more apparent
than the latter when scanning the code.

[kelvinp, 2015-03-07 00:13:45]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2015-03-07 00:13:46]

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/984793003/#ps80001 (title:
"Ready for Checkin")

[commit-bot: I haz the power, 2015-03-07 00:14:19]

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

[Jamie, 2015-03-07 00:58:57]

https://codereview.chromium.org/984793003/diff/60001/remoting/webapp/crd/js/i...
File remoting/webapp/crd/js/it2me_connect_flow.js (right):

https://codereview.chromium.org/984793003/diff/60001/remoting/webapp/crd/js/i...
remoting/webapp/crd/js/it2me_connect_flow.js:64: }, function(error){
On 2015/03/06 23:53:01, garykac wrote:
> nit: I find that:
>   }).then(function() {
>   }).catch(function(error) {
>   });
> 
> is easier to read than:
>   }).then(function() {
>   }, function(error) {
>   });
> 
> Or rather, I find that the former makes the error-handling code more apparent
> than the latter when scanning the code.

+1, FWIW :)

[commit-bot: I haz the power, 2015-03-07 02:15:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-07 02:15:44]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
Timed out jobs are not retried to avoid causing additional load on the builders
with large pending queues.

[kelvinp, 2015-03-07 03:48:02]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2015-03-07 03:48:13]

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

[commit-bot: I haz the power, 2015-03-07 03:49:03]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-03-07 03:49:52]

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