Make chromoting connect always work

Make chromoting connect always work

There was a race condition in the chromoting extension javascript. We wait
for OnUpdated() in order to send the chromoting tab a request for connection.
However OnUpdated() is insufficient for the chromoting tab is fully loaded.
And thus sending a request may not always work. This change will wait for the
tab to go to a "complete" status before a request is sent.

BUG=73202
TEST=Click on chromoting connect, it should always connect

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

[awong, 2011-02-18 00:31:01]

good catch!

Couple of comments

http://codereview.chromium.org/6532042/diff/1/remoting/client/extension/backg...
File remoting/client/extension/background.js (right):

http://codereview.chromium.org/6532042/diff/1/remoting/client/extension/backg...
remoting/client/extension/background.js:68: console.log("We're trying now to
send to " + tab.id);
At some point, I'm wondering if we should have a custom "log" function that
inspects a variable which can be toggled at runtime.

http://codereview.chromium.org/6532042/diff/1/remoting/client/extension/backg...
remoting/client/extension/background.js:81: // Wait for 500ms and then get the
tab and check its status.
Is there no way to have a signal on tab status change?  a 500ms loop is fine as
a stop-gap, but it's guarantees a 500ms latency in the UI.

[Alpha Left Google, 2011-02-18 00:44:06]

http://codereview.chromium.org/6532042/diff/1/remoting/client/extension/backg...
File remoting/client/extension/background.js (right):

http://codereview.chromium.org/6532042/diff/1/remoting/client/extension/backg...
remoting/client/extension/background.js:68: console.log("We're trying now to
send to " + tab.id);
On 2011/02/18 00:31:01, awong wrote:
> At some point, I'm wondering if we should have a custom "log" function that
> inspects a variable which can be toggled at runtime.

hm.. i was thinking about this too, but since we don't connect a lot we won't be
writing a lot of of messages so it's ok.

http://codereview.chromium.org/6532042/diff/1/remoting/client/extension/backg...
remoting/client/extension/background.js:81: // Wait for 500ms and then get the
tab and check its status.
On 2011/02/18 00:31:01, awong wrote:
> Is there no way to have a signal on tab status change?  a 500ms loop is fine
as
> a stop-gap, but it's guarantees a 500ms latency in the UI.

there's only two ways, pass arguments as url into chromoting_tab.js, or inspect
the tab status..

There's is a 500ms latency only if the first trial failed, we can shorten the
timeout though.

[awong, 2011-02-18 00:50:16]

Add a comment and lgtm
On Feb 17, 2011 4:44 PM, <hclam@chromium.org> wrote:
>
>
http://codereview.chromium.org/6532042/diff/1/remoting/client/extension/backg...
> File remoting/client/extension/background.js (right):
>
>
http://codereview.chromium.org/6532042/diff/1/remoting/client/extension/backg...
> remoting/client/extension/background.js:68: console.log("We're trying
> now to send to " + tab.id);
> On 2011/02/18 00:31:01, awong wrote:
>> At some point, I'm wondering if we should have a custom "log" function
> that
>> inspects a variable which can be toggled at runtime.
>
> hm.. i was thinking about this too, but since we don't connect a lot we
> won't be writing a lot of of messages so it's ok.
>
>
http://codereview.chromium.org/6532042/diff/1/remoting/client/extension/backg...
> remoting/client/extension/background.js:81: // Wait for 500ms and then
> get the tab and check its status.
> On 2011/02/18 00:31:01, awong wrote:
>> Is there no way to have a signal on tab status change? a 500ms loop
> is fine as
>> a stop-gap, but it's guarantees a 500ms latency in the UI.
>
> there's only two ways, pass arguments as url into chromoting_tab.js, or
> inspect the tab status..
>
> There's is a 500ms latency only if the first trial failed, we can
> shorten the timeout though.
>
> http://codereview.chromium.org/6532042/