[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: KHTMLFactory redesign
From:       Allan Sandfeld Jensen <kde () carewolf ! com>
Date:       2007-12-02 16:53:41
Message-ID: 200712021753.42487.kde () carewolf ! com
[Download RAW message or body]

On Monday 26 November 2007 18:05, David Faure wrote:
> We've always gotten some trouble from the fact that KHTMLFactory has a dual
> purpose; being the KParts::Factory (when khtmlpart is dlopened, e.g. in
> konqueror), and being the repository for a bunch of global things needed by
> khtml.
>
> I've separated those two things, creating a KHTMLGlobal for the global
> stuff and leaving the KHTMLFactory to being the kparts factory only. It
> solves some life-cycle issues and allows to remove the ugly bool clone from
> the constructor. Apart from that the patch is mainly a
> s/KHTMLFactory::/KHTMLGlobal::/ everywhere since the khtml code only cares
> about those global things, not about the actual factory. But I also
> improved the refcounting mechanism which is now in KHTMLGlobal, so that we
> can debug the famous assert a bit more easily: ref/deref are no more
> public, the API is now limited to registerPart/deregisterPart and
> registerDocumentImpl/deregisterDocumentImpl. This allows to be more verbose
> when the assert fails and to say:
>
> konqueror(31854)/khtml KHTMLGlobal::finalCheck: 1 docs not deleted
> konqueror(31854)/khtml KHTMLGlobal::finalCheck: Document
> DOM::HTMLDocumentImpl(0x10f8330) wasn't deleted
>
> OK, I still have on idea why the documentimpl itself has one ref too many
> so KHTMLPart::clear() unrefs it but doesn't destroy it, but at least I'm
> more confident about the KHTMLFactory/KHTMLGlobal side of things now :)
>
> OK with me committing? (BTW KHTMLFactory was exported but not installed, it
> was only used by testkhtml and test_regression, which I ported; this change
> is completely SC/BC). After that, I'll try to keep debugging the assert,
> but I'm not sure how....

The patch looks good. I hope we can soon nail the assert.

`Allan


[prev in list] [next in list] [prev in thread] [next in thread]