Fix for crash when assigning document.body

[Maciej Stachowiak]

On Oct 3, 2003, at 3:19 PM, Dirk Mueller wrote:

> On Friday 03 October 2003 22:37, Maciej Stachowiak wrote:
>
>> Second, the HTML parser needs to keep the "current" node ref'd, since
>> script execution can remove arbitrary nodes from the document while
>> parsing.
>
> Yeah, aljazera.net, KDE bugreport 57020. I've implemented almost 
> exactly your
> patch as well a few months ago (attached):
>
> dirk at matrix:~/t> ls -l alja.html
> -rw-r--r--    1 dirk     users          98 2003-04-14 06:23 alja.html
>
> and since then I have this patch applied to my tree. The reason I 
> didn't
> commit yet is I still keep finding regressions due to the additional
> refcounting.

OK. Can you tell me a way to reproduce one of these regressions? I'm 
willing to look into it. I'm pretty sure that in my version all refs 
and derefs are paired up, so I don't think it could cause a leak, but I 
may have missed something. Your patch does not seem to include the 
change to KHTMLParser::finished, is that a difference between the 
WebCore and khtml trees?

Also, that's this about:

> -            insertNode(e);



> One of the reasons is the memory leaking contextual fragment code that 
> we
> merged from Safari tree. See attachment for a fix. To answer the FIXME
> comments in the code: yes, it does leak.

Patch looks good. Thanks for the fix!

I don't see why you changed the while loop to a two-clause for loop:

> -    NodeImpl *node = fragment->firstChild();
> -    while (node != NULL) {
> +    for ( NodeImpl* node = fragment->firstChild(); node; ) {

Or why you reindented a bunch of lines inconsistently (note the extra 
space on the second line, and then the lines that follow in the patch):

> +           NodeImpl* firstChild = node->firstChild();
> +            NodeImpl* child = firstChild;

I'll land your patch in WebCore except for the random indentation 
changes (I don't really care about the for loop).


> BTW, any chance that we can peek into that Changelog your diff 
> referenced?

We do have a gnu-style ChangeLog for all our changes and we are 
considering sending it out, but we have to make sure it doesn't include 
any Apple confidential information first so we don't get fired. I'm 
hoping we can get to this soon.

Maybe you guys could consider keeping a ChangeLog for the master KHTML 
tree too, that would make merging easier for us since we could see what 
changes go together. We have a perl script that generates the skeleton 
of the ChangeLog (the names of files and functions changed) so it's not 
a huge burden.


Regards,
Maciej