Bug 205443 - [iOS] Issue mach sandbox extension to the frontboard and icon service when the attachment element is enabled
Summary: [iOS] Issue mach sandbox extension to the frontboard and icon service when th...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-19 02:25 PST by Per Arne Vollan
Modified: 2020-02-06 10:02 PST (History)
9 users (show)

See Also:


Attachments
Patch (8.02 KB, patch)
2019-12-19 02:35 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (8.30 KB, patch)
2019-12-19 07:10 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (19.49 KB, patch)
2020-01-16 11:42 PST, Per Arne Vollan
bfulgham: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Patch (19.50 KB, patch)
2020-01-30 10:19 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-19 02:25:37 PST
When support for the html attachment element is enabled, issue a mach lookup extension to the frontboard service for the WebContent process, since this daemon is being contacted when icons for attachments are being queried.
Comment 1 Per Arne Vollan 2019-12-19 02:35:35 PST
Created attachment 386090 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2019-12-19 02:36:15 PST
<rdar://problem/58074291>
Comment 3 Per Arne Vollan 2019-12-19 07:10:24 PST
Created attachment 386099 [details]
Patch
Comment 4 Brent Fulgham 2020-01-16 11:08:16 PST
Comment on attachment 386099 [details]
Patch

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

r=me

> Source/WebKit/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=205443

Please add the radar

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:345
> +        SandboxExtension::createHandleForMachLookup("com.apple.frontboard.systemappservices", WTF::nullopt, handle);

