Problems with KHTMView (and KHTMLPart) constructors

[Eeli Kaikkonen]

Hi, I'm new here. I had some bugs which I wanted to talk about and I
told about them in #khtml but nobody answered there. This is more than
just one bug report, therefore I thought it would be proper to use this
forum rather than bugs.kde.org.

The heart of the issue is how the KHTMLPart and KHTMLView constructors
work. It is not possible to set either Part for View later or vice
versa. This is not usually a problem but if I need to subclass both of
them it is.

Now, they are clearly meant to be subclassed because there are virtual
functions. But first problem is that there is no documented way to do
it. The only possibility which I know of is to construct the View
subclass in the initialization list of the Part subclass. That is quite
"advanced" technique IMO and should be documented clearly.

The second problem is that because the View is created in the init list
of the Part, the Part has not been fully constructed while the View is
being constructed. There was recently a bug which I reported personally
to ggarand and he fixed it in svn revision 739418. A member
function of m_part was used in the View constructor and that resulted in
crash because m_part was not constructed in that phase.

But now there appeared a new problem which I believe has basically the
same reason. Dfaure changed some things in revision 744435.
KHTMLViewPrivate::reset() has now this code:

// defaultHTMLSettings is available since a KHTMLPart has been created first
// and has ref()'ed the factory for us.
        accessKeysEnabled =
KHTMLGlobal::defaultHTMLSettings()->accessKeysEnabled();

If I have understood correctly this is called while constructing the
KHTMLView. Remembering what I just said about the constructors the
presupposition in the comment is not necessarily true: the KHTMLPart has
not necessarily been created and ref() has not been called. This leads
to assert() crash in KHTMLGlobal::defaultHTMLSettings():

KHTMLSettings *KHTMLGlobal::defaultHTMLSettings()
{
  assert( s_self );
  if ( !s_settings )
    s_settings = new KHTMLSettings();

  return s_settings;
}


I "fixed" this by removing the assert() but that is not the real
problem. The important problem is that khtml developers don't
know or don't remember to think about this specific situation every time
they change something in the code. It's very easy to make a change which
triggers this same situation, namely that the Part has not been
constructed while constructing the View.

I'm not experienced enough to say if this whole system of two-way
dependend classes and their constructors is an ugly hack but surely it
leads to problems.

The least what the khtml developers could do is to write and run a test
for this situation so that these problems would be detected before they
are committed.


  Yours,
	Eeli Kaikkonen (Mr.), Oulu, Finland
	e-mail: eekaikko at mailx.studentx.oulux.fix (with no x)