The default bug view has changed. See this FAQ.

BBP for ContentList

RESOLVED FIXED in mozilla13

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

(Blocks: 1 bug)

13 Branch
mozilla13
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 595490 [details] [diff] [review]
WIP
Blocks: 716598
Attachment #595490 - Flags: review?(jst)
Attachment #595490 - Flags: feedback?(continuation)
Comment on attachment 595490 [details] [diff] [review]
WIP

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

Looks good to me, aside from my one comment.

::: content/base/src/nsContentList.cpp
@@ +93,5 @@
> +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_BEGIN(nsBaseContentList)
> +  if (nsCCUncollectableMarker::sGeneration && tmp->IsBlack()) {
> +    JSObject* o = tmp->PreservingWrapper() ? 
> +      tmp->GetWrapperPreserveColor() : tmp->GetExpandoObjectPreserveColor();
> +    xpc_UnmarkGrayObject(o);

Didn't we just check that our wrapper is black?  Isn't o going to always be non-gray here?
Attachment #595490 - Flags: feedback?(continuation) → feedback+
Yes. silly me. I'll change the code to unmark only if !tmp->PreservingWrapper()
Er, except that is not needed.
Because if the list is owned by a black wrapper, it won't also have a separate expando object that it owns?
Well, it is the black proxy object which has the expando. And that should be be black too.
(Need to get some documentation all this preservedwrapper/proxy/expando handling)
Created attachment 596109 [details] [diff] [review]
patch
Assignee: nobody → bugs
Attachment #595490 - Attachment is obsolete: true
Attachment #595490 - Flags: review?(jst)
Attachment #596109 - Flags: review?(jst)
(In reply to Olli Pettay [:smaug] from comment #5)
> Well, it is the black proxy object which has the expando. And that should be
> be black too.
Since if something marks the proxy black, its reserved slots get marked too.

Updated

5 years ago
Attachment #596109 - Flags: review?(jst) → review+
https://hg.mozilla.org/mozilla-central/rev/445e1040bfcf
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Version: 12 Branch → 13 Branch
You need to log in before you can comment on or make changes to this bug.