Do we have a similar situation on macOS (using a different mach-port)? We should file a radar to investigate and address that, too.
Comment 5 Per Arne Vollan 2020-01-16 11:42:12 PST
Created attachment 387940 [details]
Patch
Comment 6 Per Arne Vollan 2020-01-16 11:44:04 PST
(In reply to Brent Fulgham from comment #4)
> Comment on attachment 386099 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=386099&action=review
> 
> r=me
> 
> > Source/WebKit/ChangeLog:4
> > +        https://bugs.webkit.org/show_bug.cgi?id=205443
> 
> Please add the radar
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:345
> > +        SandboxExtension::createHandleForMachLookup("com.apple.frontboard.systemappservices", WTF::nullopt, handle);
> 
> Do we have a similar situation on macOS (using a different mach-port)? We
> should file a radar to investigate and address that, too.

I don't know which port this would be on macOS, but I think it is likely there is a counterpart. I will file a radar.

I uploaded another patch with an additional fix for getting the focus ring color in the UI process.

Thanks for reviewing!
Comment 7 Brent Fulgham 2020-01-16 11:56:09 PST
Comment on attachment 387940 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/WebPageProxy.cpp:7614
> +        SandboxExtension::createHandleForMachLookup("com.apple.iconservices", WTF::nullopt, handle);

Should we handle icon services under a different radar? We probably need this on macOS, too, so it would be nice to do both together?
Comment 8 Brent Fulgham 2020-01-16 11:56:53 PST
(In reply to Brent Fulgham from comment #7)
> Comment on attachment 387940 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387940&action=review
> 
> r=me
> 
> > Source/WebKit/UIProcess/WebPageProxy.cpp:7614
> > +        SandboxExtension::createHandleForMachLookup("com.apple.iconservices", WTF::nullopt, handle);
> 
> Should we handle icon services under a different radar? We probably need
> this on macOS, too, so it would be nice to do both together?

I spoke with Per Arne offline. We should proceed with this patch as-is, and do a similar patch on macOS after validating if any other changes are needed there.
Comment 9 Per Arne Vollan 2020-01-16 13:07:37 PST
(In reply to Brent Fulgham from comment #8)
> (In reply to Brent Fulgham from comment #7)
> > Comment on attachment 387940 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=387940&action=review
> > 
> > r=me
> > 
> > > Source/WebKit/UIProcess/WebPageProxy.cpp:7614
> > > +        SandboxExtension::createHandleForMachLookup("com.apple.iconservices", WTF::nullopt, handle);
> > 
> > Should we handle icon services under a different radar? We probably need
> > this on macOS, too, so it would be nice to do both together?
> 
> I spoke with Per Arne offline. We should proceed with this patch as-is, and
> do a similar patch on macOS after validating if any other changes are needed
> there.

Thanks for reviewing!
Comment 10 WebKit Commit Bot 2020-01-30 09:56:51 PST
Comment on attachment 387940 [details]
Patch

Rejecting attachment 387940 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 387940, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=387940&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=205443&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 387940 from bug 205443.
Fetching: https://bugs.webkit.org/attachment.cgi?id=387940
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Brent Fulgham']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 23 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/rendering/RenderTheme.h
patching file Source/WebCore/rendering/RenderThemeIOS.h
patching file Source/WebCore/rendering/RenderThemeIOS.mm
patching file Source/WebCore/testing/Internals.cpp
Hunk #1 succeeded at 5455 with fuzz 1 (offset 20 lines).
patching file Source/WebCore/testing/Internals.h
Hunk #1 succeeded at 933 with fuzz 2 (offset 7 lines).
patching file Source/WebCore/testing/Internals.idl
Hunk #1 FAILED at 833.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/testing/Internals.idl.rej
patching file Source/WebKit/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb
Hunk #1 FAILED at 955.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb.rej
patching file Source/WebKit/Shared/WebPageCreationParameters.cpp
Hunk #1 succeeded at 92 with fuzz 2 (offset -7 lines).
Hunk #2 succeeded at 301 with fuzz 1 (offset -9 lines).
patching file Source/WebKit/Shared/WebPageCreationParameters.h
Hunk #1 succeeded at 152 with fuzz 2 (offset -8 lines).
patching file Source/WebKit/Shared/WebProcessCreationParameters.cpp
Hunk #1 succeeded at 173 (offset 2 lines).
Hunk #2 succeeded at 459 (offset 14 lines).
patching file Source/WebKit/Shared/WebProcessCreationParameters.h
Hunk #1 succeeded at 215 (offset 2 lines).
patching file Source/WebKit/UIProcess/WebPageProxy.cpp
Hunk #1 succeeded at 7651 with fuzz 2 (offset 45 lines).
patching file Source/WebKit/UIProcess/WebProcessProxy.h
Hunk #1 succeeded at 354 (offset 7 lines).
Hunk #2 succeeded at 537 (offset 10 lines).
patching file Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm
Hunk #1 succeeded at 360 (offset 6 lines).
patching file Source/WebKit/WebProcess/WebPage/WebPage.cpp
Hunk #1 succeeded at 526 (offset 2 lines).
patching file Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm
Hunk #1 succeeded at 267 (offset 5 lines).
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/sandbox/ios/focus-ring-color-expected.txt
patching file LayoutTests/fast/sandbox/ios/focus-ring-color.html
patching file LayoutTests/fast/sandbox/ios/sandbox-mach-lookup-attachment-element-expected.txt
patching file LayoutTests/fast/sandbox/ios/sandbox-mach-lookup-attachment-element.html

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Brent Fulgham']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/13314023
Comment 11 Per Arne Vollan 2020-01-30 10:19:47 PST
Created attachment 389265 [details]
Patch
Comment 12 WebKit Commit Bot 2020-01-30 12:39:45 PST
The commit-queue encountered the following flaky tests while processing attachment 389265 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 13 WebKit Commit Bot 2020-01-30 12:40:07 PST
The commit-queue encountered the following flaky tests while processing attachment 389265 [details]:

editing/spelling/spellcheck-attribute.html bug 206178 (authors: g.czajkowski@samsung.com, mark.lam@apple.com, and rniwa@webkit.org)
The commit-queue is continuing to process your patch.
Comment 14 WebKit Commit Bot 2020-01-30 14:13:28 PST
Comment on attachment 389265 [details]
Patch

Clearing flags on attachment: 389265

Committed r255457: <https://trac.webkit.org/changeset/255457>