1325784 - Remove PImageContainer

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

PImageContainer can be entirely replaced with the async ID used by compositables.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 1, move asyncID to CompositableClient

The async ID is currently tracked in CompositableChild, which I'd like to remove. This moves it to CompositableClient.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 2, move CompositableMap to ImageBridge

Move CompositableMap to ImageBridgeParent. This will make it easier for ImageBridgeParent to work with IDs directly. It also fixes a long-standing but innocuous bug that a compromised content process could attach a compositable from a different process.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 3, move async ID allocation

This moves async ID allocation out of the compositor and into client layers. Doing so lets us drop the synchronous requirement of PCompositable (and later PCompositable altogether).

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 4, bypass PImageContainer

PImageContainer is only used for ImageCompositeNotifications. This patch removes that use case by mapping async IDs to ImageContainers on the ImageBridgeChild side. This means we only need to forward the async ID in the composite notifications.

I had to re-add the async ID to AsyncCompositableChild, so we could unmap the ID in DeallocPCompositableChild. I also had to add a process ID to the ImageCompositeNotification buffer, since otherwise there is no way to figure out what ImageBridge to use. Otherwise this was pretty straightforward.

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 5, rm PImageContainer

Now all this code is unused and can go away.

Seems good on try so far.

Comment 6

[Nicolas Silva [:nical]]

I'm impressed by how well managed to keep the patches separate and clean considering the amount of changes.

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 6, fix memory leak

Retaining a reference to ImageContainers in ImageBridgeChild was a little shortsighted. The ImageContainer destructor will never run, and therefore the ImageClient and ID will never be released.

This patch drops the RefPtr in ImageBridgeChild's ImageContainer map, and then has ImageContainer's destructor clean up any dangling pointers. (Similar to how it worked before.)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/41e358afd351
Move asyncID out of CompositableChild and into CompositableClient. (bug 1325784 part 1, r=nical)
https://hg.mozilla.org/integration/mozilla-inbound/rev/db794b9c38c0
Move CompositableMap into ImageBridge. (bug 1325784 part 2, r=nical)
https://hg.mozilla.org/integration/mozilla-inbound/rev/f27059c293c8
Move async compositable ID allocation to ImageBridgeChild. (bug 1325784 part 3, r=nical)
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed511c350d9d
Use async compositable IDs for image composite notifications. (bug 1325784 part 4, r=nical)
https://hg.mozilla.org/integration/mozilla-inbound/rev/38cde09a9df3
Remove PImageContainer. (bug 1325784 part 5, r=nical)
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a281e6c7c56
Don't retain a ref to ImageContainers in ImageBridgeChild. (bug 1325784 part 6, r=nical)

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/41e358afd351
https://hg.mozilla.org/mozilla-central/rev/db794b9c38c0
https://hg.mozilla.org/mozilla-central/rev/f27059c293c8
https://hg.mozilla.org/mozilla-central/rev/ed511c350d9d
https://hg.mozilla.org/mozilla-central/rev/38cde09a9df3
https://hg.mozilla.org/mozilla-central/rev/8a281e6c7c56

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/d7fcda4f0cdb
Follow-up changes for the graphics branch. r=nical?