Changeset 56450 in webkit

Timestamp:

Mar 24, 2010 12:05:25 PM (14 years ago)

Author:

kenneth@webkit.org

Message:

WebCore: Add a way to check if the page client is making use of
a QWidget.

Reviewed by Simon Hausmann.

• platform/qt/QWebPageClient.h:

(QWebPageClient::isQWidgetClient):

WebKit/qt: Calling setView on a QWebPage being shown by a QGraphicsWebView,
would uninstall the page client, resulting in not expected
behaviour.

Reviewed by Simon Hausmann.

• Api/qwebpage.cpp:

(QWebPageWidgetClient::QWebPageWidgetClient):
(QWebPageWidgetClient::isQWidgetClient):
(QWebPageWidgetClient::screenNumber):
(QWebPage::QWebPage):
(QWebPage::setView):

Location:

trunk

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/platform/qt/QWebPageClient.h (modified) (1 diff)
• WebKit/qt/Api/qgraphicswebview.cpp (modified) (5 diffs)
• WebKit/qt/Api/qwebpage.cpp (modified) (4 diffs)
• WebKit/qt/Api/qwebpage.h (modified) (1 diff)
• WebKit/qt/Api/qwebview.cpp (modified) (3 diffs)
• WebKit/qt/ChangeLog (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-03-23  Kenneth Rohde Christiansen  <kenneth@webkit.org>
+
+        Reviewed by Simon Hausmann.
+
+        Add a way to check if the page client is making use of
+        a QWidget.
+
+        * platform/qt/QWebPageClient.h:
+        (QWebPageClient::isQWidgetClient):
+
 2010-03-24  Jay Campan  <jcampan@google.com>
 

trunk/WebCore/platform/qt/QWebPageClient.h

@@ -41 +41 @@
 public:
     virtual ~QWebPageClient() { }
-        
+
+    virtual bool isQWidgetClient() const { return false; }
+
     virtual void scroll(int dx, int dy, const QRect&) = 0;
     virtual void update(const QRect&) = 0;

trunk/WebKit/qt/Api/qgraphicswebview.cpp

@@ -96 +96 @@
 
     virtual ~QGraphicsWebViewPrivate();
+
     virtual void scroll(int dx, int dy, const QRect&);
     virtual void update(const QRect& dirtyRect);
@@ -122 +123 @@
     void updateCompositingScrollPosition();
 #endif
-    
+
     void updateResizesToContentsForPage();
     QRectF graphicsItemVisibleRect() const;
@@ -132 +133 @@
 
     void syncLayers();
+
+    void unsetPageIfExists();
+
     void _q_doLoadFinished(bool success);
     void _q_contentsSizeChanged(const QSize&);
@@ -688 +692 @@
 }
 
+void QGraphicsWebViewPrivate::unsetPageIfExists()
+{
+    if (!page)
+        return;
+
+    // if the page client is the special client constructed for
+    // delegating the responsibilities to a QWidget, we need
+    // to destroy it.
+
+    if (page->d->client && page->d->client->isQWidgetClient())
+        delete page->d->client;
+
+    page->d->client = 0;
+
+    // if the page was created by us, we own it and need to
+    // destroy it as well.
+
+    if (page->parent() == q)
+        delete page;
+    else
+        page->disconnect(q);
+}
+
 /*!
     Makes \a page the new web page of the web graphicsitem.
@@ -702 +729 @@
         return;
 
-    if (d->page) {
-        d->page->d->client = 0; // unset the page client
-        if (d->page->parent() == this)
-            delete d->page;
-        else
-            d->page->disconnect(this);
-    }
-
+    d->unsetPageIfExists();
     d->page = page;
+
     if (!d->page)
         return;
+
+    d->page->d->client = d; // set the page client
+
     if (d->overlay)
         d->overlay->prepareGeometryChange();
-    d->page->d->client = d; // set the page client
 
     QSize size = geometry().size().toSize();
     page->setViewportSize(size);
-    
+
     if (d->resizesToContents)
         d->updateResizesToContentsForPage();

trunk/WebKit/qt/Api/qwebpage.cpp

@@ -209 +209 @@
     }
 
+    virtual bool isQWidgetClient() const { return true; }
+
     virtual void scroll(int dx, int dy, const QRect&);
     virtual void update(const QRect& dirtyRect);
@@ -282 +284 @@
 {
 #if defined(Q_WS_X11)
-    if (view)
-        return view->x11Info().screen();
-#endif
-
+    return view->x11Info().screen();
+#endif
     return 0;
 }
@@ -1781 +1781 @@
     , d(new QWebPagePrivate(this))
 {
-    setView(qobject_cast<QWidget *>(parent));
+    setView(qobject_cast<QWidget*>(parent));
 
     connect(this, SIGNAL(loadProgress(int)), this, SLOT(_q_onLoadProgressChanged(int)));
@@ -1865 +1865 @@
     \sa view()
 */
