Closed Bug 801719 Opened 7 years ago Closed 7 years ago

Unmark certainly alive event listener and message managers

Categories

(Core :: DOM: Core & HTML, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: smaug, Assigned: smaug)

Details

Attachments

(2 files)

Comment on attachment 671476 [details] [diff] [review]
patch

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

Uck, all this null point juggling is gross, but I guess it will do for now.

::: content/base/src/nsCCUncollectableMarker.cpp
@@ +91,5 @@
>  
>  static void
>  MarkMessageManagers()
>  {
> +  nsCOMPtr<nsIMessageBroadcaster> stronGlobalMM =

|stronGlobalMM| should be |strongGlobalMM|

@@ +147,5 @@
>    }
> +  if (nsFrameMessageManager::sParentProcessManager) {
> +    nsFrameMessageManager::sParentProcessManager->MarkForCC();
> +    uint32_t childCount = 0;
> +    nsFrameMessageManager::sParentProcessManager->GetChildCount(&childCount);

Why isn't all of this child marking stuff done normally as part of MarkForCC()?  Is it usually not worth doing?

::: content/events/src/nsEventListenerManager.h
@@ +225,5 @@
>    bool MayHaveMouseEnterLeaveEventListener() { return mMayHaveMouseEnterLeaveEventListener; }
>  
>    size_t SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const;
>  
> +  void UnmarkGrayJSListenersAndRemovePurple();

Should this become a more generic name like MarkForCC() or CleanupForCC()?
Attachment #671476 - Flags: review?(continuation) → review+
(In reply to Andrew McCreight [:mccr8] from comment #1)
> Uck, all this null point juggling is gross
indeed

> |stronGlobalMM| should be |strongGlobalMM|
One day, which is not any time this decade, I'll learn to type English.
(and I'll learn to speak English a decaded after that.)

> Why isn't all of this child marking stuff done normally as part of
> MarkForCC()?  Is it usually not worth doing?
Gets tricky when dealing with process and frame mm. I prefer keeping the hacks here.
I guess that is the only reason.

> Should this become a more generic name like MarkForCC() or CleanupForCC()?
ok, I'll change to MarkForCC
(In reply to Olli Pettay [:smaug] from comment #2)
> > |stronGlobalMM| should be |strongGlobalMM|
> One day, which is not any time this decade, I'll learn to type English.
> (and I'll learn to speak English a decaded after that.)

Your English is very good!  It is just your variable names that are occasionally slightly odd.
Attached patch patchSplinter Review
https://hg.mozilla.org/mozilla-central/rev/5a6a36d109d7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Based on telemetry this affected surprisingly positively to median
CYCLE_COLLECTOR_VISITED_GCED.

(Separate issue is CYCLE_COLLECTOR_VISITED_REF_COUNTED regression end of September.)
Target Milestone: --- → mozilla19
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.