Fixing macOS native tabs for Visual Studio Code
I helped track down the misbehaving macOS compatibility patch that broke native tabs support in VS Code. I also learned to avoid introducing new bugs in bugfixes.
Introduction
After writing the AppKit Bundle ID post, a Visual Studio Code developer reached out to me about one misbehaving bundle ID check.
VSCode, it turns out, couldn’t display native macOS tabs on High Sierra: after selecting Window->Merge Window, there should be a tab bar at the top of the window:
But on High Sierra, the bar is completely empty:
(screenshot by @ddotlic)
The VS Code developers realized the issue only happens if the bundle ID begins with com.microsoft. Changing the bundle ID to com.microsoft2.VSCode fixes it.
This definitely sounds like a misbehaving compatibility patch, since it’s only applied to specific bundle IDs. But which one? And how can we disable it?
Narrowing down the issue
I was able to reproduce the issue, and confirmed that changing the bundle ID fixes it.
I then started looking for the misbehaving patch. I found the “com.microsoft.” text when running strings on AppKit: so it’s checking it somewhere. However, IDA Free’s “Xref” command showed multiple method all checking for com.microsoft. as a bundle prefix. Thus, I had to manually find which method is actually responsible for the bug.
My first approach was to attach a debugger to VS Code, and examine every access to the main bundle of the app.
All accesses to the main bundle, including Cocoa’s +[NSBundle mainBundle] and Carbon’s various methods, eventually uses CFBundleGetMainBundle(). I thought putting a breakpoint on this method would allow me to find every place that checks for bundle IDs.
When I ran the app, I was greeted with a flood of calls, too many to analyze. I should’ve known: an app needs its main bundle for many reasons: for example, getting resources. There’s no easy way to limit this breakpoint to the calls getting just the bundle ID.
Dyld injection and swizzling
I decided instead to override -[NSBundle bundleIdentifier] such that, if the original return value is “com.microsoft.VSCodeInsiders”, I return a non-Microsoft bundle ID. If this fixes the bug, then I’ll know that the broken code uses -[NSBundle bundleIdentifier], not CoreFoundation or any other bundle API.
To override an Objective-C method, I followed New Relic’s tutorial for method swizzling. I look up the original Method on the NSBundle class, save the original implementation in a function pointer, and substitute my own implementation. When my method is called, I forward it to the original code, and change the return value if needed.
To load my code into VS Code, I used the DYLD_INSERT_LIBRARIES environmental variable, which specifies a shared library to load into the process at launch. (It’s similar to LD_PRELOAD on Linux.) While there are better ways to get code to run once inserted, I chose to use a constructor function because I wanted to try it.
My initial test code is shown below:
@import Darwin;
@import Foundation;
@import ObjectiveC;
NSString* (*NSBundle_bundleIdentifier_real)(NSBundle* bundle, SEL selector);
NSString* NSBundle_bundleIdentifier_hook(NSBundle* bundle, SEL selector) {
NSString* retval = NSBundle_bundleIdentifier_real(bundle, selector);
if ([retval isEqualToString:@"com.microsoft.VSCodeInsiders"]) {
return @"com.worthdoingbadly.vscode";
}
return retval;
}
__attribute__((constructor))
void bundlehook_init() {
Method method = class_getInstanceMethod([NSBundle class], @selector(bundleIdentifier));
NSBundle_bundleIdentifier_real = method_getImplementation(method);
method_setImplementation(method, &NSBundle_bundleIdentifier_hook);
}
$ clang -g -fmodules -Xlinker -dylib -o libbundlenameinject.dylib bundlenameinject.m
$ DYLD_INSERT_LIBRARIES=`pwd`/libbundlenameinject.dylib /Applications/Visual\ Studio\ Code.app/Contents/MacOS/Electron
and the result?
Yep, the tab bar is working. So I know the issue is caused by some code calling -[NSBundle bundleIdentifier]. Now to find which one.
Isolating the method
I need to find which method is calling us to obtain the main bundle ID. A StackOverflow guide told me to use the +[NSThread callStackSymbols] method, which returns an Array of all the method names in the call stack. We’re on the top of the call stack, so the method that called us must be at position [1] of that array.
Compatibility flags are calculated with code that have names ending with DefaultValueFunction. So I tried only overriding the bundle ID if the calling function’s name contains DefaultValueFunction.
NSString* NSBundle_bundleIdentifier_hook(NSBundle* bundle, SEL selector) {
NSString* retval = NSBundle_bundleIdentifier_real(bundle, selector);
if ([retval isEqualToString:@"com.microsoft.VSCodeInsiders"]) {
NSArray<NSString*>* stack = [NSThread callStackSymbols];
bool override = [stack[1] containsString:@"DefaultValueFunction"];
fprintf(stderr, "%s %s\n", override? "yes": "no", stack[1].UTF8String);
if (override) {
return @"com.worthdoingbadly.vscode";
}
}
return retval;
}
Yep, the tab bar function is still fixed. So it’s one of the compatibility calls. But which one?
I decided to use binary search to eliminate half the candidate functions each time.
I first logged the names of all the DefaultValueFunctions that got the main bundle ID, and put them into an Array.
Next, I made my hook only return an overridden bundle ID if the method’s name is in my list.
Finally, I comment out half of the list at a time. If the tabs still work, then the method that checks the bundle ID is in the uncommented half. If the tabs broke, then the method is in the commented half.
Eliminate the wrong half, and eepeat until all but one method is commented out.
Here’s my code:
@import Darwin;
@import Foundation;
@import ObjectiveC;
NSString* (*NSBundle_bundleIdentifier_real)(NSBundle* bundle, SEL selector);
NSArray<NSString*>* mystrs;
NSString* NSBundle_bundleIdentifier_hook(NSBundle* bundle, SEL selector) {
NSString* retval = NSBundle_bundleIdentifier_real(bundle, selector);
if ([retval isEqualToString:@"com.microsoft.VSCodeInsiders"]) {
fprintf(stderr, "Bundle! %s\n", retval.UTF8String);
NSArray<NSString*>* stack = [NSThread callStackSymbols];
bool override = false;
for (NSString* str in mystrs) {
if ([stack[1] containsString:str]) {
override = true;
break;
}
}
fprintf(stderr, "%s %s\n", override? "yes": "no", stack[1].UTF8String);
if (override) {
fprintf(stderr, "%s\n", [NSThread callStackSymbols].description.UTF8String);
return @"com.worthdoingbadly.vscode";
}
}
return retval;
}
__attribute__((constructor))
void bundlehook_init() {
Method method = class_getInstanceMethod([NSBundle class], @selector(bundleIdentifier));
NSBundle_bundleIdentifier_real = method_getImplementation(method);
method_setImplementation(method, &NSBundle_bundleIdentifier_hook);
mystrs = @[
/*
@"NSScreenGettingScreensHasSideEffectsDefaultValueFunction",
@"NSWindowAllowsImplicitFullScreenDefaultValueFunction",
@"NSApplicationLaunchMicrosoftUpdaterDefaultValueFunction",
@"NSApplicationFunctionRowControllerIsWebKitPluginProcessDefaultValueFunction",
@"NSSavePanelGuardAgainstSwizzledClassDefaultValueFunction",
*/
@"NSUseImprovedLayoutPassDefaultValueFunction",
/*
@"NSViewIsWebKitPluginProcessDefaultValueFunction",
@"NSIsPreLoboOmniGraffleDefaultValueFunction",
@"NSViewBuildLayerTreeOnForcibleDisplayDefaultValueFunction",
@"NSMightNeedToWorkAroundBadAdobeReleaseBugDefaultValueFunction",
@"NSViewKeepLayerSurfacesBehindTitlebarLayerSurfaceDefaultValueFunction",
@"NSCGSIsSynchronousM7DefaultValueFunction",
@"NSCGSIsSynchronousM7DefaultValueFunction",
@"NSViewAvoidDirtyLayoutWhenUpdatingAutoresizingConstraintsDefaultValueFunction",
*/
];
fprintf(stderr, "Launched!\n");
}
Using this, I was able to isolate the method: NSUseImprovedLayoutPassDefaultValueFunction.
Examining the method
Disassembling the method in IDA Free reveals that it does check for bundle IDs beginning with com.microsoft..
This method returns true normally, but returns false if an app needs the old behaviour. However, like most compatibility patches, this value can be overridden by setting the relevant variable, NSUseImprovedLayoutPass, through NSUserDefaults, macOS’s preference system.
An NSUserDefaults value can be set temporarily by passing it on the command line. So I launched VS Code with NSUseImprovedLayoutPass temporarily set to true:
/Applications/Visual\ Studio\ Code.app/Contents/MacOS/Electron -- -NSUseImprovedLayoutPass true
And the tabs worked without changing the bundle ID.
Making the pull request
So all I had to do was to set this option in VS Code, and the tabs should be fixed.
There are two ways to set an NSUserDefaults value in code: setBool:forKey sets it permanently, while registerDefaults sets it temporarily until the program exits.
Electron exposes both methods: I decided to use the temporary method for this override since it seemed cleaner to me, so all I had to do was call
systemPreferences.registerDefaults({ NSUseImprovedLayoutPass: true });
before any windows are created, and native tabs worked in VS Code.
I quickly sent a Pull Request to the VS Code GitHub repository.
An ironic code review
Unlike my previous attempt to submit pull requests to VS Code, I got a code review almost immediately - and found out I made an ironic mistake.
My fix enables the workaround for all Mac users, not just the minority that use Native Tabs support. This makes it much more likely to break something unintentionally. The workaround isn’t specifically restricted to those with the issue - just like how Apple’s original patch broke VS Code by not being specific enough when targeting the bundle ID.
To avoid introducing new bugs, the VS Code developer changed the patch to only set the override when it’s needed. (and also switched to setUserDefault since Electron 1.7.x didn’t have registerDefaults.).
So while my original pull request wasn’t up to VS Code’s standards, I now know what to do in the future to avoid introducing new bugs.
What I learned
- Overriding methods is easy with macOS and Objective-C
- When making a change, be as specific as possible to minimize side effects
- Try to fix issues without creating more bugs