Add a pref to track progress of OOBE and resume if needed.

chrome/browser/chromeos/login/wizard_controller.cc

 // Copyright (c) 2012 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/login/wizard_controller.h"
 
 #include <signal.h>
 #include <stdlib.h>
 #include <sys/types.h>
 
@@ skipping 191 matching lines @@
     if (status == PrefService::INITIALIZATION_STATUS_ERROR) {
       OnLocalStateInitialized(false);
       return;
     } else if (status == PrefService::INITIALIZATION_STATUS_WAITING) {
       GetLocalState()->AddPrefInitObserver(
           base::Bind(&WizardController::OnLocalStateInitialized,
                      weak_factory_.GetWeakPtr()));
     }
   }
 
-  AdvanceToScreen(first_screen_name);
+  bool shoud_not_resume = false;

[Mattias Nissler (ping if slow), 2014/05/14 13:39:03]

This could just be should_not_resume = <condition-of-if-statement-below>

[pastarmovj, 2014/05/14 14:23:49]

True. This code was going through a couple of versions and I forgot to tidy it
up in the end.

+  if (is_out_of_box_ && !first_screen_name.empty() &&
+      first_screen_name != WizardController::kTestNoScreenName) {
+    shoud_not_resume = true;
+  }
+  const PrefService::Preference* oobe_stage_pref =
+      GetLocalState()->FindPreference(prefs::kOobeStagePending);

[Mattias Nissler (ping if slow), 2014/05/14 13:39:03]

it's a bit confusing that the new code calls this "stage" while everything else
calls it "screen". Maybe rename to prefs::kOobeScreenToRestartAt?

+  if (!oobe_stage_pref->IsDefaultValue() && !shoud_not_resume) {

[Mattias Nissler (ping if slow), 2014/05/14 13:39:03]

Why would you need a default value check here?

[Mattias Nissler (ping if slow), 2014/05/14 13:39:03]

nit: no curlies required.

[pastarmovj, 2014/05/14 14:23:49]

If there is no value set we don't need to intervene right.

[Mattias Nissler (ping if slow), 2014/05/14 14:54:41]

Well, you could just check for empty string I guess? In fact, doesn't the code
handle empty string just fine already? In that case, this entire thing would
work just as well in this form:

if (!should_not_resume)
  first_screen_name_ = GetLocalState()->GetString(prefs::kOobeStagePending);

[pastarmovj, 2014/05/14 15:26:44]

I think doing the check this way is more efficient isn't it and not less clear.

I don't mind changing this to a check for empty string but I would rather leave
this check here at least to keep the test code happy if not for added
belt-and-suspenders effect.

[Mattias Nissler (ping if slow), 2014/05/15 09:30:25]

I don't think efficiency matters at all here (it's hard to do something that's
not cheap).

I don't see why the test code would stop working if we just read the pref here,
and I don't see how the code you currently have (4 lines) is clearer than what I
proposed (2 lines). Unless I'm missing something and you have to actually test
for the empty string here, but I think just assigning and relying on the pref to
be registered with an empty string as the default value would do the trick?

+    first_screen_name_ = GetLocalState()->GetString(prefs::kOobeStagePending);
+  }
+
+  AdvanceToScreen(first_screen_name_);
   if (!IsMachineHWIDCorrect() && !StartupUtils::IsDeviceRegistered() &&
-      first_screen_name.empty())
+      first_screen_name_.empty())
     ShowWrongHWIDScreen();
 }
 
 chromeos::NetworkScreen* WizardController::GetNetworkScreen() {
   if (!network_screen_.get())
     network_screen_.reset(new chromeos::NetworkScreen(
         this, oobe_display_->GetNetworkScreenActor()));
   return network_screen_.get();
 }
 
@@ skipping 529 matching lines @@
   } else {
     ShowCurrentScreen();
   }
 }
 
 void WizardController::SetStatusAreaVisible(bool visible) {
   host_->SetStatusAreaVisible(visible);
 }
 
 void WizardController::AdvanceToScreen(const std::string& screen_name) {
+  // First remember how far have we reached so that we can resume if needed.

[Mattias Nissler (ping if slow), 2014/05/14 13:39:03]

Most transitions go through SetCurrentScreen instead of AdvanceToScreen, so not
sure this is good enough?

[pastarmovj, 2014/05/14 14:23:49]

Yep missed that one but now it is added and I verified that the code still
behaves. :)

+  if (is_out_of_box_)
+    StartupUtils::SaveOobePendingStage(screen_name);
+
   if (screen_name == kNetworkScreenName) {
     ShowNetworkScreen();
   } else if (screen_name == kLoginScreenName) {
     ShowLoginScreen(LoginScreenContext());
   } else if (screen_name == kUpdateScreenName) {
     InitiateOOBEUpdate();
   } else if (screen_name == kUserImageScreenName) {
     ShowUserImageScreen();
   } else if (screen_name == kEulaScreenName) {
     ShowEulaScreen();
@@ skipping 286 matching lines @@
   // cancelled on destruction.
   GetTimezoneProvider()->RequestTimezone(
       position,
       false,  // sensor
       timeout - elapsed,
       base::Bind(&WizardController::OnTimezoneResolved,
                  base::Unretained(this)));
 }
 
 }  // namespace chromeos