Suppress "errors" that aren't really errors.

Add support for suppressing error logging, and don't log errors due to cached host information or client device suspend.

BUG=139389
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150004

[Jamie, 2012-08-03 21:15:15]

This CL allows errors to be suppressed by the client in cases where we know what
went wrong, and reporting it as an error would be misleading. It implements two
such cases: connecting using cached host information; and errors occurring after
the device has been suspended (which most likely corresponds to the laptop lid
being closed). Note that I haven't suppressed errors due to online/offline
events as my tests suggest that connection closures in such cases are not
reported as errors anyway.

[simonmorris, 2012-08-03 21:48:49]

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/client_sess...
File remoting/webapp/client_session.js (right):

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/client_sess...
remoting/webapp/client_session.js:668: * JID will be refreshed and the
connectioned retried.
connectioned -> connection

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/suspend_mon...
File remoting/webapp/suspend_monitor.js (right):

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/suspend_mon...
remoting/webapp/suspend_monitor.js:28: this.start_();
It might be better to start the suspend monitor only when a session is active,
and to stop it when the session ends. It doesn't seem right to suppress errors
just because a user closed their laptop before starting a session.

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/suspend_mon...
remoting/webapp/suspend_monitor.js:41: this.callback_();
This will typically be called 30 seconds after the device resumes. Isn't it
likely that the plugin will already have detected and logged an error by then?

[simonmorris, 2012-08-03 21:49:17]

The title of the CL looks wrong.

[Jamie, 2012-08-03 23:15:14]

Wez, since Simon isn't available could you take over? I'd like to land this for
M22 if possible. I've addressed his comments.

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/client_sess...
File remoting/webapp/client_session.js (right):

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/client_sess...
remoting/webapp/client_session.js:668: * JID will be refreshed and the
connectioned retried.
On 2012/08/03 21:48:49, simonmorris wrote:
> connectioned -> connection

Done.

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/suspend_mon...
File remoting/webapp/suspend_monitor.js (right):

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/suspend_mon...
remoting/webapp/suspend_monitor.js:28: this.start_();
On 2012/08/03 21:48:49, simonmorris wrote:
> It might be better to start the suspend monitor only when a session is active,
> and to stop it when the session ends. It doesn't seem right to suppress errors
> just because a user closed their laptop before starting a session.

I don't think that will happen. If the callback is delayed, then it will occur
as soon as the lap-top is opened. In other words, the user won't have time to
initiate anything.

http://codereview.chromium.org/10825187/diff/4001/remoting/webapp/suspend_mon...
remoting/webapp/suspend_monitor.js:41: this.callback_();
On 2012/08/03 21:48:49, simonmorris wrote:
> This will typically be called 30 seconds after the device resumes. Isn't it
> likely that the plugin will already have detected and logged an error by then?

That is only true for very short suspension periods. As long as the device is
asleep for more than 2m, the callback will be invoked as soon as it resumes.

[garykac, 2012-08-04 00:43:46]

lgtm but please check the 'use strict'

http://codereview.chromium.org/10825187/diff/4002/remoting/webapp/suspend_mon...
File remoting/webapp/suspend_monitor.js (right):

http://codereview.chromium.org/10825187/diff/4002/remoting/webapp/suspend_mon...
remoting/webapp/suspend_monitor.js:12: 
Shouldn't there be
   'use strict';
   var remoting = remoting || {};
at the top?

[Jamie, 2012-08-04 00:51:39]

fyi

http://codereview.chromium.org/10825187/diff/4002/remoting/webapp/suspend_mon...
File remoting/webapp/suspend_monitor.js (right):

http://codereview.chromium.org/10825187/diff/4002/remoting/webapp/suspend_mon...
remoting/webapp/suspend_monitor.js:12: 
On 2012/08/04 00:43:46, garykac wrote:
> Shouldn't there be
>    'use strict';
>    var remoting = remoting || {};
> at the top?

Done.

[commit-bot: I haz the power, 2012-08-04 00:51:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/10825187/1007

[commit-bot: I haz the power, 2012-08-04 02:58:20]

Change committed as 150004

[Wez, 2012-08-07 00:00:59]

https://chromiumcodereview.appspot.com/10825187/diff/1007/remoting/webapp/cli...
File remoting/webapp/client_session.js (right):

https://chromiumcodereview.appspot.com/10825187/diff/1007/remoting/webapp/cli...
remoting/webapp/client_session.js:103: this.logErrors_ = true;
There's a danger that we suppress logging and miss something useful; we could
continue to log in these situations but include flags in the logged stanzas to
indicate e.g. whether the client thought it was online, etc, so that we get full
raw logging and can then post-process to get the numbers we need.

https://chromiumcodereview.appspot.com/10825187/diff/1007/remoting/webapp/rem...
File remoting/webapp/remoting.js (right):

https://chromiumcodereview.appspot.com/10825187/diff/1007/remoting/webapp/rem...
remoting/webapp/remoting.js:52: remoting.clientSession.logErrors(false);
The assumption is that once we've suspended there is no way that this session
will ever resume?

[Jamie, 2012-08-07 00:37:04]

This CL has already landed; my comments are just FYI.

https://chromiumcodereview.appspot.com/10825187/diff/1007/remoting/webapp/cli...
File remoting/webapp/client_session.js (right):

https://chromiumcodereview.appspot.com/10825187/diff/1007/remoting/webapp/cli...
remoting/webapp/client_session.js:103: this.logErrors_ = true;
On 2012/08/07 00:01:00, Wez wrote:
> There's a danger that we suppress logging and miss something useful; we could
> continue to log in these situations but include flags in the logged stanzas to
> indicate e.g. whether the client thought it was online, etc, so that we get
full
> raw logging and can then post-process to get the numbers we need.

We still log eof-session, and it will still have the error attached to it, but
we don't log it as an error any more. The data we would have logged prior to
this change is inferable if we're ever interested.

https://chromiumcodereview.appspot.com/10825187/diff/1007/remoting/webapp/rem...
File remoting/webapp/remoting.js (right):

https://chromiumcodereview.appspot.com/10825187/diff/1007/remoting/webapp/rem...
remoting/webapp/remoting.js:52: remoting.clientSession.logErrors(false);
On 2012/08/07 00:01:00, Wez wrote:
> The assumption is that once we've suspended there is no way that this session
> will ever resume?

Yes. We don't invoke the callback unless the device was idle for at least 2m,
and I think the number of connections that survive that and subsequently fail
for unrelated reasons is not likely to be statistically significant.