1341001 - MacOS build crash after merge from m-c

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ethan Lin[:ethlin]]

After bug 1325227, ContentClientRemoteBuffer uses CrossProcessSemaphore to do the Readlock for texture, but there is no MacOS version CrossProcessSemaphore. For m-c, ClientPaintedLayer uses TiledContentClient on MacOS, for graphics, there is only ContentClientRemoteBuffer for now.

Comment 1

[Ethan Lin[:ethlin]]

Matt, should I implement a MacOS CrossProcessSemaphore? It looks like MacOS doesn't support some semaphore functions.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Yeah, if WR is going to use ContentClientRemote, then we probably need to.

I know that OSX doesn't support sem_timedwait using POSIX semaphores, but you could try mach semaphores, or the Grand Central Dispatch ones.

The other alternative is to support spin waiting using the other ReadLock impls (implement TryReadLock by doing a loop of GetReadCount and sleep until the timeout passes or GetReadCount == 0).

Comment 3

[Ethan Lin[:ethlin]]

Attachment: replace ContentClient with ImageClient in WebRenderPaintedLayer

It's easier to fix the problem by using ImageClient. And we won't use ContentClient in the future.

Comment 4

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8839403 [details] [diff] [review]
replace ContentClient with ImageClient in WebRenderPaintedLayer

Review of attachment 8839403 [details] [diff] [review]:
-----------------------------------------------------------------

Review+, if a comment is applied. It might better to check reftest result.

::: gfx/layers/wr/WebRenderBridgeParent.cpp
@@ +356,5 @@
>          keysToDelete.push_back(key);
>          dSurf->Unmap();
>          // XXX workaround for releasing Readlock. See Bug 1339625
> +        if(host->GetType() == CompositableType::CONTENT_SINGLE ||
> +           host->GetType() == CompositableType::CONTENT_DOUBLE) {

This change is not necessary.

Comment 5

[Ethan Lin[:ethlin]]

(In reply to Sotaro Ikeda [:sotaro] from comment #4)
> Comment on attachment 8839403 [details] [diff] [review]
> replace ContentClient with ImageClient in WebRenderPaintedLayer
> 
> Review of attachment 8839403 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Review+, if a comment is applied. It might better to check reftest result.
> 
> ::: gfx/layers/wr/WebRenderBridgeParent.cpp
> @@ +356,5 @@
> >          keysToDelete.push_back(key);
> >          dSurf->Unmap();
> >          // XXX workaround for releasing Readlock. See Bug 1339625
> > +        if(host->GetType() == CompositableType::CONTENT_SINGLE ||
> > +           host->GetType() == CompositableType::CONTENT_DOUBLE) {
> 
> This change is not necessary.

okay, I'll remove it.

Based on [1] result, there is a failure 'layout/reftests/bugs/942672-1.html' with the patch. I'll mark it as 'fails-if'. 

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c9c7b65ab07a8d2c4f9d98cbd1d35ab304ff5e0&selectedJob=79001435

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/99999c5e6994
Using ImageClient for WebRenderPaintedLayer to prevent crash problem. r=sotaro

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/99999c5e6994