Clean up and fix for OS X's behavior of restoring windows on crash.

- Added comments and cleaned up the code to conform to style guide.
- Refactored pieces of session wrapper.
- Bugfix for handling two errors in a row - we now flush stderr buffer
  more often.

BUG=
R=scudette@gmail.com

Review URL: https://codereview.appspot.com/106850044

osx/Rekall/Rekall/Base.lproj/RKDocument.xib

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="5056" systemVersion="13C64" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" useAutolayout="YES">
+<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="5056" systemVersion="13C1021" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" useAutolayout="YES">
     <dependencies>
         <plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="5056"/>
         <plugIn identifier="com.apple.WebKitIBPlugin" version="5056"/>
@@ -14,14 +14,14 @@
         </customObject>
         <customObject id="-1" userLabel="First Responder" customClass="FirstResponder"/>
         <customObject id="-3" userLabel="Application"/>
-        <window title="Window" allowsToolTipsWhenApplicationIsInactive="NO" autorecalculatesKeyViewLoop="NO" releasedWhenClosed="NO" visibleAtLaunch="NO" animationBehavior="default" id="xOd-HO-29H" userLabel="Window">
+        <window title="Window" allowsToolTipsWhenApplicationIsInactive="NO" autorecalculatesKeyViewLoop="NO" restorable="NO" releasedWhenClosed="NO" visibleAtLaunch="NO" animationBehavior="default" id="xOd-HO-29H" userLabel="Window">
             <windowStyleMask key="styleMask" titled="YES" closable="YES" miniaturizable="YES" resizable="YES"/>
             <windowPositionMask key="initialPositionMask" leftStrut="YES" rightStrut="YES" topStrut="YES" bottomStrut="YES"/>
-            <rect key="contentRect" x="133" y="235" width="800" height="600"/>
-            <rect key="screenRect" x="0.0" y="0.0" width="1440" height="878"/>
+            <rect key="contentRect" x="133" y="235" width="816" height="616"/>
+            <rect key="screenRect" x="0.0" y="0.0" width="1920" height="1178"/>
             <value key="minSize" type="size" width="400" height="200"/>
             <view key="contentView" id="gIp-Ho-8D9">
-                <rect key="frame" x="0.0" y="0.0" width="800" height="600"/>
+                <rect key="frame" x="0.0" y="0.0" width="816" height="616"/>
                 <autoresizingMask key="autoresizingMask"/>
                 <subviews>
                     <webView misplaced="YES" translatesAutoresizingMaskIntoConstraints="NO" id="zaO-NW-tGg">
@@ -32,7 +32,7 @@
                         </webPreferences>
                     </webView>
                     <progressIndicator horizontalHuggingPriority="750" verticalHuggingPriority="750" misplaced="YES" maxValue="100" bezeled="NO" indeterminate="YES" style="spinning" translatesAutoresizingMaskIntoConstraints="NO" id="6vI-Fs-t5N">
-                        <rect key="frame" x="384" y="284" width="32" height="32"/>
+                        <rect key="frame" x="384" y="300" width="32" height="32"/>
                         <autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
                     </progressIndicator>
                 </subviews>

osx/Rekall/Rekall/RKDocument.h

@@ -9,9 +9,23 @@
 #import <Cocoa/Cocoa.h>
 #import <WebKit/WebKit.h>
 
+#import "RKSessionWrapper.h"
+
+/** This main document class handles loading of raw memory images, as well as saving and restoring
+ *  Rekall sessions.
+ */
 @interface RKDocument : NSDocument
 
-@property (retain) IBOutlet WebView *webView;
-@property (retain) IBOutlet NSProgressIndicator *spinner;
+/** The WebView we wrap around to display the webconsole. */
+@property (nonatomic, strong) IBOutlet WebView *webView;
+
+/** Displayed before the WebView is ready. */
+@property (nonatomic, strong) IBOutlet NSProgressIndicator *spinner;
+
+/** The rekall python instance we're connected to. */
+@property (strong) RKSessionWrapper *rekall;
+
+/** The (HTTP) URL that rekall's webconsole is running at. */
+@property (copy) NSURL *rekallURL;
 
 @end

osx/Rekall/Rekall/RKDocument.m

@@ -11,28 +11,54 @@
 
 @interface RKDocument ()
 
-@property (retain) RKSessionWrapper *rekall;
-@property (retain) NSURL *rekallURL;
-
+/** This is the URL we will eventually load, once the WebView is initialized and ready. */
 @property (copy) NSURL *deferredLoadURL;
