Store ImageKeys and SurfaceKeys directly on ISurfaceProviders

RESOLVED FIXED in Firefox 51

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: seth, Assigned: seth)

Tracking

unspecified
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(3 attachments)

SurfaceCache cache entries are CachedSurface objects that wrap ISurfaceProviders, and it happens that ISurfaceProviders generally need to know their own ImageKey and SurfaceKey. However, CachedSurface redundantly stores this information itself. It makes sense to just get the ImageKey and SurfaceKey from the ISurfaceProvider, which will both use less memory and ensure that the two sets of values can't get out of sync.
This patch just fixes a couple of places where we were wrongly using Image* raw
pointers instead of ImageKey values. It'd probably be smart to prevent that
using the type system, but that's better left for a different bug.
Attachment #8784586 - Flags: review?(dholbert)
This patch makes ImageKeys and SurfaceKeys accessible via the ISurfaceProvider
interface. Note that things get a bit more verbose here because ISurfaceProvider
and the SurfaceCache methods are both getting passed the same values, but
that'll go away in the next part.
Attachment #8784587 - Flags: review?(edwin)
Attachment #8784587 - Flags: review?(dholbert)
Now that part 2 is done, we can start relying on the ISurfaceProvider to know
its own ImageKey and SurfaceKey within the SurfaceCache code.

We're gradually whittling away the need for CachedSurface to exist at all!
Attachment #8784588 - Flags: review?(dholbert)
Comment on attachment 8784586 [details] [diff] [review]
(Part 1) - Use ImageKey consistently in SurfaceCache.

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

r=me
Attachment #8784586 - Flags: review?(dholbert) → review+
Comment on attachment 8784588 [details] [diff] [review]
(Part 3) - Update SurfaceCache API to rely on ImageKeys and SurfaceKeys stored on ISurfaceProviders.

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

r=me
Attachment #8784588 - Flags: review?(dholbert) → review+
Comment on attachment 8784587 [details] [diff] [review]
(Part 2) - Store ImageKeys and SurfaceKeys directly on ISurfaceProviders.

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

r=me

(Also: I'm not sure it's worth having edwin review this as well; I'm happy to call this whole patch r=me, unless there's something that you'd like edwin to look at in particular.)
Attachment #8784587 - Flags: review?(dholbert) → review+
Thanks for the reviews!

(In reply to Daniel Holbert [:dholbert] from comment #7)
> (Also: I'm not sure it's worth having edwin review this as well; I'm happy
> to call this whole patch r=me, unless there's something that you'd like
> edwin to look at in particular.)

Works for me!
Pushed by seth.bugzilla@blackhail.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/39e0894d2dfe
(Part 1) - Use ImageKey consistently in SurfaceCache. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f8d3c106410
(Part 2) - Store ImageKeys and SurfaceKeys directly on ISurfaceProviders. r=dholbert,edwin
https://hg.mozilla.org/integration/mozilla-inbound/rev/d28b09df98ff
(Part 3) - Update SurfaceCache API to rely on ImageKeys and SurfaceKeys stored on ISurfaceProviders. r=dholbert
You need to log in before you can comment on or make changes to this bug.