Closed Bug 1325784 Opened 6 years ago Closed 6 years ago

Remove PImageContainer


(Core :: Graphics: Layers, defect)

Not set



Tracking Status
firefox53 --- fixed


(Reporter: dvander, Assigned: dvander)




(6 files)

PImageContainer can be entirely replaced with the async ID used by compositables.
The async ID is currently tracked in CompositableChild, which I'd like to remove. This moves it to CompositableClient.
Attachment #8821766 - Flags: review?(nical.bugzilla)
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.
Attachment #8821767 - Flags: review?(nical.bugzilla)
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).
Attachment #8821768 - Flags: review?(nical.bugzilla)
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.
Attachment #8821769 - Flags: review?(nical.bugzilla)
Now all this code is unused and can go away.

Seems good on try so far.
Attachment #8821770 - Flags: review?(nical.bugzilla)
Attachment #8821766 - Flags: review?(nical.bugzilla) → review+
Attachment #8821767 - Flags: review?(nical.bugzilla) → review+
Attachment #8821768 - Flags: review?(nical.bugzilla) → review+
Attachment #8821769 - Flags: review?(nical.bugzilla) → review+
Attachment #8821770 - Flags: review?(nical.bugzilla) → review+
I'm impressed by how well managed to keep the patches separate and clean considering the amount of changes.
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.)
Attachment #8823448 - Flags: review?(nical.bugzilla)
Attachment #8823448 - Flags: review?(nical.bugzilla) → review+
Pushed by
Move asyncID out of CompositableChild and into CompositableClient. (bug 1325784 part 1, r=nical)
Move CompositableMap into ImageBridge. (bug 1325784 part 2, r=nical)
Move async compositable ID allocation to ImageBridgeChild. (bug 1325784 part 3, r=nical)
Use async compositable IDs for image composite notifications. (bug 1325784 part 4, r=nical)
Remove PImageContainer. (bug 1325784 part 5, r=nical)
Don't retain a ref to ImageContainers in ImageBridgeChild. (bug 1325784 part 6, r=nical)
Pushed by
Follow-up changes for the graphics branch. r=nical?
You need to log in before you can comment on or make changes to this bug.