+
+/** This gets called every time WebView finishes loading a page. */
 @property (copy, nonatomic) void (^webViewLoadCallback)(WebView *);
 
+/** Connects to the webconsole at self.rekallURL. */
 - (void)loadWebconsole:(id)sender;
+
+/** Loads an error page into the WebView and displays contents of error. */
 - (void)reportError:(NSError *)error;
 
+/** Loads url at the earliest possible time and executes callback once load finishes. */
 - (void)loadPageWhenReady:(NSURL *)url callback:(void (^)(WebView *))callback;
+
+/** Loads resource from the main bundle and executes it as javascript. */
 - (void)injectJavascriptResource:(NSString *)resource;
+
+/** Runs javascript from the argument in the main WebView.
+ *  @param js The javascript to execute.
+ *  @return Output of the javascript, as string.
+ */
 - (NSString *)injectJavascript:(NSString *)js;
-- (NSString *)callJavascriptFunction:(NSString *)function args:(NSArray *)args onReady:(BOOL)onReady;
 
-@end
+/** Calls the javascript function, while safely escaping its arguments.
+ *  @param function The name of the function to run.
+ *  @param args Arguments to the function. For now, only instances of NSString.
+ *  @param onReady Wraps the invocation in a jQuery 'onReady' block. (Must have loaded jQuery.)
+ *  @return Return value of the javascript function, as string.
+ */
+- (NSString *)callJavascriptFunction:(NSString *)function
+                                args:(NSArray *)args
+                             onReady:(BOOL)onReady;
 
+@end
 
+/** Needed to talk to the WebView's main frame. */
 @interface RKDocument (WebFrameLoadDelegate)
 
 - (void)webView:(WebView *)sender didFinishLoadForFrame:(WebFrame *)frame;
-- (void)webView:(WebView *)sender didFailProvisionalLoadWithError:(NSError *)error forFrame:(WebFrame *)frame;
-- (void)webView:(WebView *)sender didFailLoadWithError:(NSError *)error forFrame:(WebFrame *)frame;
+
+- (void)webView:(WebView *)sender
+    didFailProvisionalLoadWithError:(NSError *)error
+                           forFrame:(WebFrame *)frame;
+
+- (void)webView:(WebView *)sender
+    didFailLoadWithError:(NSError *)error
+                forFrame:(WebFrame *)frame;
 
 @end
 
@@ -48,7 +74,9 @@ - (void)webView:(WebView *)sender didFinishLoadForFrame:(WebFrame *)frame {
     [self.spinner removeFromSuperview];
 }
 
-- (void)webView:(WebView *)sender didFailProvisionalLoadWithError:(NSError *)error forFrame:(WebFrame *)frame {
+- (void)webView:(WebView *)sender
+    didFailProvisionalLoadWithError:(NSError *)error
+                           forFrame:(WebFrame *)frame {
     NSLog(@"WebView provisional load error: %@", error);
 }
 
@@ -181,7 +209,11 @@ - (NSData *)dataOfType:(NSString *)typeName error:(NSError *__autoreleasing *)ou
     return nil;
 }
 
