Activation process improvements

Additional reconnect improvements. Made sure that payment page is not displayed
twice if we have previously received payment notification.

BUG=chromium-os:10075, chromium-os:10428
TEST=tough one to test - requires OTA-SP activation of number of devices until reproduces. we should make sure that we 1) don't get stuck in 'Conneting...' phase after payment and 2) we never see the payment portal twice for the same device once the payment is processed. 


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

[zel, 2011-01-14 03:26:04]

* explicit new states for OTASP and initial OTASP try
* timer to check network while in reconnection states 
* delay for failed OTASP
* secondary check for previous payment added to NetworkLibrary that will to
prevent payment page from showing twice (in addition to explicit new states that
control that for single page instance)

[Jason Glasgow, 2011-01-14 07:58:42]

I found one nit that needs to be fixed.

I couldn't devote as much time as I would like to have, and tomorrow I am out of
the office tomorrow traveling.  Maybe you want another set of eyes on the code?

In principal I agree with the expanded state space, I just did not have time to
verify the entire state machine.

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

http://codereview.chromium.org/5861001/diff/29001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:645: <<
network->service_path().c_str();
network can be NULL, so it is not okay to log network->service_path().

[zel, 2011-01-14 17:35:02]

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

http://codereview.chromium.org/5861001/diff/29001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:645: <<
network->service_path().c_str();
On 2011/01/14 07:58:42, Jason Glasgow wrote:
> network can be NULL, so it is not okay to log network->service_path().

Done.

[zel, 2011-01-14 19:12:40]

adding chocobo@ and ers@ as a reviewers

[Charlie Lee, 2011-01-14 19:26:05]

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

http://codereview.chromium.org/5861001/diff/35001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:632: if (state_ !=
PLAN_ACTIVATION_RECONNECTING_OTASP_TRY &&
Why we are returning false if state is not equal to one of these reconnecting
states?

Maybe add a comment here.

http://codereview.chromium.org/5861001/diff/35001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:804: case
PLAN_ACTIVATION_RECONNECTING:  {
nit: don't need this extra space

[zel, 2011-01-14 20:26:54]

PTAL, I was missing some files as well

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

http://codereview.chromium.org/5861001/diff/35001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:632: if (state_ !=
PLAN_ACTIVATION_RECONNECTING_OTASP_TRY &&
On 2011/01/14 19:26:05, Charlie Lee wrote:
> Why we are returning false if state is not equal to one of these reconnecting
> states?
> 
> Maybe add a comment here.

Connection business should only happen in one of RECONNECTING* states. If the
timer fires while we have moved along, we won't try to connect then.

Added comment.

http://codereview.chromium.org/5861001/diff/35001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:804: case
PLAN_ACTIVATION_RECONNECTING:  {
On 2011/01/14 19:26:05, Charlie Lee wrote:
> nit: don't need this extra space

Done.

[zel, 2011-01-14 23:48:17]

ping?


On Fri, Jan 14, 2011 at 12:26 PM, <zelidrag@chromium.org> wrote:

> PTAL, I was missing some files as well
>
>
>
>
>
http://codereview.chromium.org/5861001/diff/35001/chrome/browser/chromeos/dom...
> File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):
>
>
>
http://codereview.chromium.org/5861001/diff/35001/chrome/browser/chromeos/dom...
> chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:632: if (state_ !=
> PLAN_ACTIVATION_RECONNECTING_OTASP_TRY &&
> On 2011/01/14 19:26:05, Charlie Lee wrote:
>
>> Why we are returning false if state is not equal to one of these
>>
> reconnecting
>
>> states?
>>
>
>  Maybe add a comment here.
>>
>
> Connection business should only happen in one of RECONNECTING* states.
> If the timer fires while we have moved along, we won't try to connect
> then.
>
> Added comment.
>
>
>
>
http://codereview.chromium.org/5861001/diff/35001/chrome/browser/chromeos/dom...
> chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:804: case
> PLAN_ACTIVATION_RECONNECTING:  {
> On 2011/01/14 19:26:05, Charlie Lee wrote:
>
>> nit: don't need this extra space
>>
>
> Done.
>
>
> http://codereview.chromium.org/5861001/
>

[Charlie Lee, 2011-01-14 23:53:23]

Looks like you need to fix up MockNetworkLibrary. Please also run tests for
this.

http://codereview.chromium.org/5861001/diff/41001/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/network_library.h (right):

http://codereview.chromium.org/5861001/diff/41001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/network_library.h:582: virtual void
SignalCellularPlanPayment() = 0;
Do you need to add these methods to mock_network_library?

[zel, 2011-01-15 00:42:32]

http://codereview.chromium.org/5861001/diff/41001/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/network_library.h (right):

http://codereview.chromium.org/5861001/diff/41001/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/network_library.h:582: virtual void
SignalCellularPlanPayment() = 0;
On 2011/01/14 23:53:23, Charlie Lee wrote:
> Do you need to add these methods to mock_network_library?

Done.

[Charlie Lee, 2011-01-15 00:45:01]

LGTM