1118105 - SurfaceCache should let you know if you tried to insert a duplicate surface

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

In bug 1079627, we need to be able to detect when we're inserting a duplicate surface in the SurfaceCache.

In this bug we'll make SurfaceCache::Insert return an enumeration instead of a bool, and change the implementation of Insert to inform us when we encounter this situation.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: Make SurfaceCache::Insert let you know if you try to insert a duplicate surface

Here's the patch. It's pretty straightforward.

Comment 2

[Daniel Holbert [:dholbert]]

Comment on attachment 8544333 [details] [diff] [review]
Make SurfaceCache::Insert let you know if you try to insert a duplicate surface

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

r=me, with one nit

::: image/src/SurfaceCache.h
@@ +117,5 @@
> +MOZ_BEGIN_ENUM_CLASS(InsertOutcome, uint8_t)
> +  SUCCESS,                 // Success (but see Insert documentation).
> +  FAILURE,                 // Couldn't insert (e.g., for capacity reasons).
> +  FAILURE_ALREADY_PRESENT  // A surface with the same key is already present.
> +MOZ_END_ENUM_CLASS(Opacity)

s/Opacity/InsertOutcome/

Comment 3

[Seth Fowler [:seth] [:s2h]]

Attachment: Make SurfaceCache::Insert let you know if you try to insert a duplicate surface

Thanks for the review! Here's an updated version that fixes the nit above.

Comment 4

[Seth Fowler [:seth] [:s2h]]

This got blocked for a bit on a patch earlier in my queue, but it should be ready to go now. Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5c63ad85aa6

Comment 5

[Carsten Book [:Tomcat]]

sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b74c9afa3b4b since one of this changes caused test failures on windows like:

https://treeherder.mozilla.org/logviewer.html#?job_id=5263429&repo=mozilla-inbound
and
https://treeherder.mozilla.org/logviewer.html#?job_id=5263747&repo=mozilla-inbound

Comment 6

[Seth Fowler [:seth] [:s2h]]

Repushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/81d28669e41e

Comment 7

[Tom S [:evilpie]]

Backout
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f3cc2c90893

Comment 8

[Seth Fowler [:seth] [:s2h]]

Looks green on this try job:

https://tbpl.mozilla.org/?tree=Try&rev=aae64e1b4638

Repushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fe67aa7e9ac

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0fe67aa7e9ac