About box auto-update improvements.

The About box now knows how to check to see if updates have been installed
in the background without anyone having to click the Update button in the box.

The About box no longer gets stuck in the "installed" state.  Even if an
update has been installed, the About box will still check for new updates when
reopened.

BUG=13165, 20488
TEST=Play with the about box and auto-update a whole lot
Review URL: http://codereview.chromium.org/338012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30078 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 326 insertions, 123 deletions

diff --git a/chrome/app/keystone_glue.h b/chrome/app/keystone_glue.h
index 9a8eb01..f8d2a53 100644
--- a/chrome/app/keystone_glue.h
+++ b/chrome/app/keystone_glue.h
@@ -6,27 +6,30 @@
 #define CHROME_APP_KEYSTONE_GLUE_H_
 
 #import <Foundation/Foundation.h>
-
-// Objects which request callbacks from KeystoneGlue (e.g. information
-// on update availability) should implement this protocol.  All callbacks
-// require the caller to be spinning in the runloop to happen.
-@protocol KeystoneGlueCallbacks
-
-// Callback when a checkForUpdate completes.
-// |latestVersion| may be nil if not returned from the server.
-// |latestVersion| is not a localizable string.
-- (void)upToDateCheckCompleted:(BOOL)upToDate
-                 latestVersion:(NSString*)latestVersion;
-
-// Callback when a startUpdate completes.
-// |successful| tells if the *check* was successful.  This does not
-//   necessarily mean updates installed successfully.
-// |installs| tells the number of updates that installed successfully
-//   (typically 0 or 1).
-- (void)updateCompleted:(BOOL)successful installs:(int)installs;
-
-@end  // protocol KeystoneGlueCallbacks
-
+#import <base/scoped_nsobject.h>
+
+// Possible outcomes of -checkForUpdate and -installUpdate.  A version may
+// accompany some of these, but beware: a version is never required.  For
+// statuses that can be accompanied by a version, the comment indicates what
+// version is referenced.
+enum AutoupdateStatus {
+  kAutoupdateCurrent = 0,   // version of the running application
+  kAutoupdateAvailable,     // version of the update that is available
+  kAutoupdateInstalled,     // version of the update that was installed
+  kAutoupdateCheckFailed,   // no version
+  kAutoupdateInstallFailed  // no version
+};
+
+// kAutoupdateStatusNotification is the name of the notification posted when
+// -checkForUpdate and -installUpdate complete.  This notification will be
+// sent with with its sender object set to the KeystoneGlue instance sending
+// the notification.  Its userInfo dictionary will contain an AutoupdateStatus
+// value as an intValue at key kAutoupdateStatusStatus.  If a version is
+// available (see AutoupdateStatus), it will be present at key
+// kAutoupdateStatusVersion.
+extern const NSString* const kAutoupdateStatusNotification;
+extern const NSString* const kAutoupdateStatusStatus;
+extern const NSString* const kAutoupdateStatusVersion;
 
 // KeystoneGlue is an adapter around the KSRegistration class, allowing it to
 // be used without linking directly against its containing KeystoneRegistration
@@ -55,10 +58,11 @@
   KSRegistration* registration_;  // strong
   NSTimer* timer_;  // strong
 
-  // Data for callbacks, all strong.  Deallocated (if needed) in a
-  // NSNotificationCenter callback.
-  NSObject<KeystoneGlueCallbacks>* startTarget_;
-  NSObject<KeystoneGlueCallbacks>* checkTarget_;
+  // The most recent kAutoupdateStatusNotification notification posted.
+  scoped_nsobject<NSNotification> recentNotification_;
+
+  // YES if an update was ever successfully installed by -installUpdate.
+  BOOL updateSuccessfullyInstalled_;
 }
 
 // Return the default Keystone Glue object.
@@ -68,21 +72,27 @@
 // with Keystone, and set up periodic activity pings.
 - (void)registerWithKeystone;
 
