GPU process remains alive even after UI process exited

• Component: WebKit2
• Status: RESOLVED FIXED

[Fujii Hironori]

[WinCairo?] GPU process remains alive even after web process exited

GPUProcess overrides AuxiliaryProcess::didClose but it does nothing.
Is this a WinCairo specific issue?

[Fujii Hironori]

Created attachment 414969 [details]
Patch

[Fujii Hironori]

Committed r270213: <https://trac.webkit.org/changeset/270213>

[Radar WebKit Bug Importer]

<rdar://problem/71770347>

[Fujii Hironori]

Landed in r270210, and reverted in r270213.
It broke TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo on iOS.

[Fujii Hironori]

Reopened.

[Wenson Hsieh]

Hmm...would this mean that if there are two web content processes (and therefore two connections) and one of the web processes die, then the GPU process will also terminate? If so, I think we'd still need the GPU process to stay alive in this scenario, since it's still required to service the other web content process that is still alive.

[Tim Horton]

(In reply to Wenson Hsieh from comment #6)
> Hmm...would this mean that if there are two web content processes (and
> therefore two connections) and one of the web processes die, then the GPU
> process will also terminate? If so, I think we'd still need the GPU process
> to stay alive in this scenario, since it's still required to service the
> other web content process that is still alive.

No, the connection this is referring to /should/ be the UI->GPU connection, right?

[Wenson Hsieh]

(In reply to Tim Horton from comment #7)
> (In reply to Wenson Hsieh from comment #6)
> > Hmm...would this mean that if there are two web content processes (and
> > therefore two connections) and one of the web processes die, then the GPU
> > process will also terminate? If so, I think we'd still need the GPU process
> > to stay alive in this scenario, since it's still required to service the
> > other web content process that is still alive.
> 
> No, the connection this is referring to /should/ be the UI->GPU connection,
> right?

Ah, if that's the case then this makes a lot more sense. I was unclear on which process the `IPC::Connection` was referring to.

I suppose this doesn't affect Cocoa platforms because the GPU process is a child process of the UI (application) process, so if the app dies then the GPU process goes along with it.

[Fujii Hironori]

Thank you for clarifying. I didn't understand GPU process model.

I confirmed GPUConnectionToWebProcess::didClose is called when Web process exits, and GPUProcess::didClose is called when UI process exits on WinCairo.

Why does TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo fail on iOS with my patch?

[Tim Horton]

(In reply to Fujii Hironori from comment #9)
> Thank you for clarifying. I didn't understand GPU process model.
> 
> I confirmed GPUConnectionToWebProcess::didClose is called when Web process
> exits, and GPUProcess::didClose is called when UI process exits on WinCairo.
> 
> Why does TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo fail on iOS with my
> patch?

Do you have a link to the test output? Also, +cdumez

[Fujii Hironori]

(In reply to Tim Horton from comment #10)
> Do you have a link to the test output? Also, +cdumez

I'm sorry. It did misunderstand. TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo is flaky without my patch.

https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo&platform=ios

I'm going to land it again.

[Chris Dumez]

(In reply to Fujii Hironori from comment #11)
> (In reply to Tim Horton from comment #10)
> > Do you have a link to the test output? Also, +cdumez
> 
> I'm sorry. It did misunderstand.
> TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo is flaky without my patch.
> 
> https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.GPUProcess.
> CrashWhilePlayingVideo&platform=ios
> 
> I'm going to land it again.

Yes, I noticed GPUProcess.CrashWhilePlayingVideo is flaky on the iOS bots too. I have not been able to reproduce this flakiness though.

[Fujii Hironori]

Committed r270279: <https://trac.webkit.org/changeset/270279>

[Darin Adler]

For future reference: A bug like this should *not* have the [WinCairo?] prefix.

Platform-independent fix to cross-platform issue!

[Fujii Hironori]

I got it.