Closed Bug 601437 Opened 11 years ago Closed 11 years ago
CSSValue::Array with append Medium
1. Run Firefox with trace-refcnt enabled. 2. Load the testcase. 3. Quit. Result: trace-refcnt reports a leak.
Leaked classes: nsBaseURLParser nsCSSValue::Array nsCSSValue::URL nsPrincipal nsSimpleURI nsStandardURL nsStringBuffer nsTArray_base
Two things going wrong here: (1) we go through the Clone path in EnsureUniqueInner (not sure why) (2) nsCSSFontFaceRule doesn't have a copy-constructor, which means the reference count of the copy is mis-initialized by the default copy-constructor
Assignee: nobody → dbaron
blocking2.0: --- → final+
I think the leak is a regression from bug 596140, so blocking+.
Maybe Boris knows why we're cloning in this case. If not, one of us should probably investigate.
Attachment #480487 - Flags: review?(bzbarsky)
> (2) nsCSSFontFaceRule doesn't have a copy-constructor, which means the > reference count of the copy is mis-initialized by the default copy-constructor Eep! Filed bug 601441 asking for a static analysis to catch bugs like this.
We're cloning because of this: #4 0x000000010047bb5a in mozilla::css::Loader::LoadSheet (this=0x1253ab700, aURL=0x1244d0d10, aOriginPrincipal=0x1253ab840, aCharset=@0x7fff5fbfc210, aObserver=0x1244d0d70) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/style/Loader.cpp:2135 #5 0x0000000100635777 in nsDocument::PreloadStyle (this=0x106083200, uri=0x1244d0d10, charset=@0x11eb09150) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsDocument.cpp:7893 #6 0x0000000100b133c4 in nsHtml5TreeOpExecutor::PreloadStyle (this=0x1244dbf20, aURL=@0x11eb09140, aCharset=@0x11eb09150) at /Users/bzbarsky/mozilla/vanilla/mozilla/parser/html/nsHtml5TreeOpExecutor.cpp:887 Henri, why are we preloading here??
Comment on attachment 480487 [details] [diff] [review] patch r=me
Attachment #480487 - Flags: review?(bzbarsky) → review+
(In reply to comment #6) > Henri, why are we preloading here?? <link rel=stylesheet> gets preloaded unconditionally. data: URLs aren't special cased in any way in the preloader. Also, everything the preloader can preload gets preloaded even if there haven't been any scripts. (I wasn't aware of this causing any problems. I thought it was cheap to promote a preload into a real load later.)
It's reasonably cheap; does involve constructing a few more objects, and slows down the first access to the stylesheet data from script a bit. I had thought we only preloaded when blocked; if we preload everything always that explains it.
Are we doing an extra *parse* of the markup in order to preload, even when parsing isn't blocked?
No, we preload off the same token stream that we create the DOM from. It's just that when blocked we continue speculatively creating the token stream and throw it away in the (rare) case that the script we blocked on invalidates it.
Status: NEW → RESOLVED
Closed: 11 years ago
OS: Linux → All
Priority: -- → P3
Hardware: x86_64 → All
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.