The default bug view has changed. See this FAQ.

Leak nsTArray_base with "backface-visibility: hidden", border-radius

RESOLVED FIXED in mozilla18

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: nrc)

Tracking

(Blocks: 1 bug, {mlk, testcase})

Trunk
mozilla18
x86_64
Mac OS X
mlk, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P3])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 637216 [details]
testcase

1. Run a debug build of Firefox with XPCOM_MEM_LEAK_LOG=2
2. Load the testcase.
3. Quit Firefox.
(Reporter)

Comment 1

5 years ago
Created attachment 637217 [details]
leak log
The MaskLayerImageKey allocated at http://hg.mozilla.org/mozilla-central/annotate/ee7a3bddfe5f/layout/base/FrameLayerBuilder.cpp#l3270 is leaked if we take the early return at http://hg.mozilla.org/mozilla-central/annotate/ee7a3bddfe5f/layout/base/FrameLayerBuilder.cpp#l3289, which this testcase triggers.

As part of this bug we should add MOZ_COUNT_CTOR/DTOR macros to MaskLayerImageKey and PixelRoundedRect.
Assignee: nobody → ncameron
Tiny leak, almost impossible to hit, not very interesting to MemShrink.
Whiteboard: [MemShrink:P3]
(Assignee)

Comment 4

5 years ago
Created attachment 657171 [details] [diff] [review]
fix
Attachment #657171 - Flags: review?(khuey)
Comment on attachment 657171 [details] [diff] [review]
fix

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

::: layout/base/FrameLayerBuilder.cpp
@@ +3274,2 @@
>    // copy and transform the rounded rects
> +  for (size_t i = 0; i < newData.mRoundedClipRects.Length(); ++i) {

Why did you switch from uint32_t to size_t?  nsTArray::Length returns a uint32_t.

::: layout/base/MaskLayerImageCache.h
@@ +114,5 @@
>      // Indices into mRadii are the NS_CORNER_* constants in nsStyleConsts.h
>      gfxFloat mRadii[8];
> +
> +  private:
> +    PixelRoundedRect() { NS_ERROR("Don't call me!"); }

No need to do this, you can make it a compile time error.

http://whereswalden.com/2011/11/09/introducing-moz_delete-a-macro-improving-upon-the-deliberately-unimplemented-method-idiom/

@@ +209,5 @@
>      MaskLayerImageEntry(const MaskLayerImageEntry& aOther)
>        : mKey(aOther.mKey.get())
>      {
>        NS_ERROR("ALLOW_MEMMOVE == true, should never be called");
>      }

You should use MOZ_DELETE here too.
Attachment #657171 - Flags: review?(khuey) → review+
(Assignee)

Comment 6

5 years ago
Try push: https://tbpl.mozilla.org/?tree=Try&rev=4ffcb9da68f4
(Assignee)

Comment 7

5 years ago
Created attachment 657787 [details] [diff] [review]
fix

Addressed khuey's comments, carrying r+.

Thanks for the pointer to MOZ_DELETE!
Attachment #657171 - Attachment is obsolete: true
Attachment #657787 - Flags: review+
(Assignee)

Comment 8

5 years ago
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=03cb77f8d75d
Push backed out for burning the tree:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4fb15293efc
(Assignee)

Comment 10

5 years ago
Try again, fixed the missing symbol (was lazy and only did incremental build, missed the linking error, sorry): https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f88af5665cf2

Thanks for the backout edmorley!
(Assignee)

Comment 11

5 years ago
backed out again: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f88af5665cf2

This one builds fine locally on Windows :-(
(Assignee)

Comment 12

5 years ago
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=61c0826e8903

(Remembered to qrefresh this time)
https://hg.mozilla.org/mozilla-central/rev/61c0826e8903
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.