Improved activation logic to cover few more edge cases. Removed button that w...

chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc

 // Copyright (c) 2006-2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/dom_ui/mobile_setup_ui.h"
 
 #include <algorithm>
 #include <map>
 #include <string>
 
@@ skipping 391 matching lines @@
       static_cast<chromeos::CellularNetwork*>(
           const_cast<chromeos::Network*>(network)));
 }
 
 void MobileSetupHandler::HandleCloseTab(const ListValue* args) {
   if (!dom_ui_)
     return;
   Browser* browser = BrowserList::FindBrowserWithFeature(
       dom_ui_->GetProfile(), Browser::FEATURE_TABSTRIP);
   if (browser)
     browser->CloseTabContents(tab_contents_);

[Eric Shienbrood, 2010/11/16 15:57:19]

Shouldn't wifi and ethernet be re-enabled here?

[Eric Shienbrood, 2010/11/16 16:22:04]

Actually, shouldn't it be calling CompleteActivation?

[zel, 2010/11/16 18:59:28]

This is dead code, we don't use it at all. Removed.

 }
 
 void MobileSetupHandler::HandleStartActivation(const ListValue* args) {
   scoped_refptr<TaskProxy> task = new TaskProxy(AsWeakPtr());
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(task.get(), &TaskProxy::HandleStartActivation));
 }
 
 void MobileSetupHandler::HandleSetTransactionStatus(const ListValue* args) {
   const size_t kSetTransactionStatusParamCount = 1;
@@ skipping 65 matching lines @@
           << "\n  activation=" << network->GetActivationStateString().c_str()
           << "\n  restricted=" << (network->restricted_pool() ? "yes" : "no")
           << "\n  error=" << network->GetErrorString().c_str()
           << "\n  setvice_path=" << network->service_path().c_str();
   switch (state_) {
     case PLAN_ACTIVATION_START: {
       switch (network->activation_state()) {
         case chromeos::ACTIVATION_STATE_ACTIVATED: {
           if (network->failed_or_disconnected()) {
             new_state = PLAN_ACTIVATION_RECONNECTING;
           } else if (network->connection_state() == chromeos::STATE_READY) {

[Eric Shienbrood, 2010/11/16 15:57:19]

This can be "... if (network->connected()) {". There are several more
occurrences of this below.

[zel, 2010/11/16 18:59:28]

Done.

             if (network->restricted_pool()) {
               new_state = PLAN_ACTIVATION_SHOWING_PAYMENT;
             } else {
               new_state = PLAN_ACTIVATION_DONE;
             }
           }
           break;
         }
         case chromeos::ACTIVATION_STATE_PARTIALLY_ACTIVATED: {
           if (!activation_status_test_) {
             if (network->failed_or_disconnected()) {
               activation_status_test_ = true;
               new_state = PLAN_ACTIVATION_INITIATING_ACTIVATION;
             } else if (network->connected()) {
               LOG(INFO) << "Disconnecting from " <<
                   network->service_path().c_str();
               chromeos::CrosLibrary::Get()->GetNetworkLibrary()->
                   DisconnectFromWirelessNetwork(network);
               // Disconnect will force networks to be reevaluated, so
               // we don't want to continue processing on this path anymore.
               return;
             }
           } else {
             if (network->connected())
               new_state = PLAN_ACTIVATION_SHOWING_PAYMENT;

[jglasgow, 2010/11/16 13:37:35]

Does this also need a check for restricted_pool()?  Maybe this is the path with
the DNS error.

[Eric Shienbrood, 2010/11/16 15:57:19]

Or change the check to network->is_active(), which is what I thought you were
going to do. This would be quicker than waiting for restricted_pool to be set.

[zel, 2010/11/16 18:59:28]

Done.

             else
               new_state = PLAN_ACTIVATION_RECONNECTING;
             break;
           }
           break;
         }
         case chromeos::ACTIVATION_STATE_UNKNOWN:
         case chromeos::ACTIVATION_STATE_NOT_ACTIVATED: {
           if (network->failed_or_disconnected()) {
             new_state = PLAN_ACTIVATION_INITIATING_ACTIVATION;
           } else if (network->connected()) {
             LOG(INFO) << "Disconnecting from " <<
                 network->service_path().c_str();
             chromeos::CrosLibrary::Get()->GetNetworkLibrary()->
                 DisconnectFromWirelessNetwork(network);
             // Disconnect will force networks to be reevaluated, so
             // we don't want to continue processing on this path anymore.
             return;
           }
           break;
         }
         default: {
           new_state = PLAN_ACTIVATION_INITIATING_ACTIVATION;

[jglasgow, 2010/11/16 13:37:35]

Looks like this could be called while connected -- which means that we could get
an EIO error from flimflam.

[Eric Shienbrood, 2010/11/16 15:57:19]

I'm not sure I see the problem here. The only other activation state is
ACTIVATION_STATE_ACTIVATING. I don't see how we'd get here when the UI state is
PLAN_ACTIVATION_START.

[zel, 2010/11/16 18:59:28]

This wasn't what was happening, but I have still removed this since we should
never end up here.

           break;
         }
       }
       break;
     }
     case PLAN_ACTIVATION_INITIATING_ACTIVATION: {
       switch (network->activation_state()) {
         case chromeos::ACTIVATION_STATE_ACTIVATED:
           if (network->failed_or_disconnected()) {
             new_state = PLAN_ACTIVATION_RECONNECTING;
           } else if (network->connection_state() == chromeos::STATE_READY) {
             if (network->restricted_pool()) {
               new_state = PLAN_ACTIVATION_SHOWING_PAYMENT;
             } else {
               new_state = PLAN_ACTIVATION_DONE;
             }
           }
           break;
         case chromeos::ACTIVATION_STATE_PARTIALLY_ACTIVATED:
-          if (network->connected())
-            new_state = PLAN_ACTIVATION_SHOWING_PAYMENT;
-          else
+          if (network->connected()) {
+            if (network->restricted_pool())
+              new_state = PLAN_ACTIVATION_SHOWING_PAYMENT;
+          } else {
             new_state = PLAN_ACTIVATION_RECONNECTING;
+          }
           break;
         case chromeos::ACTIVATION_STATE_NOT_ACTIVATED:
           // Wait in this state until activation state changes.
           break;
         case chromeos::ACTIVATION_STATE_ACTIVATING:
           break;
         default:
           break;
       }
       break;
     }
     case PLAN_ACTIVATION_RECONNECTING: {
       // Wait until the service shows up and gets activated.
       switch (network->activation_state()) {
         case chromeos::ACTIVATION_STATE_ACTIVATED:
           if (network->connection_state() == chromeos::STATE_READY) {
             if (network->restricted_pool()) {
               new_state = PLAN_ACTIVATION_SHOWING_PAYMENT;
             } else {
               new_state = PLAN_ACTIVATION_DONE;
             }
+          } else if (network->failed()) {
+            chromeos::CrosLibrary::Get()->GetNetworkLibrary()->
+                ConnectToCellularNetwork(network);
+            return;
           }
           break;
         case chromeos::ACTIVATION_STATE_PARTIALLY_ACTIVATED:
           if (network->connected()) {
             if (network->restricted_pool())
               new_state = PLAN_ACTIVATION_SHOWING_PAYMENT;
           }
           break;
         default:
           break;
       }
       break;
     }
     case PLAN_ACTIVATION_PAGE_LOADING:
       break;
     // Just ignore all signals until the site confirms payment.
     case PLAN_ACTIVATION_SHOWING_PAYMENT:
       // Activation completed/failed, ignore network changes.
     case PLAN_ACTIVATION_DONE:
     case PLAN_ACTIVATION_ERROR:
       break;
   }
 
   std::string error_description;
   if (GotActivationError(network, &error_description)) {
     // Check for this special case when we try to do activate partially

[Eric Shienbrood, 2010/11/16 15:57:19]

"...when we try to activate partially..."

     // activated device. If that attempt failed, try to disconnect to clear the
     // state and reconnect again.
     if ((network->activation_state() ==
             chromeos::ACTIVATION_STATE_PARTIALLY_ACTIVATED ||
         network->activation_state() == chromeos::ACTIVATION_STATE_ACTIVATING) &&
-        network->error() == chromeos::ERROR_UNKNOWN &&
+        (network->error() == chromeos::ERROR_UNKNOWN ||
+            network->error() == chromeos::ERROR_OTASP_FAILED)&&
         (state_ == PLAN_ACTIVATION_INITIATING_ACTIVATION ||
             state_ == PLAN_ACTIVATION_RECONNECTING) &&
         activation_status_test_ &&
         network->connection_state() == chromeos::STATE_ACTIVATION_FAILURE) {
       new_state = PLAN_ACTIVATION_RECONNECTING;
     } else {
       new_state = PLAN_ACTIVATION_ERROR;
     }
   }
   ChangeState(network, new_state, error_description);
@@ skipping 63 matching lines @@
       kJsDeviceStatusChangedHandler, device_dict);
 
   // Decide what to do with network object as a result of the new state.
   switch (new_state) {
     case PLAN_ACTIVATION_START:
       break;
     case PLAN_ACTIVATION_INITIATING_ACTIVATION:
       DCHECK(network);
       LOG(INFO) << "Activating service " << network->service_path().c_str();
       if (!network->StartActivation())
         new_state = PLAN_ACTIVATION_ERROR;

[Jason Glasgow, 2010/11/16 16:13:51]

This seems to be dead code.

I'm not sure... should this be 

state_ = PLAN_ACTIVATION_ERROR? 

But I'm not sure that this will be handled correctly in EvaluateCellularNetwork
because the call failed more or less at the level of checking arguments, and
network->activate_state() will remain unchanged (i.e. it will not be state
activation error - we could argue this point).

[zel, 2010/11/16 18:59:28]

Done.

       break;
     case PLAN_ACTIVATION_RECONNECTING: {
       DCHECK(network);
       if (network) {
         chromeos::CrosLibrary::Get()->GetNetworkLibrary()->
             ConnectToCellularNetwork(network);
       }
       break;
     }
     case PLAN_ACTIVATION_PAGE_LOADING:
@@ skipping 167 matching lines @@
       new MobileSetupUIHTMLSource(service_path);
 
   // Set up the chrome://mobilesetup/ source.
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       NewRunnableMethod(
           Singleton<ChromeURLDataManager>::get(),
           &ChromeURLDataManager::AddDataSource,
           make_scoped_refptr(html_source)));
 }