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]);
 }