The default bug view has changed. See this FAQ.

leaks on stop due to PresShell's queue of DOM events not being processed

RESOLVED FIXED in mozilla1.7alpha

Status

()

Core
Layout: Misc Code
P1
normal
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({mlk})

Trunk
mozilla1.7alpha
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch], URL)

Attachments

(1 attachment)

(Assignee)

Description

13 years ago
If I start loading www.w3.org (from X-Remote with new-tab, actually), and then
quickly stop it, I leak a good bit of stuff.  I think the root, or one of them,
at least, is that the pres shell's queues stored in the variables:

  // used for list of posted events and attribute changes. To be done
  // after reflow.
  nsDOMEventRequest* mFirstDOMEventRequest;
  nsDOMEventRequest* mLastDOMEventRequest;
  nsAttributeChangeRequest* mFirstAttributeRequest;
  nsAttributeChangeRequest* mLastAttributeRequest;
  nsCallbackEventRequest* mFirstCallbackEventRequest;
  nsCallbackEventRequest* mLastCallbackEventRequest;

are not guaranteed to be processed.  (In this case, it's the first one.)  These
request variables own references to content.  They need to be freed in the pres
shell's Destroy method.
(Assignee)

Comment 1

13 years ago
Created attachment 139293 [details] [diff] [review]
patch

I can't reproduce the leak with this patch.
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [patch]
Target Milestone: --- → mozilla1.7alpha
(Assignee)

Updated

13 years ago
Attachment #139293 - Flags: superreview?(roc)
Attachment #139293 - Flags: review?(bz-vacation)
Comment on attachment 139293 [details] [diff] [review]
patch

yum
Attachment #139293 - Flags: superreview?(roc) → superreview+
>Index: nsPresShell.cpp
>===================================================================

> struct VerifyReflowFlags {
>-  char*    name;
>+  const char*    name;
>   PRUint32 bit;
> };
> 
>-static VerifyReflowFlags gFlags[] = {
>+static const VerifyReflowFlags gFlags[] = {
>   { "verify",                VERIFY_REFLOW_ON },

Just fyi: I recently tried something similar and Visual C++ complained with an
error. Basically, I couldn't get it to compile

struct whatever {
    const int i;
};
const whatever foo = { 0 };

Removing the const from i worked, so it could be that you have to undo the
constifying of name.
(Assignee)

Comment 4

13 years ago
The analogy to |const int| for a char pointer is |char * const|, not |const
char*|, so I'm not worried.  (Furthermore, I've done this in many other places.)
Yeah sorry, I remember, I was trying const char* const.
Comment on attachment 139293 [details] [diff] [review]
patch

r=bzbarsky, but do we still need to clear these out in Destroy, just in case?
Attachment #139293 - Flags: review?(bz-vacation) → review+
(Assignee)

Comment 7

13 years ago
We shouldn't, since they should only be added to during reflow.  But I added an
assertion in the pres shell's destructor just in case.
(Assignee)

Comment 8

13 years ago
Fix checked in to trunk, 2004-01-20 20:15 -0800.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.