[WebKit-devel] Reviewing kdewebkit

[Andrea Diamantini]

Hi Dawit,
and thanks for fast response :)

> > 1.
> > I noticed the private implementation of KIO::AccessManager does an heavy
> >  use of its parent, calling it for every createRequest() call. Shouldn't
> > be better adding a KWebPage pointer there and using it?
> 
> That can and probably will be remedied. It was orignally implemented to
>  simply guard against someone setting the parent at a later point than the
>  construction of the object. However, I doubt we need to worry about such
>  corner cases specially when it can potentially have adverse impact on
>  performance...

I asked about this just to deduct a general case in the trade-off between using 
a m_parent pointer and calling the "qobject_casted" parent() method.

> So what you actually want is a
> 
> public Q_SLOTS:
> 	void downloadRequest(const QUrl& url);
> 
> Well that can be added and made to simply call downloadRequest(...). Note
>  that KWebPage does not currently have any slots because its parent does
>  not have any and tries to model itself after the parent class. Remember it
>  is simply a convenience wrapper class, i.e. drop in replacement for the
>  parent.

uhm... yes it seemed to me the best solution here. Anyway, I didn't notice 
this xWebPage design decision. Thanks for pointing this out.

> > 4.
> > I think the qt 4.6 checks in the code are unuseful because and should be
> > removed as Qt 4.6 is going to be an hard dependency of the whole KDE 4.4
> 
> You would not say that if you were developing the class and do not have Qt
> 4.6... ;) IOW, that is a non-issue that can be removed once KDE 4.4 has
>  been released...

:D
sure. Just noticing it.


Regards,

-- 
Andrea Diamantini, adjam
GPG Fingerprint: 57DE 8E32 7D1A 0E16 AA52 59D8 84F9 3ECD DBF9 730F

rekonq project
WEB: http://rekonq.sourceforge.net
IRC: rekonq at freenode