-// Check if updates are available.
-// upToDateCheckCompleted:: called on target when done.
-// Return NO if we could not start the check.
-- (BOOL)checkForUpdate:(NSObject<KeystoneGlueCallbacks>*)target;
+// -checkForUpdate launches a check for updates, and -installUpdate begins
+// installing an available update.  For each, status will be communicated via
+// a kAutoupdateStatusNotification notification, and will also be available
+// through -recentUpdateStatus.
+- (void)checkForUpdate;
+- (void)installUpdate;
+
+// Accessor for recentNotification_.  Returns an autoreleased NSNotification.
+- (NSNotification*)recentNotification;
 
-// Start an update.
-// updateCompleted:: called on target when done.
-// This cannot be cancelled.
-// Return NO if we could not start the check.
-- (BOOL)startUpdate:(NSObject<KeystoneGlueCallbacks>*)target;
+// Clears the saved recentNotification_.
+- (void)clearRecentNotification;
 
-@end  // KeystoneGlue
+@end  // @interface KeystoneGlue
 
+@interface KeystoneGlue(ExposedForTesting)
 
-@interface KeystoneGlue (ExposedForTesting)
+// Release the shared instance.  Use this in tests to reset the shared
+// instance in case strange things are done to it for testing purposes.  Never
+// call this from non-test code.
++ (void)releaseDefaultKeystoneGlue;
 
 // Load any params we need for configuring Keystone.
 - (void)loadParameters;
@@ -94,11 +104,11 @@
 - (void)stopTimer;
 
 // Called when a checkForUpdate: notification completes.
-- (void)checkComplete:(NSNotification *)notification;
+- (void)checkForUpdateComplete:(NSNotification*)notification;
 
-// Called when a startUpdate: notification completes.
-- (void)startUpdateComplete:(NSNotification *)notification;
+// Called when an installUpdate: notification completes.
+- (void)installUpdateComplete:(NSNotification*)notification;
 
-@end  // KeystoneGlue (ExposedForTesting)
+@end  // @interface KeystoneGlue(ExposedForTesting)
 
 #endif  // CHROME_APP_KEYSTONE_GLUE_H_
diff --git a/chrome/app/keystone_glue.mm b/chrome/app/keystone_glue.mm
index 736972c..689cfdb 100644
--- a/chrome/app/keystone_glue.mm
+++ b/chrome/app/keystone_glue.mm
@@ -2,16 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/mac_util.h"
 #import "chrome/app/keystone_glue.h"
 
-@interface KeystoneGlue(Private)
-
-// Called periodically to announce activity by pinging the Keystone server.
-- (void)markActive:(NSTimer*)timer;
-
-@end
-
+#include "base/logging.h"
+#include "base/mac_util.h"
+#import "base/worker_pool_mac.h"
+#include "chrome/common/chrome_constants.h"
 
 // Provide declarations of the Keystone registration bits needed here.  From
 // KSRegistration.h.
@@ -31,7 +27,9 @@ NSString *KSRegistrationRemoveExistingTag = @"";
 #define KSRegistrationPreserveExistingTag nil
 
 @interface KSRegistration : NSObject
+
 + (id)registrationWithProductID:(NSString*)productID;
+
 // Older API
 - (BOOL)registerWithVersion:(NSString*)version
        existenceCheckerType:(KSExistenceCheckerType)xctype
@@ -43,20 +41,65 @@ NSString *KSRegistrationRemoveExistingTag = @"";
      existenceCheckerString:(NSString*)xc
             serverURLString:(NSString*)serverURLString
             preserveTTToken:(BOOL)preserveToken
-                        tag:(NSString *)tag;
+                        tag:(NSString*)tag;
+
 - (void)setActive;
 - (void)checkForUpdate;
 - (void)startUpdate;
-@end
 
