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

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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-4, no longer working on Mozilla)]

Attachment: patch

I can't reproduce the leak with this patch.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

yum

Comment 3

[Peter Van der Beken [:peterv]]

>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-4, no longer working on Mozilla)]

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]]

Yeah sorry, I remember, I was trying const char* const.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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-4, no longer working on Mozilla)]

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-4, no longer working on Mozilla)]

Fix checked in to trunk, 2004-01-20 20:15 -0800.