From 5bb15df2e87609f8f19e75524af32afbd4f7ff13 Mon Sep 17 00:00:00 2001 From: "nduca@google.com" Date: Sat, 24 Mar 2012 09:27:28 +0000 Subject: Speculative Revert 128677 - 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 TBR=mark@chromium.org Review URL: https://chromiumcodereview.appspot.com/9836082 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128724 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/chrome_browser_application_mac.h | 15 +++------ chrome/browser/chrome_browser_application_mac.mm | 43 ++++++------------------ chrome/browser/chrome_browser_main.cc | 13 +++++++ chrome/browser/chrome_browser_main_mac.mm | 29 +++------------- chrome/browser/mac/dock.mm | 17 +++++++--- chrome/browser/mac/keystone_glue.mm | 13 ------- 6 files changed, 44 insertions(+), 86 deletions(-) diff --git a/chrome/browser/chrome_browser_application_mac.h b/chrome/browser/chrome_browser_application_mac.h index 0801fbb..52c6be3 100644 --- a/chrome/browser/chrome_browser_application_mac.h +++ b/chrome/browser/chrome_browser_application_mac.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// 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. @@ -10,12 +10,9 @@ #import -#include - #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 @@ -32,12 +29,8 @@ scoped_nsobject eventHooks_; // App's previous key windows. Most recent key window is last. - // Does not include current key window. Elements of this vector are weak - // references. - std::vector previousKeyWindows_; - - // Guards previousKeyWindows_. - base::Lock previousKeyWindowsLock_; + // Does not include current key window. + scoped_nsobject previousKeyWindows_; } // Our implementation of |-terminate:| only attempts to terminate the @@ -59,7 +52,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. -- (NSWindow*)previousKeyWindow; +- (id)previousKeyWindow; - (BOOL)isCyclingWindows; @end diff --git a/chrome/browser/chrome_browser_application_mac.mm b/chrome/browser/chrome_browser_application_mac.mm index 765cec2..76c1660 100644 --- a/chrome/browser/chrome_browser_application_mac.mm +++ b/chrome/browser/chrome_browser_application_mac.mm @@ -211,15 +211,8 @@ void SwizzleInit() { // Used to determine when a Panel window can become the key window. @interface NSApplication (PanelsCanBecomeKey) - (void)_cycleWindowsReversed:(BOOL)arg1; -- (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; - +- (id)_removeWindow:(id)window; +- (id)_setKeyWindow:(id)window; @end @implementation BrowserCrApplication @@ -234,6 +227,7 @@ 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. @@ -522,11 +516,8 @@ void SwizzleInit() { return cyclingWindows_; } -- (id)_removeWindow:(NSWindow*)window { - { - base::AutoLock lock(previousKeyWindowsLock_); - [self removePreviousKeyWindow:window]; - } +- (id)_removeWindow:(id)window { + [previousKeyWindows_ removeObject:window]; id result = [super _removeWindow:window]; // Ensure app has a key window after a window is removed. @@ -546,35 +537,21 @@ void SwizzleInit() { return result; } -- (id)_setKeyWindow:(NSWindow*)window { +- (id)_setKeyWindow:(id)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) { - base::AutoLock lock(previousKeyWindowsLock_); - [self removePreviousKeyWindow:window]; + [previousKeyWindows_ removeObject:window]; NSWindow* currentKeyWindow = [self keyWindow]; if (currentKeyWindow != nil && currentKeyWindow != window) - previousKeyWindows_.push_back(window); + [previousKeyWindows_ addObject:currentKeyWindow]; } return [super _setKeyWindow:window]; } -- (NSWindow*)previousKeyWindow { - base::AutoLock lock(previousKeyWindowsLock_); - return previousKeyWindows_.empty() ? nil : previousKeyWindows_.back(); +- (id)previousKeyWindow { + return [previousKeyWindows_ lastObject]; } - -- (void)removePreviousKeyWindow:(NSWindow*)window { - previousKeyWindowsLock_.AssertAcquired(); - std::vector::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 78b62df..70b629a 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -164,6 +164,7 @@ #include #include "base/mac/scoped_nsautorelease_pool.h" +#include "chrome/browser/mac/install_from_dmg.h" #include "chrome/browser/mac/keystone_glue.h" #endif @@ -1548,6 +1549,18 @@ 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 8264490..493f8ea 100644 --- a/chrome/browser/chrome_browser_main_mac.mm +++ b/chrome/browser/chrome_browser_main_mac.mm @@ -16,7 +16,6 @@ #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" @@ -95,30 +94,6 @@ 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 nib([[NSNib alloc] initWithNibNamed:@"MainMenu" @@ -129,6 +104,10 @@ 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 39c33b9..0c52150 100644 --- a/chrome/browser/mac/dock.mm +++ b/chrome/browser/mac/dock.mm @@ -243,15 +243,24 @@ 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]) { + if ([app_path isEqualToString:installed_path_dock]) { // 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]) { + } else if ([app_path isEqualToString:dmg_app_path_dock]) { // 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 @@ -355,7 +364,7 @@ void AddIcon(NSString* installed_path, NSString* dmg_app_path) { } // Set up the new Dock tile. - NSURL* url = [NSURL fileURLWithPath:installed_path isDirectory:YES]; + NSURL* url = [NSURL fileURLWithPath:installed_path_dock]; NSDictionary* url_dict = NSURLCopyDictionary(url); if (!url_dict) { LOG(ERROR) << "couldn't create url_dict"; @@ -371,7 +380,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 atIndex:app_index]; + [persistent_app_paths insertObject:installed_path_dock atIndex:app_index]; made_change = true; } diff --git a/chrome/browser/mac/keystone_glue.mm b/chrome/browser/mac/keystone_glue.mm index 4236206..8b00373 100644 --- a/chrome/browser/mac/keystone_glue.mm +++ b/chrome/browser/mac/keystone_glue.mm @@ -892,12 +892,6 @@ 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 { @@ -916,13 +910,6 @@ 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 { -- cgit v1.1