Added unpacking step

chrome/installer/mac/app/Unpacker.m

Index: chrome/installer/mac/app/Unpacker.m
diff --git a/chrome/installer/mac/app/Unpacker.m b/chrome/installer/mac/app/Unpacker.m
new file mode 100644
index 0000000000000000000000000000000000000000..d9d47d0dc9581489113a57c8063549258fd9fdf1
--- /dev/null
+++ b/chrome/installer/mac/app/Unpacker.m
@@ -0,0 +1,131 @@
+// Copyright 2016 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 "Unpacker.h"
+
+#include <DiskArbitration/DiskArbitration.h>
+
+#import "Downloader.h"
+
+// TODO: make paths not depend on location of executable
+// cocoa temporary folders: nstemporarydirectory
+#define PATH_FROM_EXECUTABLE(x)                                          \
+  [[[[NSBundle mainBundle] bundlePath] stringByAppendingPathComponent:x] \
+      stringByResolvingSymlinksInPath]

[Sidney San Martín, 2016/08/05 20:35:04]

This should be a pretty quick change, any reason not to do it now?

[Anna Zeng, 2016/08/05 21:48:40]

I plan to include progress on this in the other refactor CL, as that one
includes a new class dedicated to keeping track of where files of interest are.
Acknowledged!

+
+@implementation Unpacker
+
+@synthesize delegate = delegate_;
+
+DASessionRef session;
+
+// TODO: make testing more convenient
+
+- (void)unpackDMG {
+  // TODO: how to find this from the PATH env variable?
+  NSString* path = @"/usr/bin/hdiutil";
+  NSArray* args = @[
+    // TODO: break getDownloadsFilePath out of the downloader file
+    @"mount", [Downloader getDownloadsFilePath], @"-nobrowse", @"-mountpoint",
+    PATH_FROM_EXECUTABLE(@"tmp")
+  ];
+
+  NSTask* unmountTask = [[NSTask alloc] init];
+  unmountTask.launchPath = path;
+  unmountTask.arguments = args;
+  unmountTask.terminationHandler = ^void(NSTask* task) {

[Mark Mentovai, 2016/08/02 19:01:32]

Aha! This is what’s causing all of the trouble with the run loop stuff below.

The termination handler isn’t guaranteed to run on the same thread as
-unpackDMG. In fact, it probably doesn’t run on the same thread at all. These
block completion handlers run stuff on a libdispatch worker thread. That’s
mostly fine until you expect something to happen on your run loop. Each thread
has its own run loop, and the run loop you’re expecting things to run on. When
you schedule the DiskArbitration callback with a run loop, presently, you’re
scheduling it with the CURRENT thread’s run loop, but you’re EXPECTING it to be
scheduled with the MAIN thread’s run loop.

For that matter, -unpackDMG isn’t running on the main thread either. Must be
another worker thread!

Fortunately, it’s pretty easy to send a block over to the main thread. You can
write

dispatch_async(dispatch_get_main_queue(), ^{
    stuff_to_run_on_main_thread();
});

You can do this from within your termination handler. That should get stuff back
on track.

[Anna Zeng, 2016/08/02 23:58:22]

Awesome! This helped immensely. I ended up using a dispatch queue though, to
make sure that the main thread's UI isn't hanging while we do this work.

+    [self extractChrome];
+  };
+  [unmountTask launch];
+}
+
+- (void)extractChrome {
+  NSLog(@"extracting chrome");
+  NSFileManager* fileManager = [NSFileManager defaultManager];
+  if (![fileManager
+          fileExistsAtPath:PATH_FROM_EXECUTABLE(@"tmp/Google Chrome.app")]) {
+    NSLog(@"File in DMG doesn't exist");
+  }
+
+  // TODO: make async
+  NSError* err;
+  if (![fileManager
+          copyItemAtPath:PATH_FROM_EXECUTABLE(@"tmp/Google Chrome.app")
+                  toPath:@"/Applications/Google Chromo.app"
+                   error:&err]) {
+    NSLog(@"%@", err);
+  }
+
+  [self cleanUp];
+}
+- (void)cleanUp {
+  NSFileManager* fileManager = [NSFileManager defaultManager];
+  if ([fileManager removeItemAtPath:[Downloader getDownloadsFilePath]
+                              error:nil]) {
+    NSLog(@"Disk image removed!");
+  }
+
+  // TODO: run the app instead of removing it every time
+  if ([fileManager removeItemAtPath:@"/Applications/Google Chromo.app"
+                              error:nil]) {
+    NSLog(@"Application removed!");
+  }
+
+  session = DASessionCreate(nil);
+  DASessionScheduleWithRunLoop(session, CFRunLoopGetCurrent(),
+                               kCFRunLoopCommonModes);
+  // TODO: how can we make the run loop run using the nsrunloop?
+  // DASessionScheduleWithRunLoop(session, [[NSRunLoop currentRunLoop]
+  // getCFRunLoop],
+  //                              (CFStringRef)NSDefaultRunLoopMode);
+  DADiskRef child_disk = DADiskCreateFromVolumePath(
+      nil, session,
+      (CFURLRef)[NSURL URLWithString:PATH_FROM_EXECUTABLE(@"tmp")]);
+  DADiskRef whole_disk = DADiskCopyWholeDisk(child_disk);
+  DADiskUnmount(whole_disk, kDADiskUnmountOptionWhole, unmount_callback,
+                (void*)self);
+  NSLog(@"Releasing disk");
+  CFRelease(whole_disk);
+  CFRelease(child_disk);
+
+  // TODO: when ivan's cr lands eliminate this bandaid
+  CFRunLoopRun();
+}
+
+void unmount_callback(DADiskRef disk, DADissenterRef dissenter, void* context) {
+  NSLog(@"reached unmount callback");
+  if (dissenter) {
+    DAReturn status = DADissenterGetStatus(dissenter);
+    if (unix_err(status)) {
+      int code = err_get_code(status);
+      NSLog(@"Error code %d", code);
+    }
+  } else {
+    DADiskEject(disk, kDADiskEjectOptionDefault, eject_callback, context);
+  }
+}
+
+void eject_callback(DADiskRef disk, DADissenterRef dissenter, void* context) {
+  NSLog(@"reached eject callback");
+  if (dissenter) {
+    DAReturn status = DADissenterGetStatus(dissenter);
+    if (unix_err(status)) {
+      int code = err_get_code(status);
+      NSLog(@"Error code %d", code);
+    }
+  }
+  Unpacker* obj_self = (__bridge Unpacker*)context;

[Sidney San Martín, 2016/08/05 20:35:04]

These functions aren't really part of Unpacker, so maybe don't pretend they are?

- You should move them outside of the @implmentation/@end block (near the top of
the file is common).
- The variable should be called something like `unpacker`, not `*_self`.

[Sidney San Martín, 2016/08/05 21:13:40]

You should also use `static` on these functions to indicate that they're only
used within this file.

[Anna Zeng, 2016/08/05 21:48:40]

Done.

[Anna Zeng, 2016/08/05 21:48:40]

Done.

+  [obj_self callbackFollowDisk:disk];

[Sidney San Martín, 2016/08/05 20:35:04]

Could you include error info in the callback?

[Anna Zeng, 2016/08/05 21:48:40]

Done, with more progress to come!

+}
+
+- (void)callbackFollowDisk:(DADiskRef)disk {
+  NSLog(@"Unscheduling session");
+  DASessionUnscheduleFromRunLoop(
+      session, CFRunLoopGetCurrent(),
+      kCFRunLoopCommonModes);  // kCFRunLoopDefaultMode);
+
+  [delegate_ onUnpackSuccess];
+}
+
+@end