Fix install-from-dmg.

When installing from a disk image, the browser relaunched from the
newly-installed location should follow the proper first-run experience if
appropriate.

When installing from a disk image, existing dock tiles should be reused, and
dock tiles pointing at the application on the disk image should be replaced
with ones pointing at the newly-installed location.

When installing from a disk image and requesting administrator access (because
the user does not have permission to install to /Applications), Keystone
should be promoted to a system-level install, and Chrome's Keystone ticket
should be promoted to a system Keystone ticket.

The custom panel key window tracking stuff doesn't need to maintain strong
references to key windows, but does need to be beefed up for thread safety.

Chrome doesn't need to wait until it's mostly started up to run the
install-from-dmg flow. It should do it early, avoiding delays and reducing the
possibility that harmful services will start running.

BUG=119325
TEST=obvious from the above and the bug

Review URL: http://codereview.chromium.org/9814029

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

6 files changed, 86 insertions, 44 deletions

diff --git a/chrome/browser/chrome_browser_application_mac.h b/chrome/browser/chrome_browser_application_mac.h
index 52c6be3..0801fbb 100644
--- a/chrome/browser/chrome_browser_application_mac.h
+++ b/chrome/browser/chrome_browser_application_mac.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -10,9 +10,12 @@
 
 #import <AppKit/AppKit.h>
 
+#include <vector>
+
 #import "base/mac/scoped_sending_event.h"
 #import "base/memory/scoped_nsobject.h"
 #import "base/message_pump_mac.h"
+#include "base/synchronization/lock.h"
 
 // Event hooks must implement this protocol.
 @protocol CrApplicationEventHookProtocol
@@ -29,8 +32,12 @@
   scoped_nsobject<NSMutableArray> eventHooks_;
 
   // App's previous key windows. Most recent key window is last.
-  // Does not include current key window.
-  scoped_nsobject<NSMutableArray> previousKeyWindows_;
+  // Does not include current key window. Elements of this vector are weak
+  // references.
+  std::vector<NSWindow*> previousKeyWindows_;
+
+  // Guards previousKeyWindows_.
+  base::Lock previousKeyWindowsLock_;
 }
 
 // Our implementation of |-terminate:| only attempts to terminate the
@@ -52,7 +59,7 @@
 // Keep track of the previous key windows and whether windows are being
 // cycled for use in determining whether a Panel window can become the
 // key window.
-- (id)previousKeyWindow;
+- (NSWindow*)previousKeyWindow;
 - (BOOL)isCyclingWindows;
 @end
 
