Show progress bar as soon as update state changed to UPDATE_STATUS_UPDATE_AVAILABLE

chrome/browser/chromeos/login/update_screen.cc

Index: chrome/browser/chromeos/login/update_screen.cc
diff --git a/chrome/browser/chromeos/login/update_screen.cc b/chrome/browser/chromeos/login/update_screen.cc
index 7d04b0269a85853cc949449627ff5b9cf459b767..4af72bb719152d46bada11f802a2febe6e627f0a 100644
--- a/chrome/browser/chromeos/login/update_screen.cc
+++ b/chrome/browser/chromeos/login/update_screen.cc
@@ -64,7 +64,6 @@ bool UpdateScreen::HasInstance(UpdateScreen* inst) {
   return (found != instance_set.end());
 }
 
-
 UpdateScreen::UpdateScreen(ScreenObserver* screen_observer,
                            UpdateScreenActor* actor)
     : WizardScreen(screen_observer),
@@ -73,6 +72,7 @@ UpdateScreen::UpdateScreen(ScreenObserver* screen_observer,
       is_downloading_update_(false),
       is_ignore_update_deadlines_(false),
       is_shown_(false),
+      ignore_idle_status_(true),
       actor_(actor) {
   GetInstanceSet().insert(this);
 }
@@ -87,6 +87,9 @@ void UpdateScreen::UpdateStatusChanged(UpdateLibrary* library) {
   if (is_checking_for_update_ && status > UPDATE_STATUS_CHECKING_FOR_UPDATE) {
     is_checking_for_update_ = false;
   }
+  if (ignore_idle_status_ && status > UPDATE_STATUS_IDLE) {

[Nikita (slow), 2011/06/07 15:31:03]

This approach feels a bit dangerous because it relies on the fact that
UPDATE_STATUS_CHECKING_FOR_UPDATE is sent at least once.
I think it's the case but if it regresses (1) would make stuck on this screen.

(1) No update, no error
IDLE > CHECKING_FOR_UPDATE > IDLE

(2) Has update, installed
IDLE > CHECKING_FOR_UPDATE > ... > UPDATED_NEED_REBOOT

(3) Error
IDLE > CHECKING_FOR_UPDATE > ... > ERROR

[Dmitry Polukhin, 2011/06/08 07:24:21]

My approach assumes that status other than IDLE will be sent at least once
before getting back to IDLE. I added code to set ignore_idle_status_ to callback
from RequestUpdateCheck. But in this case if initial IDLE status is sent after
callback from RequestUpdateCheck, update screen will be skipped. Overall with
current update API it is very hard to get result of your updated check. It would
be better if updated engine directly notifies caller (via callback passed to
UpdateCheck) about status of its update check.

[petkov, 2011/06/08 18:29:00]

You could use update_engine's GetStatus D-Bus method to inquire about its status
at any given point -- I think there's already libcros support for it. If the
status is IDLE _after_ you've called AttemptUpdate then there's definitely no
update. Does this solve the issue?

[Dmitry Polukhin, 2011/06/09 12:51:32]

Is it guaranteed that update engine will not delay update ping for some time and
change status on return from AttemptUpdate? Also I'm worry about async
communication between Chrome and update engine i.e. Chrome may use old cached
update status. If updated engine always sends CHECKING_FOR_UPDATE status,
current approach should be reliable. As far as I can see in update engine code
it always sends CHECKING_FOR_UPDATE status.

[petkov, 2011/06/09 13:24:20]

That's right. As long as the status is IDLE when the AttemptUpdate call is
received, it's guaranteed that update_engine will send CHECKING_FOR_UPDATE
before returning from the AttemptUpdate call.

+    ignore_idle_status_ = false;
+  }
 
   switch (status) {
     case UPDATE_STATUS_CHECKING_FOR_UPDATE:
@@ -103,6 +106,8 @@ void UpdateScreen::UpdateStatusChanged(UpdateLibrary* library) {
       } else {
         LOG(INFO) << "Critical update available: "
                   << library->status().new_version;
+        actor_->ShowPreparingUpdatesInfo(true);
+        actor_->ShowCurtain(false);
       }
       break;
     case UPDATE_STATUS_DOWNLOADING:
@@ -119,9 +124,10 @@ void UpdateScreen::UpdateStatusChanged(UpdateLibrary* library) {
           } else {
             LOG(INFO) << "Critical update available: "
                       << library->status().new_version;
+            actor_->ShowPreparingUpdatesInfo(false);
+            actor_->ShowCurtain(false);
           }
         }
-        actor_->ShowCurtain(false);
         int download_progress = static_cast<int>(
             library->status().download_progress * kDownloadProgressIncrement);
         actor_->SetProgress(kBeforeDownloadProgress + download_progress);
@@ -152,6 +158,12 @@ void UpdateScreen::UpdateStatusChanged(UpdateLibrary* library) {
       }
       break;
     case UPDATE_STATUS_IDLE:
+      if (ignore_idle_status_) {
+        // It is first IDLE status that is sent before we initiated the check.
+        break;
+      }
+      // else no break
+
     case UPDATE_STATUS_ERROR:
     case UPDATE_STATUS_REPORTING_ERROR_EVENT:
       ExitUpdate(REASON_UPDATE_ENDED);