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
fix: remove usage of private APIs in the MAS build #17224
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
7f63632
fix: remove usage of abort_report_np in MAS builds
MarshallOfSound 86aa9ca
fix: remove usage of pthread_chdir in MAS builds
MarshallOfSound d229583
fix: remove usage of setapplicationisdaemon in MAS builds
MarshallOfSound 55552bb
chore: update patch manifest
MarshallOfSound File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
36 changes: 36 additions & 0 deletions
36
patches/common/chromium/fix_disable_usage_of_abort_report_np_in_mas_builds.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Samuel Attard <sattard@slack-corp.com> | ||
Date: Mon, 4 Mar 2019 14:43:36 -0800 | ||
Subject: fix: disable usage of abort_report_np in MAS builds | ||
|
||
|
||
diff --git a/sandbox/mac/sandbox_logging.cc b/sandbox/mac/sandbox_logging.cc | ||
index 4eebcea13d17093a685cc79b8a8f61fb3894c71b..d704dced154f1d5db097e566a9c681438f94475d 100644 | ||
--- a/sandbox/mac/sandbox_logging.cc | ||
+++ b/sandbox/mac/sandbox_logging.cc | ||
@@ -22,9 +22,11 @@ | ||
__builtin_unreachable(); \ | ||
} | ||
|
||
+#if !defined(MAS_BUILD) | ||
extern "C" { | ||
void abort_report_np(const char*, ...); | ||
} | ||
+#endif | ||
|
||
namespace sandbox { | ||
|
||
@@ -94,11 +96,13 @@ void SendAslLog(Level level, const char* message) { | ||
asl_set(asl_message.get(), ASL_KEY_MSG, message); | ||
asl_send(asl_client.get(), asl_message.get()); | ||
|
||
+ #if !defined(MAS_BUILD) | ||
if (__builtin_available(macOS 10.11, *)) { | ||
if (level == Level::FATAL) { | ||
abort_report_np(message); | ||
} | ||
} | ||
+ #endif | ||
} | ||
|
||
// |error| is strerror(errno) when a P* logging function is called. Pass |
54 changes: 54 additions & 0 deletions
54
.../common/chromium/fix_disable_usage_of_pthread_fchdir_np_and_pthread_chdir_np_in_mas.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Samuel Attard <sattard@slack-corp.com> | ||
Date: Mon, 4 Mar 2019 14:46:48 -0800 | ||
Subject: fix: disable usage of pthread_fchdir_np and pthread_chdir_np in MAS | ||
builds | ||
|
||
|
||
diff --git a/base/process/launch_mac.cc b/base/process/launch_mac.cc | ||
index 6c0e14fc3332c27309c83137cff9f060ed306aea..2f77af0cafbc0122603bc2735f6327e2e42a07b6 100644 | ||
--- a/base/process/launch_mac.cc | ||
+++ b/base/process/launch_mac.cc | ||
@@ -24,10 +24,12 @@ | ||
// descriptor. libpthread only exposes a syscall wrapper starting in | ||
// macOS 10.12, but the system call dates back to macOS 10.5. On older OSes, | ||
// the syscall is issued directly. | ||
+#if !defined(MAS_BUILD) | ||
extern "C" { | ||
int pthread_chdir_np(const char*) API_AVAILABLE(macosx(10.12)); | ||
int pthread_fchdir_np(int fd) API_AVAILABLE(macosx(10.12)); | ||
} | ||
+#endif | ||
|
||
namespace base { | ||
|
||
@@ -94,21 +96,29 @@ class PosixSpawnFileActions { | ||
}; | ||
|
||
int ChangeCurrentThreadDirectory(const char* path) { | ||
+ #if defined(MAS_BUILD) | ||
+ return syscall(SYS___pthread_chdir, path); | ||
+ #else | ||
if (__builtin_available(macOS 10.12, *)) { | ||
return pthread_chdir_np(path); | ||
} else { | ||
return syscall(SYS___pthread_chdir, path); | ||
} | ||
+ #endif | ||
} | ||
|
||
// The recommended way to unset a per-thread cwd is to set a new value to an | ||
// invalid file descriptor, per libpthread-218.1.3/private/private.h. | ||
int ResetCurrentThreadDirectory() { | ||
+ #if defined(MAS_BUILD) | ||
+ return syscall(SYS___pthread_fchdir, -1); | ||
+ #else | ||
if (__builtin_available(macOS 10.12, *)) { | ||
return pthread_fchdir_np(-1); | ||
} else { | ||
return syscall(SYS___pthread_fchdir, -1); | ||
} | ||
+ #endif | ||
} | ||
|
||
struct GetAppOutputOptions { |
52 changes: 52 additions & 0 deletions
52
patches/common/chromium/fix_disable_usage_of_setapplicationisdaemon_and.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Samuel Attard <sattard@slack-corp.com> | ||
Date: Mon, 4 Mar 2019 14:51:45 -0800 | ||
Subject: fix: disable usage of SetApplicationIsDaemon and | ||
_LSSetApplicationLaunchServicesServerConnectionStatus in MAS builds | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here too |
||
|
||
|
||
diff --git a/content/utility/utility_service_factory.cc b/content/utility/utility_service_factory.cc | ||
index 4450cc711772c600f138acb5458eb8ab0801ecf5..58e81aac8f8c97e5a3b3cd32b4d95789e14d2d31 100644 | ||
--- a/content/utility/utility_service_factory.cc | ||
+++ b/content/utility/utility_service_factory.cc | ||
@@ -195,7 +195,7 @@ void UtilityServiceFactory::RunNetworkServiceOnIOThread( | ||
std::unique_ptr<service_manager::Service> | ||
UtilityServiceFactory::CreateAudioService( | ||
service_manager::mojom::ServiceRequest request) { | ||
-#if defined(OS_MACOSX) | ||
+#if defined(OS_MACOSX) && !defined(MAS_BUILD) | ||
// Don't connect to launch services when running sandboxed | ||
// (https://crbug.com/874785). | ||
if (base::FeatureList::IsEnabled( | ||
diff --git a/sandbox/mac/system_services.cc b/sandbox/mac/system_services.cc | ||
index caa30bb378b30331f90057fe7ce3aec724104bf8..a766daa808495f7872051e129c6ad9f76f54e4fe 100644 | ||
--- a/sandbox/mac/system_services.cc | ||
+++ b/sandbox/mac/system_services.cc | ||
@@ -9,16 +9,19 @@ | ||
|
||
#include "base/mac/mac_logging.h" | ||
|
||
+#if !defined(MAS_BUILD) | ||
extern "C" { | ||
OSStatus SetApplicationIsDaemon(Boolean isDaemon); | ||
void _LSSetApplicationLaunchServicesServerConnectionStatus( | ||
uint64_t flags, | ||
bool (^connection_allowed)(CFDictionaryRef options)); | ||
} // extern "C" | ||
+#endif | ||
|
||
namespace sandbox { | ||
|
||
void DisableLaunchServices() { | ||
+ #if !defined(MAS_BUILD) | ||
// Allow the process to continue without a LaunchServices ASN. The | ||
// INIT_Process function in HIServices will abort if it cannot connect to | ||
// launchservicesd to get an ASN. By setting this flag, HIServices skips | ||
@@ -32,6 +35,7 @@ void DisableLaunchServices() { | ||
0, ^bool(CFDictionaryRef options) { | ||
return false; | ||
}); | ||
+ #endif | ||
} | ||
|
||
} // namespace sandbox |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hrrrrm is this what
export-patches
produced? the 2-line subject line is probably gonna cause issuesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, this is exactly what the script produced. It looks like the builds are OK and handle it though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm... does it survive
import-patches && export-patches
? if not, might have to kick theexport-patches
script