General comment cleaning / refactoring for Mac Installer

chrome/installer/mac/app/AppDelegate.mm

Index: chrome/installer/mac/app/AppDelegate.mm
diff --git a/chrome/installer/mac/app/AppDelegate.mm b/chrome/installer/mac/app/AppDelegate.mm
index 1f6e4e990b274785461c0daf78923dc30b2d69dd..8d4c639926c59fc87ca135b162cf7d09c965c5aa 100644
--- a/chrome/installer/mac/app/AppDelegate.mm
+++ b/chrome/installer/mac/app/AppDelegate.mm
@@ -39,11 +39,17 @@
 
 // Sets up the main window and begins the downloading process.
 - (void)applicationDidFinishLaunching:(NSNotification*)aNotification {
-  // TODO: fix UI not loading until after asking for authorization.
+  // TODO: Despite what the code implies -- when the installer is run, the main
+  // window of the application is not visible until the user has taken action on
+  // the Authorization modal.
   window_.delegate = self;
   installerWindowController_ =
       [[InstallerWindowController alloc] initWithWindow:window_];
   authorizedInstall_ = [[AuthorizedInstall alloc] init];
+  // TODO: Since the tasks taken care of by the AuthorizedInstall tool are
+  // relatively simple if we don't need to worry about privilege escalation, we
+  // can easily create a pure-Objective-C class that takes care of
+  // AuthorizedInstall's job when the class doesn't load successfully.
   if ([authorizedInstall_ loadInstallationTool]) {
     [self startDownload];
   } else {
@@ -56,14 +62,20 @@
 
 - (NSApplicationTerminateReply)applicationShouldTerminate:
     (NSApplication*)sender {
-  return preventTermination_ ? NSTerminateCancel : NSTerminateNow;

[Elly Fong-Jones, 2016/08/31 14:25:55]

you can use a ternary for this :)

[Anna Zeng, 2016/08/31 14:31:17]

Done.

+  if (preventTermination_) {
+    return NSTerminateCancel;
+  } else {
+    return NSTerminateNow;
+  }
 }
 
 // This function effectively takes the place of
 // applicationShouldTerminateAfterLastWindowClosed: to make sure that the
 // application does correctly terminate after closing the installer, but does
 // not terminate when we call orderOut: to hide the installer during its
-// tear-down steps.
+// tear-down steps. If the user force-quits the application, the below delegate
+// method gets called. However, when orderOut is called, the below delegate
+// method does not get called.
 - (BOOL)windowShouldClose:(id)sender {
   [self exit];
   return YES;
@@ -84,7 +96,7 @@
 
 - (void)onLoadInstallationToolFailure {
   NSError* loadToolError = [NSError
-       errorForAlerts:@"Could not load installion tool"
+       errorForAlerts:@"Internal Error"
       withDescription:
           @"Your Chrome Installer may be corrupted. Download and try again."
         isRecoverable:NO];
@@ -135,14 +147,15 @@
 - (void)unpacker:(Unpacker*)unpacker onMountSuccess:(NSString*)tempAppPath {
   SecStaticCodeRef diskStaticCode;
   SecRequirementRef diskRequirement;
-  // TODO: flush out error handling more
+  // TODO: Include some better error handling below than NSLog
   OSStatus oserror;
   oserror = SecStaticCodeCreateWithPath(
       (__bridge CFURLRef)[NSURL fileURLWithPath:tempAppPath isDirectory:NO],
       kSecCSDefaultFlags, &diskStaticCode);
   if (oserror != errSecSuccess)
     NSLog(@"code %d", oserror);
-  // TODO: add in a more specific code sign requirement
+  // TODO: The below requirement is too general as most signed entities have the
+  // below requirement; replace it with something adequately specific.
   oserror =
       SecRequirementCreateWithString((CFStringRef) @"anchor apple generic",
                                      kSecCSDefaultFlags, &diskRequirement);
@@ -171,6 +184,11 @@
   if ([installerWindowController_ isDefaultBrowserChecked])
     [installerSettings
         addObject:[NSString
+                      // NOTE: the |kMakeDefaultBrowser| constant used as a
+                      // command-line switch here only will apply at a user
+                      // level, since the application itself is not running with
+                      // privileges. grt@ suggested this constant should be
+                      // renamed |kMakeDefaultBrowserforUser|.
                       stringWithUTF8String:switches::kMakeDefaultBrowser]];
 
   NSError* error = nil;
@@ -186,7 +204,7 @@
     NSLog(@"Chrome failed to launch: %@", error);
   }
 
-  // Begin teardown stuff!
+  // Begin teardown step!
   dispatch_async(dispatch_get_main_queue(), ^{
     [window_ orderOut:nil];
   });
@@ -196,7 +214,7 @@
 
 - (void)unpacker:(Unpacker*)unpacker onMountFailure:(NSError*)error {
   NSError* extractError =
-      [NSError errorForAlerts:@"Install Failure"
+      [NSError errorForAlerts:@"Install Error"
               withDescription:@"Unable to add Google Chrome to Applications."
                 isRecoverable:NO];
   [self displayError:extractError];
@@ -209,9 +227,9 @@
 
 - (void)unpacker:(Unpacker*)unpacker onUnmountFailure:(NSError*)error {
   NSLog(@"error unmounting");
-  // NOTE: since we are not deleting the temporary folder if the unmount fails,
+  // NOTE: Since we are not deleting the temporary folder if the unmount fails,
   // we'll just leave it up to the computer to delete the temporary folder on
-  // its own time, and to unmount the disk during a restart at some point. There
+  // its own time and to unmount the disk during a restart at some point. There
   // is no other work to be done in the mean time.
   [self exit];
 }