Added [Close] button on completed activation overlay div....

Added [Close] button on completed activation overlay div.
Included logic for connection recovery - if we fail or timeout (30 sec)
while trying to reconnect the process of reconnecting will be
repeated 10 times before we throw an error.

BUG=chromium-os:9304, chromium-os:9333
TEST=none


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

[Charlie Lee, 2010-11-18 02:30:21]

LGTM

[oshima, 2010-11-18 02:44:56]

lgtm

[zel, 2010-11-18 04:08:42]

jglasgow@ and ers@, please review reconnection logic that has been added to this
CL.

HTML+JS parts have been already reviewed.

[Jason Glasgow, 2010-11-18 04:31:12]

Really just a few nits.  

I think this makes sense given the schedule, but I'd also like to ensure that
flimflam always transitions the service/network to failure after a failure to
connect.

It is of questionable utility to have the UI time out connect requests (unless
we want to argue that flimflam could disappear).  Flimflam should provide a
guarantee that every connect call leads either to a FAILURE state, or a READY
state.

Also, not mentioned below, but modem APIs sometimes have 5 minute (yes, 300
second) timeouts for connect operations.

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:490:
UMA_HISTOGRAM_COUNTS("Cellular.ConnectionRetry", 1);
Maybe you do not want this UMA call, since you also make the call below, and
ConnectToNetwork is also called on the first try, which is NOT a retry.

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:637: // reasons we are still
connected yet after all this time.
hard to parse comment.  Maybe you mean
... if for some reason we are still not connected after all this time.

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:640:
UMA_HISTOGRAM_COUNTS("Cellular.ConnectionRetry", 1);
The UMA call is also in ConnectToNetwork().

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:758: DCHECK(network);
I think DCHECK belongs at start of this function.

[zel, 2010-11-18 04:39:51]

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:490:
UMA_HISTOGRAM_COUNTS("Cellular.ConnectionRetry", 1);
On 2010/11/18 04:31:12, Jason Glasgow wrote:
> Maybe you do not want this UMA call, since you also make the call below, and
> ConnectToNetwork is also called on the first try, which is NOT a retry.

Done.

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:637: // reasons we are still
connected yet after all this time.
On 2010/11/18 04:31:12, Jason Glasgow wrote:
> hard to parse comment.  Maybe you mean
> ... if for some reason we are still not connected after all this time.

Done.

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:640:
UMA_HISTOGRAM_COUNTS("Cellular.ConnectionRetry", 1);
On 2010/11/18 04:31:12, Jason Glasgow wrote:
> The UMA call is also in ConnectToNetwork().

Removed call from from ConnectToNetwork()

http://codereview.chromium.org/5147004/diff/14001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:758: DCHECK(network);
On 2010/11/18 04:31:12, Jason Glasgow wrote:
> I think DCHECK belongs at start of this function.

I am not sure about that. We call also this method above directly at
MobileSetupHandler::StartActivation() with:

ChangeState(NULL, PLAN_ACTIVATION_ERROR, std::string());

[jglasgow, 2010-11-18 05:03:37]

LGTM (Agree on your DCHECK pushback.  I missed the other call).

[Charlie Lee, 2010-11-18 05:16:03]

LGTM