786412 - Consolidate ContainerLayer functions

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Chris Lord [:cwiiis]]

The code in the different implementations of ContainerLayer for InsertAfter and RemoveChild is actually identical (or at least, it will be after bug 785333) - we should consolidate this instead of copy-pasting.

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Move child add/remove functions to ContainerLayer

Try looks good and it's a nice reduction.

I wonder if we can do anything with ComputeEffectiveTransform?

Comment 2

[Chris Lord [:cwiiis]]

I guess this may be obsoleted by nrc's shadow layers work?

Will hold off on doing anything else - just a note to self, is it ok that BasicShadowableRefLayer uses ContainerComputeEffectiveTransforms instead of DefaultComputeEffectiveTransforms(?)

Comment 3

[Bas Schouten (:bas.schouten)]

Comment on attachment 662136 [details] [diff] [review]
Move child add/remove functions to ContainerLayer

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

This patch is obsoleted I think because of Nick's work, although it's certainly a good idea in general.

Comment 4

[Nick Cameron [:nrc]]

We should totally do this after the refactoring lands, I do something similar, so it should be easier then too.

Comment 5

[Chris Lord [:cwiiis]]

Nick, can this bug be closed, or can I assign it to you?

Comment 6

[Nick Cameron [:nrc]]

assign it to me is fine, I'll check how this tands in the refactoring and apply your patch if we can.

Comment 7

[Chris Lord [:cwiiis]]

Cool, thanks :)

Comment 8

[Nick Cameron [:nrc]]

So, I think I'll leave this until after the refactoring lands. I'm pretty sure it will still work, but it will be a little bit of effort to adjust for composite container layers.

Comment 9

[Nick Cameron [:nrc]]

Attachment: Sort out container layer methods

Comment 10

[Nick Cameron [:nrc]]

(In reply to Nick Cameron [:nrc] from comment #8)
> So, I think I'll leave this until after the refactoring lands. I'm pretty
> sure it will still work, but it will be a little bit of effort to adjust for
> composite container layers.

Finally got to this. Removed quite a satisfying amount of copy pasta.

Comment 11

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 796473 [details] [diff] [review]
Sort out container layer methods

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

rs=me assuming it passes tests :)

Comment 12

[Nick Cameron [:nrc]]

Try looks good: https://tbpl.mozilla.org/?tree=Try&rev=9b638460e95a

Comment 13

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/45668135bbf4

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/45668135bbf4