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
Status: RESOLVED FIXED
[patch]
: 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-7 (review requests must explain patch)
:
Mentors:
http://www.w3.org/
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-01-17 13:25 PST by David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
Modified: 2005-02-12 21:27 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

Description David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-01-17 13:25:48 PST
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.
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-01-17 13:41:34 PST
Created attachment 139293 [details] [diff] [review]
patch

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

yum
Comment 3 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 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 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 Peter Van der Beken [:peterv] 2004-01-19 15:23:43 PST
Yeah sorry, I remember, I was trying const char* const.
Comment 6 Boris Zbarsky [:bz] 2004-01-20 19:01:19 PST
Comment on attachment 139293 [details] [diff] [review]
patch

r=bzbarsky, but do we still need to clear these out in Destroy, just in case?
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 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 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 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.