Bug 205316 - [iOS] Issue mach lookup extension to launch services daemon for Mail
Summary: [iOS] Issue mach lookup extension to launch services daemon for Mail
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks: 207708
  Show dependency treegraph
 
Reported: 2019-12-16 16:59 PST by Per Arne Vollan
Modified: 2020-02-13 10:14 PST (History)
5 users (show)

See Also:


Attachments
Patch (5.69 KB, patch)
2019-12-16 17:16 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (14.17 KB, patch)
2019-12-19 11:48 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (16.66 KB, patch)
2020-01-04 16:17 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2019-12-16 16:59:58 PST
Issue mach lookup extension to "com.apple.lsd.open" for Mail, since this service will be removed from the WebContent sandbox.
Comment 1 Radar WebKit Bug Importer 2019-12-16 17:00:25 PST
<rdar://problem/57990991>
Comment 2 Per Arne Vollan 2019-12-16 17:16:05 PST
Created attachment 385836 [details]
Patch
Comment 3 Per Arne Vollan 2019-12-16 17:36:51 PST
I am also looking into creating a test for this.
Comment 4 Per Arne Vollan 2019-12-19 11:48:25 PST
Created attachment 386128 [details]
Patch
Comment 5 Brent Fulgham 2019-12-19 11:56:36 PST
Comment on attachment 386128 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386128&action=review

> LayoutTests/fast/sandbox/ios/sandbox-mach-lookup-mail.html:1
> +<!DOCTYPE html><!-- webkit-test-runner [ applicationBundleIdentifier=com.apple.mobilemail ] -->

Cool!
Comment 6 Per Arne Vollan 2020-01-04 16:17:40 PST
Created attachment 386772 [details]
Patch
Comment 7 Brent Fulgham 2020-01-04 18:31:43 PST
Comment on attachment 386772 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386772&action=review

R=me

> Tools/WebKitTestRunner/TestController.cpp:627
> +        // Exit if the application bundle identifier has already been set, since it can only be set once.

Could this just be a RELEASE_ASSERT?
Comment 8 Per Arne Vollan 2020-01-06 07:21:43 PST
(In reply to Brent Fulgham from comment #7)
> Comment on attachment 386772 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=386772&action=review
> 
> R=me
> 
> > Tools/WebKitTestRunner/TestController.cpp:627
> > +        // Exit if the application bundle identifier has already been set, since it can only be set once.
> 
> Could this just be a RELEASE_ASSERT?

Having a RELEASE_ASSERT when this happens, would then show up as a test crash, I think. Exiting will let the next WebKitTestRunner continue where the one that exited left off.

Thanks for reviewing!
Comment 9 WebKit Commit Bot 2020-01-06 08:03:51 PST
Comment on attachment 386772 [details]
Patch

Clearing flags on attachment: 386772

Committed r254052: <https://trac.webkit.org/changeset/254052>
Comment 10 WebKit Commit Bot 2020-01-06 08:03:53 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 mitz 2020-01-06 11:38:30 PST
Why is this being done this way? Client identity checks are necessary when the client is some existing code that’s not part of the system and can’t be updated. In this case, however, Mail can either be provided with explicit API for opting into this behavior, or it can probably just as easily restructure its code such that the interaction with lsd happens in the UI process.
Comment 12 Brent Fulgham 2020-01-06 12:15:09 PST
(In reply to mitz from comment #11)
> Why is this being done this way? Client identity checks are necessary when
> the client is some existing code that’s not part of the system and can’t be
> updated. In this case, however, Mail can either be provided with explicit
> API for opting into this behavior, or it can probably just as easily
> restructure its code such that the interaction with lsd happens in the UI
> process.

That is an excellent idea, assuming we can get time on Mail's schedule to adopt such an API.

We should proceed with this first step, and open a task to create relevant API and track adoption, at which point we could remove this internal client check.