-void QWebPage::setView(QWidget *view)
-{
-    if (this->view() != view) {
-        d->view = view;
-        if (!view) {
-            delete d->client;
-            d->client = 0;
-        } else {
-            if (!d->client) 
-                d->client = new QWebPageWidgetClient(view);
-            else
-                static_cast<QWebPageWidgetClient*>(d->client)->view = view;
-        }
-        setViewportSize(view ? view->size() : QSize(0, 0));
-    }
+void QWebPage::setView(QWidget* view)
+{
+    if (this->view() == view)
+        return;
+
+    d->view = view;
+    setViewportSize(view ? view->size() : QSize(0, 0));
+
+    // If we have no client, we install a special client delegating
+    // the responsibility to the QWidget. This is the code path
+    // handling a.o. the "legacy" QWebView.
+    //
+    // If such a special delegate already exist, we substitute the view.
+
+    if (d->client) {
+        if (d->client->isQWidgetClient())
+            static_cast<QWebPageWidgetClient*>(d->client)->view = view;
+        return;
+    }
+
+    if (view)
+        d->client = new QWebPageWidgetClient(view);
 }
 

trunk/WebKit/qt/Api/qwebpage.h

@@ -357 +357 @@
     friend class QWebPagePrivate;
     friend class QWebView;
+    friend class QWebViewPrivate;
     friend class QGraphicsWebView;
+    friend class QGraphicsWebViewPrivate;
     friend class QWebInspector;
     friend class WebCore::ChromeClientQt;

trunk/WebKit/qt/Api/qwebview.cpp

@@ -46 +46 @@
 
     void _q_pageDestroyed();
+    void unsetPageIfExists();
 
     QWebView *view;
@@ -348 +349 @@
 }
 
+void QWebViewPrivate::unsetPageIfExists()
+{
+    if (!page)
+        return;
+
+    // if the page client is the special client constructed for
+    // delegating the responsibilities to a QWidget, we need
+    // to destroy it.
+
+    if (page->d->client && page->d->client->isQWidgetClient())
+        delete page->d->client;
+
+    page->d->client = 0;
+
+    // if the page was created by us, we own it and need to
+    // destroy it as well.
+
+    if (page->parent() == view)
+        delete page;
+    else
+        page->disconnect(view);
+}
+
 /*!
     Makes \a page the new web page of the web view.
@@ -361 +385 @@
     if (d->page == page)
         return;
-    if (d->page) {
-        d->page->d->client = 0; // unset the page client
-        if (d->page->parent() == this)
-            delete d->page;
-        else
-            d->page->disconnect(this);
-    }
+
+    d->unsetPageIfExists();
     d->page = page;
+
     if (d->page) {
         d->page->setView(this);

trunk/WebKit/qt/ChangeLog

@@ +1 @@
+2010-03-23  Kenneth Rohde Christiansen  <kenneth@webkit.org>
+
+        Reviewed by Simon Hausmann.
+
+        Calling setView(0) on a QWebPage being shown by a QGraphicsWebView,
+        would uninstall the page client, deleting the QGraphicsWebViewPrivate
+        instance. If called with an argument, it would do a wrong static_cast
+        and crash.
+
+        * Api/qwebpage.cpp:
+        (QWebPageWidgetClient::QWebPageWidgetClient):
+        (QWebPageWidgetClient::isQWidgetClient):
+        (QWebPageWidgetClient::screenNumber):
+        (QWebPage::QWebPage):
+        (QWebPage::setView):
+        * Api/qgraphicswebview.cpp:
+        (QGraphicsWebViewPrivate::unsetPageIfExists):
+        (QGraphicsWebView::setPage):
+        * Api/qwebpage.cpp:
+        (QWebPageWidgetClient::isQWidgetClient):
+        (QWebPageWidgetClient::screenNumber):
+        (QWebPage::QWebPage):
+        (QWebPage::setView):
+        * Api/qwebpage.h:
+        * Api/qwebview.cpp:
+        (QWebViewPrivate::unsetPageIfExists):
+        (QWebView::setPage):
+
 2010-03-24  Kent Tamura  <tkent@chromium.org>