+@end  // @interface KSRegistration
+
+@interface KeystoneGlue(Private)
+
+// Called periodically to announce activity by pinging the Keystone server.
+- (void)markActive:(NSTimer*)timer;
+
+// Called when an update check or update installation is complete.  Posts the
+// kAutoupdateStatusNotification notification to the default notification
+// center.
+- (void)updateStatus:(AutoupdateStatus)status version:(NSString*)version;
+
+// These three methods are used to determine the version of the application
+// currently installed on disk, compare that to the currently-running version,
+// decide whether any updates have been installed, and call
+// -updateStatus:version:.
+//
+// In order to check the version on disk, the installed application's
+// Info.plist dictionary must be read; in order to see changes as updates are
+// applied, the dictionary must be read each time, bypassing any caches such
+// as the one that NSBundle might be maintaining.  Reading files can be a
+// blocking operation, and blocking operations are to be avoided on the main
+// thread.  I'm not quite sure what jank means, but I bet that a blocked main
+// thread would cause some of it.
+//
+// -determineUpdateStatusAsync is called on the main thread to initiate the
+// operation.  It performs initial set-up work that must be done on the main
+// thread and arranges for -determineUpdateStatusAtPath: to be called on a
+// work queue thread managed by NSOperationQueue.
+// -determineUpdateStatusAtPath: then reads the Info.plist, gets the version
+// from the CFBundleShortVersionString key, and performs
+// -determineUpdateStatusForVersion: on the main thread.
+// -determineUpdateStatusForVersion: does the actual comparison of the version
+// on disk with the running version and calls -updateStatus:version: with the
+// results of its analysis.
+- (void)determineUpdateStatusAsync;
+- (void)determineUpdateStatusAtPath:(NSString*)appPath;
+- (void)determineUpdateStatusForVersion:(NSString*)version;
+
+@end  // @interface KeystoneGlue(Private)
+
+const NSString* const kAutoupdateStatusNotification =
+    @"AutoupdateStatusNotification";
+const NSString* const kAutoupdateStatusStatus = @"status";
+const NSString* const kAutoupdateStatusVersion = @"version";
 
 @implementation KeystoneGlue
 
