[Webkit-unassigned] [Bug 29803] [Qt] QWebHistory and QWebPage autotests crash

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=29803


Holger Freyther <zecke at selfish.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #41660|review?                     |review-
               Flag|                            |




--- Comment #24 from Holger Freyther <zecke at selfish.org>  2009-10-23 21:09:12 PDT ---
(From update of attachment 41660)
> diff --git a/WebKit/qt/Api/qwebframe.cpp b/WebKit/qt/Api/qwebframe.cpp
> index 45dbfba..d5b5f9e 100644
> --- a/WebKit/qt/Api/qwebframe.cpp
> +++ b/WebKit/qt/Api/qwebframe.cpp
> @@ -41,6 +41,7 @@
>  #include "JSDOMWindowBase.h"
>  #include "JSLock.h"
>  #include "JSObject.h"
> +#include "moc_qwebframe.cpp"


No... can you show me any other file doing that? We tend to put it at the
bottom...below anything else..


>  #include "NodeList.h"
>  #include "Page.h"
>  #include "PlatformMouseEvent.h"
> @@ -207,15 +208,21 @@ QWebFrameData::QWebFrameData(WebCore::Page* parentPage, WebCore::Frame* parentFr
>  void QWebFramePrivate::init(QWebFrame *qframe, QWebFrameData *frameData)
>  {
>      q = qframe;
> -

still a bogus whitespace change...



>      static WebCore::Frame* core(QWebFrame*);
>      static QWebFrame* kit(WebCore::Frame*);
> +    static QWebFrame* get(QWebFramePrivate* priv) { return priv->q; }
> +    static QWebFramePrivate* get(QWebFrame* pub) { return pub->d; }
>  

they are not used, there is no reason to have this in the patch.


> --- a/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp
> +++ b/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp
> @@ -123,11 +123,14 @@ private slots:
>      void inputMethods();
>      void defaultTextEncoding();
>      void errorPageExtension();
> -
>      void crashTests_LazyInitializationOfMainFrame();
> -

why did you remove this space?


> +void tst_QWebPage::frameCreatedSignal()
> +{
> +    // A MainFrame should be created before ~QWebPage because the WebCore::Page (the base classs)
> +    // use the frame in the destructor.
> +    QWebPage page;
> +    bool sign = waitForSignal(&page, SIGNAL(frameCreated(QWebFrame*)));
> +    QVERIFY2(sign, "A QWebFrame hasn't been created before ~QWebPage");
> +}
> +

One thing I don't like about this is the following.

if I change the test case above to do

QWebPage page;
page.load("<script>window.print()</script>");
bool sign = waitForSi...

I will be likely to get the "printRequested(QWebFrame*)" before the
frameCreated signal... which is making things worse...

I think the API contract should be changed to not emit frameCreated from the
QWebPage c'tor and that it is guranteed that the mainFrame() exists...

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.