| Summary: | [iOS, macOS] Allow access to the container manager to support Mail InjectedBundle | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||||||||
| Component: | WebKit Misc. | Assignee: | Brent Fulgham <bfulgham> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | bfulgham, darin, ddkilzer, pvollan | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Brent Fulgham
2020-06-18 15:19:12 PDT
Created attachment 402240 [details]
Patch
Created attachment 402242 [details]
Patch
Comment on attachment 402242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402242&action=review Is there any more scalable way to do this? Seems unfortunate to hardcode a client app bundle ID again. > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:299 > +#else How about this instead? #elif PLATFORM(IOS) return WebCore::IOSApplication::isMobileMail() #else return false; #endif Comment on attachment 402242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402242&action=review >> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:299 >> +#else > > How about this instead? > > #elif PLATFORM(IOS) > return WebCore::IOSApplication::isMobileMail() > #else > return false; > #endif The problem is that it’s not just MobileMail that needs access. We see other apps that use the WebContent process and talk to containermanagerd as well. This only fixes the majority of cases, but not all of them. Comment on attachment 402242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402242&action=review >>> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:299 >>> +#else >> >> How about this instead? >> >> #elif PLATFORM(IOS) >> return WebCore::IOSApplication::isMobileMail() >> #else >> return false; >> #endif > > The problem is that it’s not just MobileMail that needs access. We see other apps that use the WebContent process and talk to containermanagerd as well. > > This only fixes the majority of cases, but not all of them. Darin: Will do! David: I'll add telemetry-backtrace so we get a backtrace if this is hit by anyone else. I'm very interested to see how that's possible. Created attachment 402319 [details]
Patch for landing
Committed r263287: <https://trac.webkit.org/changeset/263287> All reviewed patches have been landed. Closing bug and clearing flags on attachment 402319 [details]. This change caused testing to exit early on Catalina due to crashes: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebKit 0x0000000104e58d95 WebKit::AuxiliaryProcess::initializeSandbox(WebKit::AuxiliaryProcessInitializationParameters const&, WebKit::SandboxInitializationParameters&) + 6783 (AuxiliaryProcessMac.mm:693) 1 com.apple.WebKit 0x00000001050fcdb0 WebKit::WebProcess::initializeSandbox(WebKit::AuxiliaryProcessInitializationParameters const&, WebKit::SandboxInitializationParameters&) + 196 (WebProcessCocoa.mm:589) 2 com.apple.WebKit 0x0000000104c8e1ff WebKit::AuxiliaryProcess::initialize(WebKit::AuxiliaryProcessInitializationParameters const&) + 103 3 com.apple.WebKit 0x000000010506cab9 void WebKit::XPCServiceInitializer<WebKit::WebProcess, WebKit::XPCServiceInitializerDelegate>(WTF::OSObjectPtr<NSObject<OS_xpc_object>*>, NSObject<OS_xpc_object>*, NSObject<OS_xpc_object>*) + 645 (XPCServiceEntryPoint.h:135) 4 com.apple.WebKit 0x000000010506c7fe WebContentServiceInitializer + 61 (WebContentServiceEntryPoint.mm:53) https://build.webkit.org/builders/Apple-Catalina-Release-WK2-Tests/builds/6485 Created attachment 402363 [details]
Catalina crash log
Reverted in https://trac.webkit.org/changeset/263307/webkit Created attachment 402397 [details]
Patch
Committed r263320: <https://trac.webkit.org/changeset/263320> All reviewed patches have been landed. Closing bug and clearing flags on attachment 402397 [details]. |