[Webkit-unassigned] [Bug 20932] Notable leaks in QtWebkit - from Qt library

[bugzilla-daemon at webkit.org]

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


hausmann at webkit.org changed:

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




------- Comment #13 from hausmann at webkit.org  2008-11-05 08:05 PDT -------
(From update of attachment 24227)
This patch looks a lot better! :-)

> Index: WebKit/qt/ChangeLog
> ===================================================================
> --- WebKit/qt/ChangeLog	(revision 37444)
> +++ WebKit/qt/ChangeLog	(working copy)
> @@ -1,3 +1,24 @@
> +2008-10-09  System User  <set EMAIL_ADDRESS environment variable>

You're missing your name and email address here :)

> Index: WebKit/qt/QtLauncher/QtLauncher.pro
> ===================================================================
> --- WebKit/qt/QtLauncher/QtLauncher.pro	(revision 37432)
> +++ WebKit/qt/QtLauncher/QtLauncher.pro	(working copy)
> @@ -3,6 +3,9 @@ SOURCES += main.cpp
>  CONFIG -= app_bundle
>  CONFIG += uitools
>  DESTDIR = ../../../bin
> +INCPATH += $$PWD/../../../JavaScriptCore/wtf
> +LIBS += -L../../../JavaScriptCore/ 
> +LIBS += -lJavaScriptCore

Ohh, is this needed because of the use of WTF::Vector?

I think in that case it would be easier to use QVector.
The code becomes a bit simpler at the same time - you could replace
m_urls.insert(i++, url) with m_urls.append(url).

Then it should not be necessary to link against the static JavaScriptCore
library.

>  include(../../../WebKit.pri)
>  
> Index: WebKit/qt/QtLauncher/main.cpp
> ===================================================================
> --- WebKit/qt/QtLauncher/main.cpp	(revision 37432)
> +++ WebKit/qt/QtLauncher/main.cpp	(working copy)
> @@ -42,6 +42,11 @@
>  
>  #include <QtUiTools/QUiLoader>
>  
> +#include <Vector.h>
> +#include <QTextStream>
> +#include <QFile>
> +#include <cstdio>
> +
>  class WebPage : public QWebPage
>  {
>  public:
> @@ -83,10 +88,14 @@ public:
>          }
>      }
>  
> -    QWebPage *webPage() const {
> +    QWebPage* webPage() const {
>          return view->page();
>      }
>  
> +    QWebView* webView() const {
> +        return view;
> +    }
> +
>  protected slots:
>  
>      void changeLocation() {
> @@ -322,6 +331,72 @@ QObject *WebPage::createPlugin(const QSt
>      return loader.createWidget(classId, view());
>  }
>  
> +class URLLoader : public QObject
> +{
> +    Q_OBJECT
> +public:
> +    URLLoader(QWebView* view, const QString& inputFileName)
> +        : m_view(view),
> +         stdOut(stdout)
> +    {
> +        init(inputFileName);
> +    }
> +
> +public slots:
> +    void loadNext()
> +    {
> +        QString qstr;
> +        if (getUrl(qstr)) {
> +            QUrl url(qstr, QUrl::StrictMode);
> +            if (url.isValid()) {
> +                stdOut<<"Loading "<<qstr<<" ......"<<endl;

Please put a space between the arguments.

r- because of the small issues, but in general the patch looks good!


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