Closed Bug 1325784 Opened 4 years ago Closed 4 years ago

Remove PImageContainer

Categories

(Core :: Graphics: Layers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: dvander, Assigned: dvander)

References

Details

Attachments

(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 danderson@mozilla.com:
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)
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/d7fcda4f0cdb
Follow-up changes for the graphics branch. r=nical?
Depends on: 1329432
You need to log in before you can comment on or make changes to this bug.