diff --git a/chrome/browser/chrome_browser_application_mac.mm b/chrome/browser/chrome_browser_application_mac.mm
index 76c1660..765cec2 100644
--- a/chrome/browser/chrome_browser_application_mac.mm
+++ b/chrome/browser/chrome_browser_application_mac.mm
@@ -211,8 +211,15 @@ void SwizzleInit() {
 // Used to determine when a Panel window can become the key window.
 @interface NSApplication (PanelsCanBecomeKey)
 - (void)_cycleWindowsReversed:(BOOL)arg1;
-- (id)_removeWindow:(id)window;
-- (id)_setKeyWindow:(id)window;
+- (id)_removeWindow:(NSWindow*)window;
+- (id)_setKeyWindow:(NSWindow*)window;
+@end
+
+@interface BrowserCrApplication (PrivateInternal)
+
+// This must be called under the protection of previousKeyWindowsLock_.
+- (void)removePreviousKeyWindow:(NSWindow*)window;
+
 @end
 
 @implementation BrowserCrApplication
@@ -227,7 +234,6 @@ void SwizzleInit() {
   SwizzleInit();
   if ((self = [super init])) {
     eventHooks_.reset([[NSMutableArray alloc] init]);
-    previousKeyWindows_.reset([[NSMutableArray alloc] init]);
   }
 
   // Sanity check to alert if overridden methods are not supported.
@@ -516,8 +522,11 @@ void SwizzleInit() {
   return cyclingWindows_;
 }
 
-- (id)_removeWindow:(id)window {
-  [previousKeyWindows_ removeObject:window];
+- (id)_removeWindow:(NSWindow*)window {
+  {
+    base::AutoLock lock(previousKeyWindowsLock_);
+    [self removePreviousKeyWindow:window];
+  }
   id result = [super _removeWindow:window];
 
   // Ensure app has a key window after a window is removed.
@@ -537,21 +546,35 @@ void SwizzleInit() {
   return result;
 }
 
-- (id)_setKeyWindow:(id)window {
+- (id)_setKeyWindow:(NSWindow*)window {
   // |window| is nil when the current key window is being closed.
   // A separate call follows with a new value when a new key window is set.
   // Closed windows are not tracked in previousKeyWindows_.
   if (window != nil) {
-    [previousKeyWindows_ removeObject:window];
+    base::AutoLock lock(previousKeyWindowsLock_);
+    [self removePreviousKeyWindow:window];
     NSWindow* currentKeyWindow = [self keyWindow];
     if (currentKeyWindow != nil && currentKeyWindow != window)
-      [previousKeyWindows_ addObject:currentKeyWindow];
+      previousKeyWindows_.push_back(window);
   }
 
   return [super _setKeyWindow:window];
 }
 
-- (id)previousKeyWindow {
-  return [previousKeyWindows_ lastObject];
+- (NSWindow*)previousKeyWindow {
+  base::AutoLock lock(previousKeyWindowsLock_);
+  return previousKeyWindows_.empty() ? nil : previousKeyWindows_.back();
 }
+
+- (void)removePreviousKeyWindow:(NSWindow*)window {
+  previousKeyWindowsLock_.AssertAcquired();
+  std::vector<NSWindow*>::iterator window_iterator =
+      std::find(previousKeyWindows_.begin(),
+                previousKeyWindows_.end(),
+                window);
+  if (window_iterator != previousKeyWindows_.end()) {
+    previousKeyWindows_.erase(window_iterator);
+  }
+}
+
 @end
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index 70b629a..78b62df 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -164,7 +164,6 @@
 #include <Security/Security.h>
 
 #include "base/mac/scoped_nsautorelease_pool.h"
-#include "chrome/browser/mac/install_from_dmg.h"
 #include "chrome/browser/mac/keystone_glue.h"
 #endif
 
@@ -1549,18 +1548,6 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   translate_manager_ = TranslateManager::GetInstance();
   DCHECK(translate_manager_ != NULL);
 
-#if defined(OS_MACOSX)
-  if (!parsed_command_line().HasSwitch(switches::kNoFirstRun)) {
-    // Disk image installation is sort of a first-run task, so it shares the
-    // kNoFirstRun switch.
-    if (MaybeInstallFromDiskImage()) {
-      // The application was installed and the installed copy has been
-      // launched.  This process is now obsolete.  Exit.
-      return content::RESULT_CODE_NORMAL_EXIT;
-    }
-  }
-#endif
-
   // TODO(stevenjb): Move WIN and MACOSX specific code to apprpriate Parts.
   // (requires supporting early exit).
   PostProfileInit();
diff --git a/chrome/browser/chrome_browser_main_mac.mm b/chrome/browser/chrome_browser_main_mac.mm
index 493f8ea..8264490 100644
--- a/chrome/browser/chrome_browser_main_mac.mm
+++ b/chrome/browser/chrome_browser_main_mac.mm
@@ -16,6 +16,7 @@
 #include "chrome/app/breakpad_mac.h"
 #import "chrome/browser/app_controller_mac.h"
 #import "chrome/browser/chrome_browser_application_mac.h"
+#include "chrome/browser/mac/install_from_dmg.h"
 #import "chrome/browser/mac/keystone_glue.h"
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/common/chrome_paths.h"
@@ -94,6 +95,30 @@ void ChromeBrowserMainPartsMac::PreMainMessageLoopStart() {
     ResourceBundle::AddDataPackToSharedInstance(resources_pack_path);
   }
 
+  // This is a no-op if the KeystoneRegistration framework is not present.
+  // The framework is only distributed with branded Google Chrome builds.
+  [[KeystoneGlue defaultKeystoneGlue] registerWithKeystone];
+
+  // Disk image installation is sort of a first-run task, so it shares the
+  // kNoFirstRun switch.
+  //
+  // This needs to be done after the resource bundle is initialized (for
+  // access to localizations in the UI) and after Keystone is initialized
+  // (because the installation may need to promote Keystone) but before the
+  // app controller is set up (and thus before MainMenu.nib is loaded, because
+  // the app controller assumes that a browser has been set up and will crash
+  // upon receipt of certain notifications if no browser exists), before
+  // anyone tries doing anything silly like firing off an import job, and
+  // before anything creating preferences like Local State in order for the
+  // relaunched installed application to still consider itself as first-run.
+  if (!parsed_command_line().HasSwitch(switches::kNoFirstRun)) {
+    if (MaybeInstallFromDiskImage()) {
+      // The application was installed and the installed copy has been
+      // launched.  This process is now obsolete.  Exit.
+      exit(0);
+    }
+  }
+
   // Now load the nib (from the right bundle).
   scoped_nsobject<NSNib>
       nib([[NSNib alloc] initWithNibNamed:@"MainMenu"
@@ -104,10 +129,6 @@ void ChromeBrowserMainPartsMac::PreMainMessageLoopStart() {
   // Make sure the app controller has been created.
   DCHECK([NSApp delegate]);
 
-  // This is a no-op if the KeystoneRegistration framework is not present.
-  // The framework is only distributed with branded Google Chrome builds.
-  [[KeystoneGlue defaultKeystoneGlue] registerWithKeystone];
-
   // Prevent Cocoa from turning command-line arguments into
   // |-application:openFiles:|, since we already handle them directly.
   [[NSUserDefaults standardUserDefaults]
diff --git a/chrome/browser/mac/dock.mm b/chrome/browser/mac/dock.mm
index 0c52150..39c33b9 100644
--- a/chrome/browser/mac/dock.mm
+++ b/chrome/browser/mac/dock.mm
@@ -243,24 +243,15 @@ void AddIcon(NSString* installed_path, NSString* dmg_app_path) {
     return;
   }
 
-  // Directories in the Dock's plist are given with trailing slashes. Since
-  // installed_path and dmg_app_path both refer to application bundles,
-  // they're directories and will show up with trailing slashes. This is an
-  // artifact of the Dock's internal use of CFURL. Look for paths that match,
-  // and when adding an item to the Dock's plist, keep it in the form that the
-  // Dock likes.
-  NSString* installed_path_dock = [installed_path stringByAppendingString:@"/"];
-  NSString* dmg_app_path_dock = [dmg_app_path stringByAppendingString:@"/"];
-
   NSUInteger already_installed_app_index = NSNotFound;
   NSUInteger app_index = NSNotFound;
   for (NSUInteger index = 0; index < [persistent_apps count]; ++index) {
     NSString* app_path = [persistent_app_paths objectAtIndex:index];
-    if ([app_path isEqualToString:installed_path_dock]) {
+    if ([app_path isEqualToString:installed_path]) {
       // If the Dock already contains a reference to the newly installed
       // application, don't add another one.
       already_installed_app_index = index;
-    } else if ([app_path isEqualToString:dmg_app_path_dock]) {
+    } else if ([app_path isEqualToString:dmg_app_path]) {
       // If the Dock contains a reference to the application on the disk
       // image, replace it with a reference to the newly installed
       // application. However, if the Dock contains a reference to both the
@@ -364,7 +355,7 @@ void AddIcon(NSString* installed_path, NSString* dmg_app_path) {
     }
 
     // Set up the new Dock tile.
-    NSURL* url = [NSURL fileURLWithPath:installed_path_dock];
+    NSURL* url = [NSURL fileURLWithPath:installed_path isDirectory:YES];
     NSDictionary* url_dict = NSURLCopyDictionary(url);
     if (!url_dict) {
       LOG(ERROR) << "couldn't create url_dict";
@@ -380,7 +371,7 @@ void AddIcon(NSString* installed_path, NSString* dmg_app_path) {
 
     // Add the new tile to the Dock.
     [persistent_apps insertObject:new_tile atIndex:app_index];
-    [persistent_app_paths insertObject:installed_path_dock atIndex:app_index];
+    [persistent_app_paths insertObject:installed_path atIndex:app_index];
     made_change = true;
   }
 
diff --git a/chrome/browser/mac/keystone_glue.mm b/chrome/browser/mac/keystone_glue.mm
index 8b00373..4236206 100644
--- a/chrome/browser/mac/keystone_glue.mm
+++ b/chrome/browser/mac/keystone_glue.mm
@@ -892,6 +892,12 @@ NSString* const kVersionKey = @"KSVersion";
 
   // Upon completion, ksr::KSRegistrationPromotionDidCompleteNotification will
   // be posted, and -promotionComplete: will be called.
+
+  // If synchronous, see to it that this happens immediately. Give it a
+  // 10-second deadline.
+  if (synchronous) {
+    CFRunLoopRunInMode(kCFRunLoopDefaultMode, 10, false);
+  }
 }
 
 - (void)promotionComplete:(NSNotification*)notification {
@@ -910,6 +916,13 @@ NSString* const kVersionKey = @"KSVersion";
     authorization_.reset();
     [self updateStatus:kAutoupdatePromoteFailed version:nil];
   }
+
+  if (synchronousPromotion_) {
+    // The run loop doesn't need to wait for this any longer.
+    CFRunLoopRef runLoop = CFRunLoopGetCurrent();
+    CFRunLoopStop(runLoop);
+    CFRunLoopWakeUp(runLoop);
+  }
 }
 
 - (void)changePermissionsForPromotionAsync {