-- (BOOL)readFromURL:(NSURL *)url ofType:(NSString *)typeName error:(NSError *__autoreleasing *)outError {
+- (BOOL)readFromURL:(NSURL *)url
+             ofType:(NSString *)typeName
+              error:(NSError *__autoreleasing *)outError {
+    NSLog(@"Opening document of type %@ from %@.", typeName, url);
+
     self.rekall = [[RKSessionWrapper alloc] init];
     __weak id weakSelf = self;
 
@@ -191,11 +223,13 @@ - (BOOL)readFromURL:(NSURL *)url ofType:(NSString *)typeName error:(NSError *__a
     };
 
     self.rekall.onErrorCallback = ^(NSError *error) {
+        // I get called if RKSessionWrapper detects an error.
         [weakSelf reportError:error];
     };
 
+    // Did we get passed a saved session? If so, restore it.
     if ([typeName isEqualToString:@"com.google.rekall-session"]) {
-        // Restore saved session.
+        // This is a work in progress.
         NSLog(@"Not yet implemented.");
         return NO;
     }

osx/Rekall/Rekall/RKSessionWrapper.h

@@ -8,21 +8,48 @@
 
 #import <Foundation/Foundation.h>
 
+/** RKSessionWrapper.port will be set to this value until it's set to a real value. */
 extern const NSInteger RKNullPortNumberSentinel;
+
+/** Error domain for all errors encountered while talking to the python rekall instance. */
 extern NSString *const RKErrorDomain;
+
+/** Key used to store error title in the userInfo dict of errors in the RKErrorDomain. */
 extern NSString *const RKErrorTitle;
+
+/** Key used to store error description in the userInfo dict of errors in the RKErrorDomain. */
 extern NSString *const RKErrorDescription;
 
+/** Error number for unspecified error when talking to python rekall instance. */
 #define RKSessionRekallError 500
 
-
+/** This class is responsible for executing a rekall python instance, launching a web console
+ *  session, figuring out the port it binds to and making that information available.
+ */
 @interface RKSessionWrapper : NSObject
 
+/** The port number on loopback that the rekall webconsole is running its HTTP server.
+ *  This is initialized to RKNullPortNumberSentinel and stays that way until the real port is known.
+ */
 @property (assign, readonly, nonatomic) NSInteger port;
+
+/** Callback once the web session is up and the port is known. */
 @property (copy, nonatomic) void (^onLaunchCallback)(void);
+
+/** Callback in case errors are encountered. */
 @property (copy, nonatomic) void (^onErrorCallback)(NSError *error);
 
+/** Will start a rekall instance on image at path and launch a webconsole.
+ *  On success, will call onLaunchCallback. On failure, will either fill errorBuf or call
+ *  onErrorCallback, depending on when the error occurs.
+ *
+ *  @param path The path to a memory image to be passed to rekall process.
+ *  @param errorBuf Will be populated with an NSError instance in case of immediate errors.
+ *  @return NO if errorBuf is filled, otherwise YES.
+ */
 - (BOOL)startWebconsoleWithImage:(NSURL *)path error:(NSError **)errorBuf;
+
+/** Will terminate the current rekall instance, if any. */
 - (void)stopRekallWebconsoleSession;
 
 @end

osx/Rekall/Rekall/RKSessionWrapper.m

@@ -9,25 +9,32 @@
 #import "RKSessionWrapper.h"
 
 const NSInteger RKNullPortNumberSentinel = -1;
-
 NSString * const RKErrorDomain = @"RKErrorDomain";
 NSString * const RKErrorTitle = @"RKErrorTitle";
 NSString * const RKErrorDescription = @"RKErrorDescription";
-
 NSString * const RKWrapperPortPattern = @"Server running at http://.*?:(\\d+)";
 NSString * const RKWrapperErrorPattern = @"([a-z]*?Error):\\s?(.*?)$";
 
+/** Used to look for webconsole port number in rekall's stderr. */
 static NSRegularExpression *RKWrapperPortRegex;
+
+/** Used to look for python exceptions and errors in rekall's stderr. */
 static NSRegularExpression *RKWrapperErrorRegex;
 
 @interface RKSessionWrapper ()
 
+@property (assign, readwrite) NSInteger port; // override to make readwrite
+
+/** Handle to the rekall python process. */
 @property (retain) NSTask *rekallTask;
-@property (assign, readwrite) NSInteger port;
 
+/** Pipe to rekall's stderr output. */
 @property (retain) NSPipe *rekallStdErr;
+
+/** Buffer that holds recent stderr output from rekall. */
 @property (retain) NSMutableString *rekallStdErrString;
 
+/** Will attempt to extract the webconsole port, or an error message from rekall's stderr. */
 - (void)tryExtractWebconsoleAddress;
 
 @end
@@ -37,30 +44,41 @@ @implementation RKSessionWrapper
 
 @synthesize port, rekallTask, rekallStdErr, rekallStdErrString, onLaunchCallback, onErrorCallback;
 
-+ (void)initialize {
+- (id)init {
+    if(![super init]) {
+        return nil;
+    }
 
+    self.port = RKNullPortNumberSentinel;
+
+    return self;
 }
 
 - (void)tryExtractWebconsoleAddress {
     NSLog(@"Parsing rekall stderr output for errors and bind info.");
 
     if(!RKWrapperPortRegex) {
-        RKWrapperErrorRegex = [NSRegularExpression regularExpressionWithPattern:RKWrapperErrorPattern
-                                                                        options:NSRegularExpressionCaseInsensitive
-                                                                          error:nil];
-        RKWrapperPortRegex = [NSRegularExpression regularExpressionWithPattern:RKWrapperPortPattern
-                                                                       options:NSRegularExpressionCaseInsensitive
-                                                                         error:nil];
+        // Initialize regular expressions only once.
+        RKWrapperErrorRegex = [NSRegularExpression
+                               regularExpressionWithPattern:RKWrapperErrorPattern
+                                                    options:NSRegularExpressionCaseInsensitive
+                                                      error:nil];
+
+        RKWrapperPortRegex = [NSRegularExpression
+                              regularExpressionWithPattern:RKWrapperPortPattern
+                                                   options:NSRegularExpressionCaseInsensitive
+                                                     error:nil];
     }
 
     NSTextCheckingResult *match;
 
-    // Any errors?
+    // First, check for errors in the stderr buffer.
     match = [RKWrapperErrorRegex firstMatchInString:self.rekallStdErrString
                                            options:0
                                              range:(NSRange) {0, self.rekallStdErrString.length}];
 
     if(match) {
+        // We found an error message. Create an NSError and call the appropriate callback.
         NSString *errorTitle = [self.rekallStdErrString substringWithRange:[match rangeAtIndex:1]];
         NSString *errorDesc = [self.rekallStdErrString substringWithRange:[match rangeAtIndex:2]];
         NSLog(@"Rekall error: %@\n%@", errorTitle, errorDesc);
@@ -77,18 +95,24 @@ - (void)tryExtractWebconsoleAddress {
             self.onErrorCallback(error);
         }
 
+        // Always flush the stderr buffer when we find a match.
+        [self.rekallStdErrString setString:@""];
+
         return;
     }
 
-    // Port number?
+    // No errors - look for the port number.
     match = [RKWrapperPortRegex firstMatchInString:self.rekallStdErrString
                                            options:0
                                              range:(NSRange) {0, self.rekallStdErrString.length}];
 
     if(!match) {
+        // No port number and no errors - we need more stuff in our buffer. Wait.
         return;
     }
 
+    // Port number found - parse it out and report to the callback.
+
     NSString *result = [self.rekallStdErrString substringWithRange:[match rangeAtIndex:1]];
     NSLog(@"Match found: %@", result);
 
@@ -97,22 +121,29 @@ - (void)tryExtractWebconsoleAddress {
     [scanner scanInteger:&scannedPortNumber];
     self.port = scannedPortNumber;
 
+    // Flush on match.
+    [self.rekallStdErrString setString:@""];
+
     // Execute callback since we found the port number.
     if(self.onLaunchCallback) {
         self.onLaunchCallback();
     }
 }
 
 - (BOOL)startWebconsoleWithImage:(NSURL *)path error:(NSError **)errorBuf {
+    // There shouldn't be a rekall instance already running, but let's err on the side of caution,
+    // since we don't want to leave orphaned processes lying around.
     [self stopRekallWebconsoleSession];
 
+    // Path to the rekall (rekal) executable.
     NSString *rekallPath = [[NSBundle mainBundle] pathForResource:@"rekal"
                                                            ofType:nil
                                                       inDirectory:@"rekal"];
+    // Arguments:
     NSArray *rekallArgs = [NSArray arrayWithObjects:
-                           @"-f", [path path],
-                           @"webconsole",
-                           @"--no_browser",
+                           @"-f", [path path], // image path
+                           @"webconsole", // plugin name
+                           @"--no_browser", // don't open Safari
                            nil];
 
     // Omitting the port number will cause rekall to bind a random available port.
@@ -121,8 +152,9 @@ - (BOOL)startWebconsoleWithImage:(NSURL *)path error:(NSError **)errorBuf {
     // Until the port number is detected, self.port will be RKNullPortNumberSentinel.
 
     self.rekallStdErrString = [[NSMutableString alloc] init];
-    self.port = RKNullPortNumberSentinel;
 
+    // Set up a pipe for rekall to write standard error into, and attach a handler that'll
+    // append everything to a buffer and pass it to tryExtractWebconsoleAddress.
     self.rekallStdErr = [NSPipe pipe];
     self.rekallStdErr.fileHandleForReading.readabilityHandler = ^(NSFileHandle *handle) {
         NSData *data = [handle availableData];
@@ -133,7 +165,7 @@ - (BOOL)startWebconsoleWithImage:(NSURL *)path error:(NSError **)errorBuf {
             [self.rekallStdErrString appendString:string];
         }
 
-        // Pass this through to the actual stderr.
+        // Pass this through to the actual stderr, in case anyone's reading it.
         NSLog(@"Rekall stderr output:\n%@", string);
 
         // Try and see if rekall has written a port number to stderr.
@@ -151,6 +183,7 @@ - (BOOL)startWebconsoleWithImage:(NSURL *)path error:(NSError **)errorBuf {
 }
 
 - (void)stopRekallWebconsoleSession {
+    self.port = RKNullPortNumberSentinel;
     [self.rekallTask terminate];
 }