Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: re-enable devtools renderer sandbox #16864

Merged
merged 2 commits into from Feb 15, 2019

Conversation

@miniak
Copy link
Contributor

commented Feb 10, 2019

Description of Change

/cc @nornagon, @deepak1556

Checklist

Release Notes

Notes: Enabled sandboxing of devtools and chrome extension background script host renderers.

@miniak miniak added the wip label Feb 10, 2019

@miniak miniak self-assigned this Feb 10, 2019

@miniak miniak requested a review from as a code owner Feb 10, 2019

@miniak miniak force-pushed the miniak/sandbox-devtools-renderers branch from dad8bf8 to 8beae03 Feb 11, 2019

@miniak

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

The tests for MAS builds are failing due to the devtools renderer crashing with this call stack:

Application Specific Information:
abort() called
rdar://problem/28724618 Couldn't create connection object
_RegisterApplication(), unable to get application ASN from launchservicesd, and this application requires an ASN, so aborting.  error=#-1.
 

Thread 0 Crashed:: CrRendererMain  Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	0x00007fff5efb4b66 __pthread_kill + 10
1   libsystem_pthread.dylib       	0x00007fff5f17f080 pthread_kill + 333
2   libsystem_c.dylib             	0x00007fff5ef1024d __abort + 144
3   libsystem_c.dylib             	0x00007fff5ef101bd abort + 142
4   com.apple.HIServices          	0x00007fff3578f60a _RegisterApplication + 7911
5   com.apple.HIServices          	0x00007fff3578d64c GetCurrentProcess + 24
6   com.apple.HIToolbox           	0x00007fff361ed4ab MenuBarInstance::GetAggregateUIMode(unsigned int*, unsigned int*) + 63
7   com.apple.AppKit              	0x00007fff34eb5627 _NSScreenConfigurationUpdateSharedInfoForReason + 1546
8   com.apple.AppKit              	0x00007fff34eb4fc9 ___NSScreenConfigurationEnsureInitialUpdateOccurred_block_invoke + 137
9   libdispatch.dylib             	0x00007fff5ee2adb8 _dispatch_client_callout + 8
10  libdispatch.dylib             	0x00007fff5ee2ad6b dispatch_once_f + 41
11  com.apple.AppKit              	0x00007fff34eb4f3e +[_NSScreenConfiguration latestGreatestBackingScaleFactor] + 128
12  com.apple.AppKit              	0x00007fff3450fd83 -[NSView _transformToBackingUsingIntegralizationSpace:] + 242
13  com.apple.AppKit              	0x00007fff3454a063 -[NSView _primitiveConvertSizeToBacking:useIntegralizationSpace:] + 63
14  com.apple.AppKit              	0x00007fff3455535b NSCellImageRectWithSize_centeredInRect_scaling_flipped + 56
15  com.apple.AppKit              	0x00007fff34568ec4 -[NSButtonCell(NSButtonCellPrivate) _imageRectWithRect:] + 244
16  com.apple.AppKit              	0x00007fff34940407 -[NSButtonCell _imageRect:titleRect:forBounds:] + 275
17  com.apple.AppKit              	0x00007fff3458fa4f -[NSButtonCell imageRectForBounds:] + 114
18  com.apple.AppKit              	0x00007fff34619436 -[NSButtonCell drawInteriorWithFrame:inView:] + 1627
19  com.apple.AppKit              	0x00007fff346184fc -[NSButtonCell drawWithFrame:inView:] + 481
20  libblink_core.dylib           	0x000000014972eda5 blink::ThemePainterMac::PaintRadio(blink::Node const*, blink::Document const&, blink::ComputedStyle const&, blink::PaintInfo const&, blink::IntRect const&) + 1317 (theme_painter_mac.mm:694)
...

@nornagon, @zcbenz do you have any idea why this could be happening? The non-MAS builds are working fine.

@nornagon

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2019

//electron/patches/common/chromium/mas_blink_no_private_api.patch does contain changes to ThemePainterMac; that'd be a good place to start.

@miniak

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

@nornagon I've checked the changes in ThemePainterMac, we are not changing the way we render radio buttons. Actually, just calling GetCurrentProcess in AtomMain for example crashes the process in the same way:

Application Specific Information:
dyld: in dlopen()
abort() called
rdar://problem/28724618 Couldn't create connection object
_RegisterApplication(), unable to get application ASN from launchservicesd, and this application requires an ASN, so aborting.  error=#-1.
 

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	0x00007fff5efb4b66 __pthread_kill + 10
1   libsystem_pthread.dylib       	0x00007fff5f17f080 pthread_kill + 333
2   libsystem_c.dylib             	0x00007fff5ef101ae abort + 127
3   com.apple.HIServices          	0x00007fff3578f60a _RegisterApplication + 7911
4   com.apple.HIServices          	0x00007fff3578d64c GetCurrentProcess + 24
5   com.github.Electron.framework 	0x00000001056ae757 AtomMain + 71 (atom_library_main.mm:26)
6   com.github.Electron.helper    	0x00000001054eb8ae main + 430 (atom_main.cc:245)
7   libdyld.dylib                 	0x00007fff5ee64015 start + 1

when that happens, I can see that SeatbeltExecServer::InitializeSandbox was used to sandbox the process, which seems to be causing the issue somehow. Continuing the investigation...

@miniak miniak force-pushed the miniak/sandbox-devtools-renderers branch 3 times, most recently from ff3f490 to 4ba455f Feb 13, 2019

@miniak miniak force-pushed the miniak/sandbox-devtools-renderers branch from 4ba455f to dcb75d5 Feb 14, 2019

@miniak miniak changed the title [wip] chore: re-enable devtools renderer sandbox chore: re-enable devtools renderer sandbox Feb 14, 2019

@miniak miniak removed the wip label Feb 14, 2019

@miniak miniak requested review from nornagon and deepak1556 Feb 14, 2019

@deepak1556 deepak1556 merged commit 975a035 into master Feb 15, 2019

15 checks passed

Artifact Comparison No Changes
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-debug AppVeyor build succeeded
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-debug AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20190214.27 succeeded
Details
electron-arm64-testing Build #20190214.27 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Feb 15, 2019

Release Notes Persisted

Enabled sandboxing of devtools and chrome extension background script host renderers.

@deepak1556 deepak1556 deleted the miniak/sandbox-devtools-renderers branch Feb 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.