Bug 227103 – Clear state after process crash so that we know to re-issue sandbox extensions

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brent Fulgham]

We dynamically extend access to certain mach services when the ATTACHMENT_ELEMENT feature is enabled. If a process with this feature activated crashes and WebKit relaunches it, we don't grant the new process the same extensions because we believe they were already previously sent.

This patch clears the state after a process crash so that it properly handles the restart case.

[Brent Fulgham]

<rdar://problem/77588760>

[Brent Fulgham]

Created attachment 431624 [details]
Patch

[Per Arne Vollan]

Comment on attachment 431624 [details]
Patch

R=me.

[EWS]

Committed r279014 (238939@main): <https://commits.webkit.org/238939@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431624 [details].

[Brent Fulgham]

Reopening to attach new patch.

[Brent Fulgham]

Created attachment 431745 [details]
Patch

[Tim Horton]

Comment on attachment 431745 [details]
Patch

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

> Source/WebKit/UIProcess/WebPageProxy.cpp:8201
> -    if (m_preferences->attachmentElementEnabled() && !m_process->hasIssuedAttachmentElementRelatedSandboxExtensions()) {
> +    if (m_preferences->attachmentElementEnabled() && !process.hasIssuedAttachmentElementRelatedSandboxExtensions()) {

There's other code that uses m_process in this function... is it wrong too??

[Brent Fulgham]

(In reply to Tim Horton from comment #7)
> Comment on attachment 431745 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=431745&action=review
> 
> > Source/WebKit/UIProcess/WebPageProxy.cpp:8201
> > -    if (m_preferences->attachmentElementEnabled() && !m_process->hasIssuedAttachmentElementRelatedSandboxExtensions()) {
> > +    if (m_preferences->attachmentElementEnabled() && !process.hasIssuedAttachmentElementRelatedSandboxExtensions()) {
> 
> There's other code that uses m_process in this function... is it wrong too??

Probably? I’ll take a look….

[Brent Fulgham]

Created attachment 431784 [details]
Patch for landing

[Brent Fulgham]

(In reply to Brent Fulgham from comment #8)
> (In reply to Tim Horton from comment #7)
> > Comment on attachment 431745 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=431745&action=review
> > 
> > > Source/WebKit/UIProcess/WebPageProxy.cpp:8201
> > > -    if (m_preferences->attachmentElementEnabled() && !m_process->hasIssuedAttachmentElementRelatedSandboxExtensions()) {
> > > +    if (m_preferences->attachmentElementEnabled() && !process.hasIssuedAttachmentElementRelatedSandboxExtensions()) {
> > 
> > There's other code that uses m_process in this function... is it wrong too??
> 
> Probably? I’ll take a look….

The only other instance I saw was this one:

    parameters.shouldCaptureDisplayInUIProcess = m_process->processPool().configuration().shouldCaptureDisplayInUIProcess();

I didn't change it because I wanted to check with GPU Process people. I suspect its fine as-is, since they are likely using the same process pool. I wasn't sure if the state about capturing display in the UI Process would be correct in the new process yet, either.

[EWS]

Committed r279045 (238965@main): <https://commits.webkit.org/238965@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431784 [details].