Show progress bar as soon as update state changed to UPDATE_STATUS_UPDATE_AVAILABLE

chrome/browser/chromeos/login/update_screen.cc

 // Copyright (c) 2011 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/update_screen.h"
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/threading/thread_restrictions.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
@@ skipping 46 matching lines @@
   return instance_set;
 }
 
 // static
 bool UpdateScreen::HasInstance(UpdateScreen* inst) {
   InstanceSet& instance_set = GetInstanceSet();
   InstanceSet::iterator found = instance_set.find(inst);
   return (found != instance_set.end());
 }
 
-
 UpdateScreen::UpdateScreen(ScreenObserver* screen_observer,
                            UpdateScreenActor* actor)
     : WizardScreen(screen_observer),
       reboot_check_delay_(0),
       is_checking_for_update_(true),
       is_downloading_update_(false),
       is_ignore_update_deadlines_(false),
       is_shown_(false),
+      ignore_idle_status_(true),
       actor_(actor) {
   GetInstanceSet().insert(this);
 }
 
 UpdateScreen::~UpdateScreen() {
   CrosLibrary::Get()->GetUpdateLibrary()->RemoveObserver(this);
   GetInstanceSet().erase(this);
 }
 
 void UpdateScreen::UpdateStatusChanged(UpdateLibrary* library) {
   UpdateStatusOperation status = library->status().status;
   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:
       // Do nothing in these cases, we don't want to notify the user of the
       // check unless there is an update.
       break;
     case UPDATE_STATUS_UPDATE_AVAILABLE:
       MakeSureScreenIsShown();
       actor_->SetProgress(kBeforeDownloadProgress);
       if (!HasCriticalUpdate()) {
         LOG(INFO) << "Noncritical update available: "
                   << library->status().new_version;
         ExitUpdate(REASON_UPDATE_NON_CRITICAL);
       } else {
         LOG(INFO) << "Critical update available: "
                   << library->status().new_version;
+        actor_->ShowPreparingUpdatesInfo(true);
+        actor_->ShowCurtain(false);
       }
       break;
     case UPDATE_STATUS_DOWNLOADING:
       {
         MakeSureScreenIsShown();
         if (!is_downloading_update_) {
           // Because update engine doesn't send UPDATE_STATUS_UPDATE_AVAILABLE
           // we need to is update critical on first downloading notification.
           is_downloading_update_ = true;
           if (!HasCriticalUpdate()) {
             LOG(INFO) << "Non-critical update available: "
                       << library->status().new_version;
             ExitUpdate(REASON_UPDATE_NON_CRITICAL);
           } 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);
       }
       break;
     case UPDATE_STATUS_VERIFYING:
       MakeSureScreenIsShown();
       actor_->SetProgress(kBeforeVerifyingProgress);
       break;
     case UPDATE_STATUS_FINALIZING:
@@ skipping 10 matching lines @@
         VLOG(1) << "Initiate reboot after update";
         CrosLibrary::Get()->GetUpdateLibrary()->RebootAfterUpdate();
         reboot_timer_.Start(base::TimeDelta::FromSeconds(reboot_check_delay_),
                             this,
                             &UpdateScreen::OnWaitForRebootTimeElapsed);
       } else {
         ExitUpdate(REASON_UPDATE_NON_CRITICAL);
       }
       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);
       break;
     default:
       NOTREACHED();
       break;
   }
 }
 
@@ skipping 103 matching lines @@
       deadline.empty()) {
     return false;
   }
 
   // TODO(dpolukhin): Analyze file content. Now we can just assume that
   // if the file exists and not empty, there is critical update.
   return true;
 }
 
 }  // namespace chromeos