Last Comment Bug 231278 - leaks on stop due to PresShell's queue of DOM events not being processed
: leaks on stop due to PresShell's queue of DOM events not being processed
: mlk
Product: Core
Classification: Components
Component: Layout: Misc Code (show other bugs)
: Trunk
: x86 Linux
P1 normal (vote)
: mozilla1.7alpha
Assigned To: David Baron :dbaron: ⌚️UTC-8
: Jet Villegas (:jet)
Depends on:
  Show dependency treegraph
Reported: 2004-01-17 13:25 PST by David Baron :dbaron: ⌚️UTC-8
Modified: 2005-02-12 21:27 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (6.14 KB, patch)
2004-01-17 13:41 PST, David Baron :dbaron: ⌚️UTC-8
bzbarsky: review+
roc: superreview+
Details | Diff | Splinter Review

Description User image David Baron :dbaron: ⌚️UTC-8 2004-01-17 13:25:48 PST
If I start loading (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.
Comment 1 User image David Baron :dbaron: ⌚️UTC-8 2004-01-17 13:41:34 PST
Created attachment 139293 [details] [diff] [review]

I can't reproduce the leak with this patch.
Comment 2 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2004-01-18 10:45:05 PST
Comment on attachment 139293 [details] [diff] [review]

Comment 3 User image Peter Van der Beken [:peterv] 2004-01-19 14:44:56 PST
>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.
Comment 4 User image David Baron :dbaron: ⌚️UTC-8 2004-01-19 15:00:02 PST
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.)
Comment 5 User image Peter Van der Beken [:peterv] 2004-01-19 15:23:43 PST
Yeah sorry, I remember, I was trying const char* const.
Comment 6 User image Boris Zbarsky [:bz] (still a bit busy) 2004-01-20 19:01:19 PST
Comment on attachment 139293 [details] [diff] [review]

r=bzbarsky, but do we still need to clear these out in Destroy, just in case?
Comment 7 User image David Baron :dbaron: ⌚️UTC-8 2004-01-20 20:13:18 PST
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.
Comment 8 User image David Baron :dbaron: ⌚️UTC-8 2004-01-20 20:15:22 PST
Fix checked in to trunk, 2004-01-20 20:15 -0800.

Note You need to log in before you can comment on or make changes to this bug.