Objective-C: Methods called by a swizzled method should call the original implementation

Question

I'm fixing bugs in someone else's closed-source app.

In macOS, scrollbars can be set in System Preferences to display "always" (NSScrollerStyleLegacy), "when scrolling" (NSScrollerStyleOverlay), or "automatically based on mouse or trackpad" (NSScrollerStyleOverlay if a trackpad is connected, otherwise NSScrollerStyleLegacy). To check which style is in use, apps are supposed to do something like:

if ([NSScroller preferredScrollerStyle] == NSScrollerStyleLegacy)
    addPaddingForLegacyScrollbars();

Unfortunately, for some reason, this app is reading the value from NSUserDefaults instead (confirmed using a decompiler).

NSUserDefaults *defaults = [NSUserDefaults standardUserDefaults];
if ([[defaults objectForKey:@"AppleShowScrollBars"] isEqual: @"Always"])
    addPaddingForLegacyScrollbars();

This code incorrectly assumes any value of AppleShowScrollBars other than "Always" is equivalent to NSScrollerStyleOverlay. This will be wrong if the default is set to "Automatic" and no Trackpad is connected.

To fix this, I used the ZKSwizzle library to swizzle the NSUserDefaults objectForKey: method:

- (id)objectForKey:(NSString *)defaultName {
    if ([defaultName isEqual: @"AppleShowScrollBars"]) {
        if ([NSScroller preferredScrollerStyle] == NSScrollerStyleLegacy) {
            return @"Always";
        } else {
            return @"WhenScrolling";
        }
    }
    return ZKOrig(id, defaultName);
}

Unfortunately, this led to a stack overflow, because [NSScroller preferredScrollerStyle] will itself initially call [NSUserDefaults objectForKey:@"AppleShowScrollBars"] to check the user's preference. After some searching, I came across this answer on how to obtain the class name of a caller, and wrote:

- (id)objectForKey:(NSString *)defaultName {
    if ([defaultName isEqual: @"AppleShowScrollBars"]) {
        NSString *caller = [[[NSThread callStackSymbols] objectAtIndex:1] substringWithRange:NSMakeRange(4, 6)];
        if (![caller isEqualToString:@"AppKit"]) {
            if ([NSScroller preferredScrollerStyle] == NSScrollerStyleLegacy) {
                return @"Always";
            } else {
                return @"WhenScrolling";
            }
        }
    }
    return ZKOrig(id, defaultName);
}

This works perfectly! However, obtaining the caller uses the backtrace_symbols API intended for debugging, and comments on the aforementioned answer suggest this is a very bad idea. And, in general, returning different values depending on the caller feels yucky.

Obviously, if this was my own code, I would rewrite it to use preferredScrollerStyle instead of NSUserDefaults in the first place, but it's not, so I can only make changes at method boundaries.

What I fundamentally want is for this method to be swizzled only when it's called above me in the stack. Any calls further down the stack should use the original implementation.

Is there a way to do this, or is my current solution reasonable?

Answer 1

This approach is probably ok (within the context of "I've already decided to swizzle"), but it does feel a bit fragile as you note, and callStackSymbols can be very slow, and what information is available depends on whether debug symbols are available (which probably won't ever break this particular use case, but if it does, the bug will be very confusing).

I think you can make this more robust and much faster by short-circuiting recursion with a static variable.

- (id)objectForKey:(NSString *)defaultName {
    static BOOL isRunning = false;

    if (!isRunning && [defaultName isEqual: @"AppleShowScrollBars"]) {
        isRunning = true;
        NSScrollerStyle scrollerStyle = [NSScroller preferredScrollerStyle];
        isRunning = false;

        if (scrollerStyle == NSScrollerStyleLegacy) {
            return @"Always";
        } else {
            return @"WhenScrolling";
        }
    }
    return ZKOrig(id, defaultName);
}

static variables within a function retain their value between calls, so you can use this to detect that recursion is happening. (This is not thread-safe, but that shouldn't be a problem in this use case. Also note that all instances of this class share the same static variable. That shouldn't matter here since you're swizzling a specific object.)

If this function is reentered, then it'll just skip down to the original implementation.