-+ (id)defaultKeystoneGlue {
-  // TODO(jrg): use base::SingletonObjC<KeystoneGlue>
-  static KeystoneGlue* sDefaultKeystoneGlue = nil;  // leaked
+// TODO(jrg): use base::SingletonObjC<KeystoneGlue>
+static KeystoneGlue* sDefaultKeystoneGlue = nil;  // leaked
 
-  if (sDefaultKeystoneGlue == nil) {
++ (id)defaultKeystoneGlue {
+  if (!sDefaultKeystoneGlue) {
     sDefaultKeystoneGlue = [[KeystoneGlue alloc] init];
     [sDefaultKeystoneGlue loadParameters];
     if (![sDefaultKeystoneGlue loadKeystoneRegistration]) {
@@ -67,6 +110,29 @@ NSString *KSRegistrationRemoveExistingTag = @"";
   return sDefaultKeystoneGlue;
 }
 
++ (void)releaseDefaultKeystoneGlue {
+  [sDefaultKeystoneGlue release];
+  sDefaultKeystoneGlue = nil;
+}
+
+- (id)init {
+  if ((self = [super init])) {
+    NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
+
+    [center addObserver:self
+               selector:@selector(checkForUpdateComplete:)
+                   name:KSRegistrationCheckForUpdateNotification
+                 object:nil];
+
+    [center addObserver:self
+               selector:@selector(installUpdateComplete:)
+                   name:KSRegistrationStartUpdateNotification
+                 object:nil];
+  }
+
+  return self;
+}
+
 - (void)dealloc {
   [url_ release];
   [productID_ release];
@@ -174,64 +240,155 @@ NSString *KSRegistrationRemoveExistingTag = @"";
   [ksr setActive];
 }
 
-- (void)checkComplete:(NSNotification *)notification {
-  NSDictionary *userInfo = [notification userInfo];
-  BOOL updatesAvailable = [[userInfo objectForKey:KSRegistrationStatusKey]
-                            boolValue];
-  NSString *latestVersion = [userInfo objectForKey:KSRegistrationVersionKey];
+- (void)checkForUpdate {
+  if (!registration_) {
+    [self updateStatus:kAutoupdateCheckFailed version:nil];
+    return;
+  }
 
-  [checkTarget_ upToDateCheckCompleted:updatesAvailable
-                         latestVersion:latestVersion];
-  [checkTarget_ release];
-  checkTarget_ = nil;
+  [registration_ checkForUpdate];
 
-  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
-  [center removeObserver:self
-                    name:KSRegistrationCheckForUpdateNotification
-                  object:nil];
+  // Upon completion, KSRegistrationCheckForUpdateNotification will be posted,
+  // and -checkForUpdateComplete: will be called.
 }
 
-- (BOOL)checkForUpdate:(NSObject<KeystoneGlueCallbacks>*)target {
-  if (registration_ == nil)
-    return NO;
-  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
-  [center addObserver:self
-             selector:@selector(checkComplete:)
-                 name:KSRegistrationCheckForUpdateNotification
-               object:nil];
-  checkTarget_ = [target retain];
-  [registration_ checkForUpdate];
-  return YES;
+- (void)checkForUpdateComplete:(NSNotification*)notification {
+  NSDictionary* userInfo = [notification userInfo];
+  BOOL updatesAvailable =
+      [[userInfo objectForKey:KSRegistrationStatusKey] boolValue];
+
+  if (updatesAvailable) {
+    // If an update is known to be available, go straight to
+    // -updateStatus:version:.  It doesn't matter what's currently on disk.
+    NSString* version = [userInfo objectForKey:KSRegistrationVersionKey];
+    [self updateStatus:kAutoupdateAvailable version:version];
+  } else {
+    // If no updates are available, check what's on disk, because an update
+    // may have already been installed.  This check happens on another thread,
+    // and -updateStatus:version: will be called on the main thread when done.
+    [self determineUpdateStatusAsync];
+  }
 }
 
-- (void)startUpdateComplete:(NSNotification *)notification {
-  NSDictionary *userInfo = [notification userInfo];
-  BOOL checkSuccessful = [[userInfo objectForKey:KSUpdateCheckSuccessfulKey]
-                           boolValue];
-  int installs = [[userInfo objectForKey:KSUpdateCheckSuccessfullyInstalledKey]
-                   intValue];
+- (void)installUpdate {
+  if (!registration_) {
+    [self updateStatus:kAutoupdateInstallFailed version:nil];
+    return;
+  }
 
-  [startTarget_ updateCompleted:checkSuccessful installs:installs];
-  [startTarget_ release];
-  startTarget_ = nil;
+  [registration_ startUpdate];
 
-  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
-  [center removeObserver:self
-                    name:KSRegistrationStartUpdateNotification
-                  object:nil];
+  // Upon completion, KSRegistrationStartUpdateNotification will be posted,
+  // and -installUpdateComplete: will be called.
 }
 
-- (BOOL)startUpdate:(NSObject<KeystoneGlueCallbacks>*)target {
-  if (registration_ == nil)
-    return NO;
-  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
-  [center addObserver:self
-             selector:@selector(startUpdateComplete:)
-                 name:KSRegistrationStartUpdateNotification
-               object:nil];
-  startTarget_ = [target retain];
-  [registration_ startUpdate];
-  return YES;
+- (void)installUpdateComplete:(NSNotification*)notification {
+  NSDictionary* userInfo = [notification userInfo];
+  BOOL checkSuccessful =
+      [[userInfo objectForKey:KSUpdateCheckSuccessfulKey] boolValue];
+  int installs =
+      [[userInfo objectForKey:KSUpdateCheckSuccessfullyInstalledKey] intValue];
+
+  if (!checkSuccessful || !installs) {
+    [self updateStatus:kAutoupdateInstallFailed version:nil];
+  } else {
+    updateSuccessfullyInstalled_ = YES;
+
+    // Nothing in the notification dictionary reports the version that was
+    // installed.  Figure it out based on what's on disk.
+    [self determineUpdateStatusAsync];
+  }
+}
+
+// Runs on the main thread.
+- (void)determineUpdateStatusAsync {
+  // NSBundle is not documented as being thread-safe.  Do NSBundle operations
+  // on the main thread before jumping over to a NSOperationQueue-managed
+  // thread to do blocking file input.
+  DCHECK([NSThread isMainThread]);
+
+  SEL selector = @selector(determineUpdateStatusAtPath:);
+  NSString* appPath = [[NSBundle mainBundle] bundlePath];
+  NSInvocationOperation* operation =
+      [[[NSInvocationOperation alloc] initWithTarget:self
+                                            selector:selector
+                                              object:appPath] autorelease];
+
+  NSOperationQueue* operationQueue = [WorkerPoolObjC sharedOperationQueue];
+  [operationQueue addOperation:operation];
+}
+
+// Runs on a thread managed by NSOperationQueue.
+- (void)determineUpdateStatusAtPath:(NSString*)appPath {
+  DCHECK(![NSThread isMainThread]);
+
+  NSString* appInfoPlistPath =
+      [[appPath stringByAppendingPathComponent:@"Contents"]
+          stringByAppendingPathComponent:@"Info.plist"];
+  NSDictionary* infoPlist =
+      [NSDictionary dictionaryWithContentsOfFile:appInfoPlistPath];
+  NSString* version = [infoPlist objectForKey:@"CFBundleShortVersionString"];
+
+  [self performSelectorOnMainThread:@selector(determineUpdateStatusForVersion:)
+                         withObject:version
+                      waitUntilDone:NO];
+}
+
+// Runs on the main thread.
+- (void)determineUpdateStatusForVersion:(NSString*)version {
+  DCHECK([NSThread isMainThread]);
+
+  AutoupdateStatus status;
+  if (updateSuccessfullyInstalled_) {
+    // If an update was successfully installed and this object saw it happen,
+    // then don't even bother comparing versions.
+    status = kAutoupdateInstalled;
+  } else {
+    NSString* currentVersion =
+        [NSString stringWithUTF8String:chrome::kChromeVersion];
+    if (!version) {
+      // If the version on disk could not be determined, assume that
+      // whatever's running is current.
+      version = currentVersion;
+      status = kAutoupdateCurrent;
+    } else if ([version isEqualToString:currentVersion]) {
+      status = kAutoupdateCurrent;
+    } else {
+      // If the version on disk doesn't match what's currently running, an
+      // update must have been applied in the background, without this app's
+      // direct participation.  Leave updateSuccessfullyInstalled_ alone
+      // because there's no direct knowledge of what actually happened.
+      status = kAutoupdateInstalled;
+    }
+  }
+
+  [self updateStatus:status version:version];
+}
+
+- (void)updateStatus:(AutoupdateStatus)status version:(NSString*)version {
+  NSNumber* statusNumber = [NSNumber numberWithInt:status];
+  NSMutableDictionary* dictionary =
+      [NSMutableDictionary dictionaryWithObject:statusNumber
+                                         forKey:kAutoupdateStatusStatus];
+  if (version) {
+    [dictionary setObject:version forKey:kAutoupdateStatusVersion];
+  }
+
+  NSNotification* notification =
+      [NSNotification notificationWithName:kAutoupdateStatusNotification
+                                    object:self
+                                  userInfo:dictionary];
+  recentNotification_.reset([notification retain]);
+
+  [[NSNotificationCenter defaultCenter] postNotification:notification];
+}
+
+- (NSNotification*)recentNotification {
+  return [[recentNotification_ retain] autorelease];
+}
+
+- (void)clearRecentNotification {
+  recentNotification_.reset(nil);
 }
 
-@end
+@end  // @implementation KeystoneGlue
diff --git a/chrome/app/keystone_glue_unittest.mm b/chrome/app/keystone_glue_unittest.mm
index 614bdbc..90a2f40 100644
--- a/chrome/app/keystone_glue_unittest.mm
+++ b/chrome/app/keystone_glue_unittest.mm
@@ -14,18 +14,38 @@
 
 
 @implementation FakeGlueRegistration
-- (void)checkForUpdate { }
-- (void)startUpdate { }
+
+// Send the notifications that a real KeystoneGlue object would send.
+
+- (void)checkForUpdate {
+  NSNumber* yesNumber = [NSNumber numberWithBool:YES];
+  NSString* statusKey = @"Status";
+  NSDictionary* dictionary = [NSDictionary dictionaryWithObject:yesNumber
+                                                         forKey:statusKey];
+  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
+  [center postNotificationName:@"KSRegistrationCheckForUpdateNotification"
+                        object:nil
+                      userInfo:dictionary];
+}
+
+- (void)startUpdate {
+  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
+  [center postNotificationName:@"KSRegistrationStartUpdateNotification"
+                        object:nil];
+}
+
 @end
 
 
-@interface FakeKeystoneGlue : KeystoneGlue<KeystoneGlueCallbacks> {
+@interface FakeKeystoneGlue : KeystoneGlue {
  @public
   BOOL upToDate_;
   NSString *latestVersion_;
   BOOL successful_;
   int installs_;
 }
+
+- (void)fakeAboutWindowCallback:(NSNotification*)notification;
 @end
 
 
@@ -38,10 +58,23 @@
     latestVersion_ = @"foo bar";
     successful_ = YES;
     installs_ = 1010101010;
+
+    // Set up an observer that takes the notification that the About window
+    // listens for.
+    NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
+    [center addObserver:self
+               selector:@selector(fakeAboutWindowCallback:)
+                   name:kAutoupdateStatusNotification
+                 object:nil];
   }
   return self;
 }
 
+- (void)dealloc {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+  [super dealloc];
+}
+
 // For mocking
 - (NSDictionary*)infoDictionary {
   NSDictionary* dict = [NSDictionary dictionaryWithObjectsAndKeys:
@@ -69,21 +102,24 @@
   return timer_ ? YES : NO;
 }
 
-- (void)upToDateCheckCompleted:(BOOL)upToDate
-                 latestVersion:(NSString*)latestVersion {
-  upToDate_ = upToDate;
-  latestVersion_ = latestVersion;
-}
-
-- (void)updateCompleted:(BOOL)successful installs:(int)installs {
-  successful_ = successful;
-  installs_ = installs;
-}
-
 - (void)addFakeRegistration {
   registration_ = [[FakeGlueRegistration alloc] init];
 }
 
+- (void)fakeAboutWindowCallback:(NSNotification*)notification {
+  NSDictionary* dictionary = [notification userInfo];
+  AutoupdateStatus status = static_cast<AutoupdateStatus>(
+      [[dictionary objectForKey:kAutoupdateStatusStatus] intValue]);
+
+  if (status == kAutoupdateAvailable) {
+    upToDate_ = NO;
+    latestVersion_ = [dictionary objectForKey:kAutoupdateStatusVersion];
+  } else if (status == kAutoupdateInstallFailed) {
+    successful_ = NO;
+    installs_ = 0;
+  }
+}
+
 // Confirm we look like callbacks with nil NSNotifications
 - (BOOL)confirmCallbacks {
   return (!upToDate_ &&
@@ -114,12 +150,16 @@ TEST_F(KeystoneGlueTest, BasicGlobalCreate) {
   Method loadMethod_ = class_getInstanceMethod([KeystoneGlue class], lks);
   method_setImplementation(loadMethod_, newLoadImp_);
 
+  // Dump any existing KeystoneGlue shared instance so that a new one can be
+  // created with the mocked methods.
+  [KeystoneGlue releaseDefaultKeystoneGlue];
   KeystoneGlue *glue = [KeystoneGlue defaultKeystoneGlue];
   ASSERT_TRUE(glue);
 
   // Fix back up the class to the way we found it.
   method_setImplementation(infoMethod_, oldInfoImp_);
   method_setImplementation(loadMethod_, oldLoadImp_);
+  [KeystoneGlue releaseDefaultKeystoneGlue];
 }
 
 TEST_F(KeystoneGlueTest, BasicUse) {
@@ -136,14 +176,10 @@ TEST_F(KeystoneGlueTest, BasicUse) {
   ASSERT_TRUE([glue hasATimer]);
   [glue stopTimer];
 
-  ASSERT_TRUE(![glue checkForUpdate:glue] && ![glue startUpdate:glue]);
-
   // Brief exercise of callbacks
   [glue addFakeRegistration];
-  ASSERT_TRUE([glue checkForUpdate:glue]);
-  [glue checkComplete:nil];
-  ASSERT_TRUE([glue startUpdate:glue]);
-  [glue startUpdateComplete:nil];
+  [glue checkForUpdate];
+  [glue installUpdate];
   ASSERT_TRUE([glue confirmCallbacks]);
 }