Changeset 62251 in webkit

Timestamp:

Jul 1, 2010 5:17:13 AM (14 years ago)

Author:

jocelyn.turcotte@nokia.com

Message:

2010-07-01 Jocelyn Turcotte <​jocelyn.turcotte@nokia.com>

Reviewed by Kenneth Rohde Christiansen.

[Qt] Create QComboBoxes when clicked and destroy on hide.
​https://bugs.webkit.org/show_bug.cgi?id=41451

Currently a QComboBox is created for each RenderMenuList and
it gets destroyed either when the RenderMenuList or the
QWebView (its Qt parent) is destroyed. This cause a crash
when the QWebView is destroyed before the render tree (which
is kept in cache).

This patch aim to destroy the QComboBox as soon as its popup
gets hidden, and likewise, create it only when the popup is
requested to be shown.
It also removes the unneeded reference to the QGraphicsProxyWidget,
destroying the QComboBox automatically destroys its bound
proxywidget.

• WebCoreSupport/QtFallbackWebPopup.cpp: (WebCore::QtFallbackWebPopupCombo::hidePopup): (WebCore::QtFallbackWebPopup::QtFallbackWebPopup): (WebCore::QtFallbackWebPopup::~QtFallbackWebPopup): (WebCore::QtFallbackWebPopup::show): (WebCore::QtFallbackWebPopup::hide): (WebCore::QtFallbackWebPopup::destroyPopup): (WebCore::QtFallbackWebPopup::populate):
• WebCoreSupport/QtFallbackWebPopup.h:

Location:

trunk/WebKit/qt

Files:

• ChangeLog (modified) (1 diff)
• WebCoreSupport/QtFallbackWebPopup.cpp (modified) (6 diffs)
• WebCoreSupport/QtFallbackWebPopup.h (modified) (2 diffs)

trunk/WebKit/qt/ChangeLog

@@ +1 @@
+2010-07-01  Jocelyn Turcotte  <jocelyn.turcotte@nokia.com>
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        [Qt] Create QComboBoxes when clicked and destroy on hide.
+        https://bugs.webkit.org/show_bug.cgi?id=41451
+
+        Currently a QComboBox is created for each RenderMenuList and
+        it gets destroyed either when the RenderMenuList or the
+        QWebView (its Qt parent) is destroyed. This cause a crash
+        when the QWebView is destroyed before the render tree (which
+        is kept in cache).
+
+        This patch aim to destroy the QComboBox as soon as its popup
+        gets hidden, and likewise, create it only when the popup is
+        requested to be shown.
+        It also removes the unneeded reference to the QGraphicsProxyWidget,
+        destroying the QComboBox automatically destroys its bound
+        proxywidget.
+
+        * WebCoreSupport/QtFallbackWebPopup.cpp:
+        (WebCore::QtFallbackWebPopupCombo::hidePopup):
+        (WebCore::QtFallbackWebPopup::QtFallbackWebPopup):
+        (WebCore::QtFallbackWebPopup::~QtFallbackWebPopup):
+        (WebCore::QtFallbackWebPopup::show):
+        (WebCore::QtFallbackWebPopup::hide):
+        (WebCore::QtFallbackWebPopup::destroyPopup):
+        (WebCore::QtFallbackWebPopup::populate):
+        * WebCoreSupport/QtFallbackWebPopup.h:
+
 2010-07-01  Satish Sampath  <satish@chromium.org>
 

trunk/WebKit/qt/WebCoreSupport/QtFallbackWebPopup.cpp

@@ -76 +76 @@
     QComboBox::hidePopup();
 
-    if (QGraphicsProxyWidget* proxy = graphicsProxyWidget())
-        proxy->setVisible(false);
-
     if (!m_ownerPopup.m_popupVisible)
         return;
@@ -84 +81 @@
     m_ownerPopup.m_popupVisible = false;
     m_ownerPopup.popupDidHide();
+    m_ownerPopup.destroyPopup();
 }
 
@@ -103 +101 @@
     : QtAbstractWebPopup()
     , m_popupVisible(false)
-    , m_combo(new QtFallbackWebPopupCombo(*this))
-    , m_proxy(0)
-{
-    connect(m_combo, SIGNAL(activated(int)),
-            SLOT(activeChanged(int)), Qt::QueuedConnection);
+    , m_combo(0)
+{
 }
 
 QtFallbackWebPopup::~QtFallbackWebPopup()
 {
-    // If we create a proxy, then the deletion of the proxy and the
-    // combo will be done by the proxy's parent (QGraphicsWebView)
-    if (!m_proxy && m_combo)
-        m_combo->deleteLater();
+    destroyPopup();
 }
 
@@ -126 +118 @@
     TRAP_IGNORE(showS60BrowserDialog());
 #else
+
+    destroyPopup();
+    m_combo = new QtFallbackWebPopupCombo(*this);
+    connect(m_combo, SIGNAL(activated(int)),
+            SLOT(activeChanged(int)), Qt::QueuedConnection);
+
     populate();
     m_combo->setCurrentIndex(currentIndex());
@@ -131 +129 @@
     QRect rect = geometry();
     if (QGraphicsWebView *webView = qobject_cast<QGraphicsWebView*>(pageClient()->pluginParent())) {
-        if (!m_proxy) {
-            m_proxy = new QGraphicsProxyWidget(webView);
-            m_proxy->setWidget(m_combo);
-        } else
-            m_proxy->setVisible(true);
-        m_proxy->setGeometry(rect);
+        QGraphicsProxyWidget* proxy = new QGraphicsProxyWidget(webView);
+        proxy->setWidget(m_combo);
+        proxy->setGeometry(rect);
     } else {
         m_combo->setParent(pageClient()->ownerWidget());
@@ -205 +200 @@
 void QtFallbackWebPopup::hide()
 {
-    m_combo->hidePopup();
+    // Destroying the QComboBox here cause problems if the popup is in the
+    // middle of its show animation. Instead we rely on the fact that the
+    // Qt::Popup window will hide itself on mouse events outside its window.
+}
+
+void QtFallbackWebPopup::destroyPopup()
+{
+    if (m_combo) {
+        m_combo->deleteLater();
+        m_combo = 0;
+    }
 }
 
 void QtFallbackWebPopup::populate()
 {
-    m_combo->clear();
-
     QStandardItemModel* model = qobject_cast<QStandardItemModel*>(m_combo->model());
     Q_ASSERT(model);

trunk/WebKit/qt/WebCoreSupport/QtFallbackWebPopup.h

@@ -43 +43 @@
     virtual void hide();
 
+    void destroyPopup();
+
 private slots:
     void activeChanged(int);
@@ -50 +52 @@
     bool m_popupVisible;
     QtFallbackWebPopupCombo* m_combo;
-    QGraphicsProxyWidget* m_proxy;
 
     void populate();