Implement a destruction handshake in PCompositable similar to PTexture's to avoid races between messages and the protocol

RESOLVED FIXED in mozilla45

Status

()

Core
Graphics: Layers
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: nical, Assigned: nical)

Tracking

unspecified
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8680649 [details] [diff] [review]
Move the deallocation handshake in a helper class and make PTexture and PCompositable use it.

This should fix the potential races between messages using a PCompositable actor and the actor's destruction.
Assignee: nobody → nical.bugzilla
Attachment #8680649 - Flags: review?(sotaro.ikeda.g)
Comment on attachment 8680649 [details] [diff] [review]
Move the deallocation handshake in a helper class and make PTexture and PCompositable use it.

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

Nice! Looks good.

::: gfx/layers/IPDLActor.h
@@ +96,5 @@
> +  ~ParentActor() { MOZ_ASSERT(mDestroyed); }
> +
> +  bool CanSend() const { return !mDestroyed; }
> +
> +  // Override this rather than ActorDestroy 

nits. Remove space at end of the line.

::: gfx/layers/client/CompositableClient.cpp
@@ +159,5 @@
>  
>  bool
>  CompositableClient::Connect(ImageContainer* aImageContainer)
>  {
> +  MOZ_ASSERT(!IsConnected());

Isn't it better just checking MOZ_ASSERT(!mCompositableChild)?

::: gfx/layers/client/TiledContentClient.h
@@ +647,5 @@
>  protected:
>    ~MultiTiledContentClient()
>    {
>      MOZ_COUNT_DTOR(MultiTiledContentClient);
>   

nits. Remove redundant space.

::: gfx/layers/ipc/PCompositable.ipdl
@@ +30,2 @@
>  };
>      

nits. Remove redundant spaces.
Attachment #8680649 - Flags: review?(sotaro.ikeda.g) → review+

Updated

3 years ago
Blocks: 1219529

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45

Updated

2 years ago
Depends on: 1261321
You need to log in before you can comment on or make changes to this bug.