Implements It2MeBrowserTest CancelShare and VerifyAccessCodeNotReusable

Implements It2MeBrowserTest CancelShare and VerifyAccessCodeNotReusable

This CL implements two P0 browser test cases
- It2MeBrowserTest.CancelShare
- It2MeBrowserTest.VerifyAccessCodeNotReusable

This only behavioral change in this CL is that we will always return INVALID_ACCESS_CODE
for It2Me Connections failed with HOST_IS_OFFLINE.
HOST_IS_OFFLINE suggests that the host portion of the access code
is invalid and we should hide this implementation detail from the user.

BUG=466849, 466850
Committed: https://crrev.com/6f0f0e5434eaa270179676272c9b34f0b3882554
Cr-Commit-Position: refs/heads/master@{#320835}

[kelvinp, 2015-03-13 00:03:17]

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

[kelvinp, 2015-03-13 00:05:00]

Patchset #1 (id:1) has been deleted

[kelvinp, 2015-03-13 00:05:21]

PTAL

[Jamie, 2015-03-13 18:38:47]

https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_remoting.js (right):

https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:261: error =
remoting.Error.INVALID_ACCESS_CODE;
There are two cases to consider here: where the directory returns an error when
we query the host id (probably 404, but I'm not sure) and where the host returns
a JID, but we're not able to connect to it. It seems like the latter case is a
real error (though it might happen if there are replication delays in the
directory--Renato would know better), whereas the former is not handled here.

[kelvinp, 2015-03-13 18:49:00]

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

[kelvinp, 2015-03-13 18:49:37]

On 2015/03/13 18:38:47, Jamie wrote:
>
https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
> File remoting/webapp/crd/js/desktop_remoting.js (right):
> 
>
https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
> remoting/webapp/crd/js/desktop_remoting.js:261: error =
> remoting.Error.INVALID_ACCESS_CODE;
> There are two cases to consider here: where the directory returns an error
when
> we query the host id (probably 404, but I'm not sure) and where the host
returns
> a JID, but we're not able to connect to it. It seems like the latter case is a
> real error (though it might happen if there are replication delays in the
> directory--Renato would know better), whereas the former is not handled here.

Adding Renato so that he can chime in for Jamie's question.

[kelvinp, 2015-03-13 18:55:26]

https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_remoting.js (right):

https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:261: error =
remoting.Error.INVALID_ACCESS_CODE;
On 2015/03/13 18:38:47, Jamie wrote:
> There are two cases to consider here: where the directory returns an error
when
> we query the host id (probably 404, but I'm not sure) and where the host
returns
> a JID, but we're not able to connect to it. It seems like the latter case is a
> real error (though it might happen if there are replication delays in the
> directory--Renato would know better), whereas the former is not handled here.

For case 1)
If the directory returns 404, It2MeConnectFlow will return InvalidAccessCode. 
See
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/webapp/cr...

For case 2)
From debugging, I noticed that case 2 is actually happening.  The host is
returned with a valid JID but we just can't connect to it. I will wait and see
what Renato's feedback is.

[rmsousa, 2015-03-13 22:14:27]

https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_remoting.js (right):

https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting.js:261: error =
remoting.Error.INVALID_ACCESS_CODE;
On 2015/03/13 18:55:25, kelvinp wrote:
> On 2015/03/13 18:38:47, Jamie wrote:
> > There are two cases to consider here: where the directory returns an error
> when
> > we query the host id (probably 404, but I'm not sure) and where the host
> returns
> > a JID, but we're not able to connect to it. It seems like the latter case is
a
> > real error (though it might happen if there are replication delays in the
> > directory--Renato would know better), whereas the former is not handled
here.
> 
> For case 1)
> If the directory returns 404, It2MeConnectFlow will return InvalidAccessCode. 
> See
>
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/webapp/cr...
> 
> For case 2)
> From debugging, I noticed that case 2 is actually happening.  The host is
> returned with a valid JID but we just can't connect to it. I will wait and see
> what Renato's feedback is.
> 


HOST_IS_OFFLINE does not indicate that the host portion of the code is invalid.
It means a different thing. it means that the host is _valid_, that we got a JID
from the directory, tried to connect to it, and got *no response*. (e.g. because
the host user disconnected). HOST_IS_OFFLINE is the right response for this
case.

A different issue is a 404 when getting the host (host portion is invalid).
InvalidAccessCode here is correct. THIS one is *theoretically* subject to
replication delays (host registers itself to share the machine, client tries
that host id and it hasn't shown up yet), but we use some "sharding" heuristic
to avoid that situation (each shard generates its own ids, and when the client
tries to fetch the host that shard is consulted first), so the actual
probability of a replication-related issue here is close to nil. I think it's
safe to use InvalidAccessCode for 404s.

When the host portion is valid but the PIN portion is invalid, I suppose we
already return INVALID_ACCESS_CODE, so that's correct.

There's also another case - a host is valid, but someone already used that
access code. In that case, the host will actively reject the second connection.
I can't remember what the error code for this is, I'm guessing InvalidAccessCode
as well.

I'm not sure why you're not able to connect to the host JID, but that's not
directory related.

[kelvinp, 2015-03-13 23:18:57]

On 2015/03/13 22:14:27, rmsousa wrote:
>
https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
> File remoting/webapp/crd/js/desktop_remoting.js (right):
> 
>
https://codereview.chromium.org/1007543003/diff/20001/remoting/webapp/crd/js/...
> remoting/webapp/crd/js/desktop_remoting.js:261: error =
> remoting.Error.INVALID_ACCESS_CODE;
> On 2015/03/13 18:55:25, kelvinp wrote:
> > On 2015/03/13 18:38:47, Jamie wrote:
> > > There are two cases to consider here: where the directory returns an error
> > when
> > > we query the host id (probably 404, but I'm not sure) and where the host
> > returns
> > > a JID, but we're not able to connect to it. It seems like the latter case
is
> a
> > > real error (though it might happen if there are replication delays in the
> > > directory--Renato would know better), whereas the former is not handled
> here.
> > 
> > For case 1)
> > If the directory returns 404, It2MeConnectFlow will return
InvalidAccessCode. 
> > See
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/webapp/cr...
> > 
> > For case 2)
> > From debugging, I noticed that case 2 is actually happening.  The host is
> > returned with a valid JID but we just can't connect to it. I will wait and
see
> > what Renato's feedback is.
> > 
> 
> 
> HOST_IS_OFFLINE does not indicate that the host portion of the code is
invalid.
> It means a different thing. it means that the host is _valid_, that we got a
JID
> from the directory, tried to connect to it, and got *no response*. (e.g.
because
> the host user disconnected). HOST_IS_OFFLINE is the right response for this
> case.
> 
> A different issue is a 404 when getting the host (host portion is invalid).
> InvalidAccessCode here is correct. THIS one is *theoretically* subject to
> replication delays (host registers itself to share the machine, client tries
> that host id and it hasn't shown up yet), but we use some "sharding" heuristic
> to avoid that situation (each shard generates its own ids, and when the client
> tries to fetch the host that shard is consulted first), so the actual
> probability of a replication-related issue here is close to nil. I think it's
> safe to use InvalidAccessCode for 404s.
> 
> When the host portion is valid but the PIN portion is invalid, I suppose we
> already return INVALID_ACCESS_CODE, so that's correct.
> 
> There's also another case - a host is valid, but someone already used that
> access code. In that case, the host will actively reject the second
connection.
> I can't remember what the error code for this is, I'm guessing
InvalidAccessCode
> as well.
> 
> I'm not sure why you're not able to connect to the host JID, but that's not
> directory related.

Spoke with Sergey.  Turns out we don't unregister the host from the directory
when
the shared is canceled.  That explains the HOST_IS_OFFLINE error.  For the
purpose of
this CL, I am going to make the test accepts both as valid outcomes and revert
the 
change in production code.

[kelvinp, 2015-03-16 18:31:02]

PTAL

After talking to sergey, the existing implementation does not unregister the
host from the directory after a share is cancelled. 

I am leaving the production code as is (except for fixing a bug that only
refresh the JID on me2me connections).

[Jamie, 2015-03-16 20:40:56]

lgtm

[kelvinp, 2015-03-16 20:47:03]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2015-03-16 20:47:30]

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

[commit-bot: I haz the power, 2015-03-16 21:56:31]

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

[commit-bot: I haz the power, 2015-03-16 21:56:32]

Try jobs failed on following builders:
  linux_chromium_asan_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kelvinp, 2015-03-16 22:12:22]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2015-03-16 22:12:30]

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

[commit-bot: I haz the power, 2015-03-16 22:33:32]

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

[commit-bot: I haz the power, 2015-03-16 22:33:35]

Try jobs failed on following builders:
  linux_chromium_asan_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kelvinp, 2015-03-16 23:21:40]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2015-03-16 23:21:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1007543003/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2015-03-16 23:22:06]

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

[commit-bot: I haz the power, 2015-03-17 00:55:43]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-03-17 00:56:26]

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