Prompt user before restarting System Preferences.

remoting/host/me2me_preference_pane.mm

 // 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.
 
 #import "remoting/host/me2me_preference_pane.h"
 
 #import <Cocoa/Cocoa.h>
 #include <CommonCrypto/CommonHMAC.h>
 #include <launch.h>
 #import <PreferencePanes/PreferencePanes.h>
@@ skipping 165 matching lines @@
                                         target:self
                                       selector:@selector(refreshServiceStatus:)
                                       userInfo:nil
                                        repeats:YES] retain];
   [self updateServiceStatus];
   [self updateAuthorizationStatus];
   [self readNewConfig];
   [self updateUI];
 }
 
+- (void)didSelect {
+  [self checkInstalledVersion];
+}
+
 - (void)willUnselect {
   NSDistributedNotificationCenter* center =
       [NSDistributedNotificationCenter defaultCenter];
   [center removeObserver:self];
 
   [service_status_timer_ invalidate];
   [service_status_timer_ release];
   service_status_timer_ = nil;
   if (have_new_config_) {
     [self notifyPlugin:kUpdateFailedNotificationName];
@@ skipping 78 matching lines @@
 - (void)updateServiceStatus {
   pid_t job_pid = base::mac::PIDForJob(kServiceName);
   is_service_running_ = (job_pid > 0);
 }
 
 - (void)updateAuthorizationStatus {
   is_pane_unlocked_ = [authorization_view_ updateStatus:authorization_view_];
 }
 
 - (void)readNewConfig {
-  if ([self restartPanelIfDifferentVersionInstalled]) {
-    // Don't read any new config if the version is mismatched.  Return control
-    // to the main run-loop instead, so the application can be terminated.
+  [self checkInstalledVersion];
+  if (is_pane_disabled_) {
+    // Don't read or delete any new config here if the version is mismatched.
+    // When the pane restarts, it should see the new config file.
     return;
   }

[Jamie, 2012/06/06 16:26:49]

As my previous remark states, I don't think either of these checks is correct
with the function named the way it is.

[Lambros, 2012/06/07 00:25:28]

Done.

 
   std::string file;
   if (!GetTemporaryConfigFilePath(&file)) {
     LOG(ERROR) << "Failed to get path of configuration data.";
     [self showError];
     return;
   }
   if (access(file.c_str(), F_OK) != 0)
     return;
 
@@ skipping 45 matching lines @@
   [status_message_ setStringValue:message];
 
   std::string email;
   if (config_.get()) {
     bool result = config_->GetString(remoting::kXmppLoginConfigPath, &email);
 
     // The config has already been checked by |IsConfigValid|.
     DCHECK(result);
   }
 
-  [disable_view_ setEnabled:(is_pane_unlocked_ && is_service_running_)];
-  [confirm_pin_view_ setEnabled:is_pane_unlocked_];
+  [disable_view_ setEnabled:(is_pane_unlocked_ && is_service_running_ &&
+                             !is_pane_disabled_)];
+  [confirm_pin_view_ setEnabled:(is_pane_unlocked_ && !is_pane_disabled_)];
   [confirm_pin_view_ setEmail:base::SysUTF8ToNSString(email)];
   NSString* applyButtonText = is_service_running_ ? @"Confirm" : @"Enable";
   [confirm_pin_view_ setButtonText:applyButtonText];
+
+  if (is_pane_disabled_) {
+    [authorization_view_ setEnabled:NO];
+  }
 }
 
 - (void)showError {
   NSAlert* alert = [[NSAlert alloc] init];
   [alert setMessageText:@"An unexpected error occurred."];
   [alert setInformativeText:@"Check the system log for more information."];
   [alert setAlertStyle:NSWarningAlertStyle];
   [alert beginSheetModalForWindow:[[self mainView] window]
                     modalDelegate:nil
                    didEndSelector:nil
@@ skipping 153 matching lines @@
 
 - (void)notifyPlugin:(const char*)message {
   NSDistributedNotificationCenter* center =
       [NSDistributedNotificationCenter defaultCenter];
   NSString* name = [NSString stringWithUTF8String:message];
   [center postNotificationName:name
                         object:nil
                       userInfo:nil];
 }
 
-- (BOOL)restartPanelIfDifferentVersionInstalled {
-  NSBundle* this_bundle = [NSBundle bundleForClass:[self class]];
-  NSDictionary* this_plist = [this_bundle infoDictionary];
-  NSString* this_version = [this_plist objectForKey:@"CFBundleVersion"];
+- (void)checkInstalledVersion {
+  // There's no point repeating the check if the pane has already been disabled
+  // from a previous call to this method.  The pane only gets disabled when a
+  // version-mismatch has been detected here, so skip the check, but continue to
+  // handle the version-mismatch case.
+  if (!is_pane_disabled_) {
+    NSBundle* this_bundle = [NSBundle bundleForClass:[self class]];
+    NSDictionary* this_plist = [this_bundle infoDictionary];
+    NSString* this_version = [this_plist objectForKey:@"CFBundleVersion"];
 
-  NSString* bundle_path = [this_bundle bundlePath];
-  NSString* plist_path =
-      [bundle_path stringByAppendingString:@"/Contents/Info.plist"];
-  NSDictionary* disk_plist =
-      [NSDictionary dictionaryWithContentsOfFile:plist_path];
-  NSString* disk_version = [disk_plist objectForKey:@"CFBundleVersion"];
+    NSString* bundle_path = [this_bundle bundlePath];
+    NSString* plist_path =
+        [bundle_path stringByAppendingString:@"/Contents/Info.plist"];
+    NSDictionary* disk_plist =
+        [NSDictionary dictionaryWithContentsOfFile:plist_path];
+    NSString* disk_version = [disk_plist objectForKey:@"CFBundleVersion"];
 
-  if (disk_version == nil) {
-    LOG(ERROR) << "Failed to get installed version information";
-    [self showError];
-    return NO;
+    if (disk_version == nil) {
+      LOG(ERROR) << "Failed to get installed version information";
+      [self showError];
+      return;
+    }
+
+    if ([this_version isEqualToString:disk_version]) {
+      return;
+    }
+
+    is_pane_disabled_ = YES;
+    [self updateUI];
   }
 
-  if ([this_version isEqualToString:disk_version]) {
-    return NO;
+  NSWindow* window = [[self mainView] window];
+  if (window == nil) {
+    // Defer the alert until |didSelect| is called, which happens just after
+    // the window is created.
+    return;
+  }
+
+  // This alert appears as a sheet over the top of the Chromoting pref-pane,
+  // underneath the title, so it's OK to refer to "this preference pane" rather
+  // than repeat the title "Chromoting" here.
+  NSAlert* alert = [[NSAlert alloc] init];
+  [alert setMessageText:@"System update detected"];
+  [alert setInformativeText:@"To use this preference pane, System Preferences "
+      "needs to be restarted"];
+  [alert addButtonWithTitle:@"OK"];
+  NSButton* cancel_button = [alert addButtonWithTitle:@"Cancel"];
+  [cancel_button setKeyEquivalent:@"\e"];
+  [alert setAlertStyle:NSWarningAlertStyle];
+  [alert beginSheetModalForWindow:window
+                    modalDelegate:self
+                   didEndSelector:@selector(
+                       mismatchAlertDidEnd:returnCode:contextInfo:)
+                      contextInfo:nil];
+  [alert release];
+}
+
+- (void)mismatchAlertDidEnd:(NSAlert*)alert
+                 returnCode:(NSInteger)returnCode
+                contextInfo:(void*)contextInfo {
+  if (returnCode == NSAlertFirstButtonReturn) {
+    // OK was pressed.
+
+    // Dismiss the alert window here, so that the application will respond to
+    // the NSApp terminate: message.
+    [[alert window] orderOut:nil];
+    [self restartSystemPreferences];
   } else {
-    // Terminate and relaunch System Preferences.
+    // Cancel was pressed.
 
-    // TODO(lambroslambrou): Improve this when System Preferences supports
-    // dynamic reloading of pref-panes.
-    NSTask* task = [[NSTask alloc] init];
-    NSArray* arguments = [NSArray arrayWithObjects:@"--relaunch-prefpane", nil];
-    [task setLaunchPath:[NSString stringWithUTF8String:kHelperTool]];
-    [task setArguments:arguments];
-    [task setStandardInput:[NSPipe pipe]];
-    [task launch];
-    [task release];
-    [NSApp terminate:nil];
-    return YES;
+    // If there is a new config file, delete it and notify the web-app of
+    // failure to apply the config.  Otherwise, the web-app will remain in a
+    // spinning state until System Preferences eventually gets restarted and
+    // the user visits this pane again.
+    std::string file;
+    if (!GetTemporaryConfigFilePath(&file)) {
+      // There's no point in alerting the user here.  The same error would
+      // happen when the pane is eventually restarted, so the user would be
+      // alerted at that time.
+      LOG(ERROR) << "Failed to get path of configuration data.";
+      return;
+    }
+    if (access(file.c_str(), F_OK) != 0)
+      return;
+
+    remove(file.c_str());
+    [self notifyPlugin:kUpdateFailedNotificationName];
   }
 }
 
+- (void)restartSystemPreferences {
+  NSTask* task = [[NSTask alloc] init];
+  NSArray* arguments = [NSArray arrayWithObjects:@"--relaunch-prefpane", nil];
+  [task setLaunchPath:[NSString stringWithUTF8String:kHelperTool]];
+  [task setArguments:arguments];
+  [task setStandardInput:[NSPipe pipe]];
+  [task launch];
+  [task release];
+  [NSApp terminate:nil];
+}
+
 @end