Fixed host list cache and "no hosts" icon flash

If an account with hosts registered is accessed on a machine on which those hosts are cached, then the host list should not display the "no hosts" icon while the host list is being refreshed. This CL fixes a bug related to the asynchronous nature of getting the local host id for the purpose of filtering it from the host list, which caused the cached host list never to be displayed.

Note that if the account has hosts registered, but is accessed from a computer with no cache, then the "no hosts" icon will still flash briefly (although it will generally be hidden behind the infographic).

BUG=135288
TEST=Manual

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=146200

[Jamie, 2012-07-11 00:58:33]

ptal

[Wez, 2012-07-11 15:56:31]

LGTM, but please clarify in the CL description why it's unnecessary.

[Jamie, 2012-07-11 17:04:46]

On further inspection, it was intended to display the cached host list, but
didn't work for the reasons described in the CL description. I've updated the CL
to fix that problem, which also fixes the linked bug.

PTAL.

[Jamie, 2012-07-11 19:58:29]

As discussed, here's a solution that fixes the problem by not clearing the host
list until a response is received from the server, or an error occurs. PTAL.

[Wez, 2012-07-11 20:48:10]

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/host_list.js
File remoting/webapp/host_list.js (left):

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/host_list.j...
remoting/webapp/host_list.js:123: this.lastError_ = '';
Are you relying on the fact that lastError_ will only be checked if
onDone(false) is called, i.e. in the onError() case?

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/host_list.js
File remoting/webapp/host_list.js (right):

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/host_list.j...
remoting/webapp/host_list.js:137: this.hosts_ = [];
I think you need to clear lastError_ here to avoid the onDone() call later on
getting confused by seeing lastError_ values from previous failures?

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/remoting.js
File remoting/webapp/remoting.js (right):

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/remoting.js...
remoting/webapp/remoting.js:96: // Display the cached host list, then refresh
and display the current state.
nit: Display the cached host list, and asynchronously refresh & update it.

[Jamie, 2012-07-11 20:53:10]

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/host_list.js
File remoting/webapp/host_list.js (left):

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/host_list.j...
remoting/webapp/host_list.js:123: this.lastError_ = '';
On 2012/07/11 20:48:10, Wez wrote:
> Are you relying on the fact that lastError_ will only be checked if
> onDone(false) is called, i.e. in the onError() case?

No, I think your next comment is correct, and I should be resetting it when we
get a response.

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/host_list.js
File remoting/webapp/host_list.js (right):

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/host_list.j...
remoting/webapp/host_list.js:137: this.hosts_ = [];
On 2012/07/11 20:48:10, Wez wrote:
> I think you need to clear lastError_ here to avoid the onDone() call later on
> getting confused by seeing lastError_ values from previous failures?

Done.

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/remoting.js
File remoting/webapp/remoting.js (right):

http://codereview.chromium.org/10736020/diff/6002/remoting/webapp/remoting.js...
remoting/webapp/remoting.js:96: // Display the cached host list, then refresh
and display the current state.
On 2012/07/11 20:48:10, Wez wrote:
> nit: Display the cached host list, and asynchronously refresh & update it.

Done (more or less).

[Wez, 2012-07-11 20:53:49]

lgtm