⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 88796 in webkit


Ignore:
Timestamp:
Jun 14, 2011, 6:53:38 AM (15 years ago)
Author:
caio.oliveira@openbossa.org
Message:

2011-06-14 Caio Marcelo de Oliveira Filho <​caio.oliveira@openbossa.org>

Reviewed by Andreas Kling.

[Qt] tst_QWebFrame::overloadedSlots() fails
​https://bugs.webkit.org/show_bug.cgi?id=37319

Increase the likeness that JSElements are converted to QWebElements. When hinted
with QWebElement metatype, we qualify the conversion from JSElement as a "perfect
match".

The test was failing because the wrong slot was called, since the QWebElement
match was taken as equal to the others and not chosen when the metacall happened.

We also remove the implicit conversion between JSDocument (which is not an
element) to QWebElement. The conversion only worked for calling slots, while
without hint it returned a QVariantMap (as can be seen in domCycles test). It was
added for supporting DRT, but since this change we can use it as QVariantMap and
get the value for "documentElement".

This patch is based on Noam Rosenthal original patch in the same bug.

  • bridge/qt/qt_runtime.cpp: (JSC::Bindings::hintForRealType): Extracted function to choose the metatype hint based on the JSValue type. Add QWebElement metatype as a hint for JSElement objects, this way if no hint is provided, JSElement objects will always be converted to QWebElements.

(JSC::Bindings::convertValueToQVariant):
Use previous function. Identify the conversion between JSElement to QWebElement
as a "perfect match" (dist = 0). And remove the implicit conversion when the hint
is QWebElement metatype and we have a JSDocument. Changed from JSHTMLElement to
JSElement to cover the 'documentElement'.

2011-06-14 Caio Marcelo de Oliveira Filho <​caio.oliveira@openbossa.org>

Reviewed by Andreas Kling.

[Qt] tst_QWebFrame::overloadedSlots() fails
​https://bugs.webkit.org/show_bug.cgi?id=37319

Since the implicit conversion was removed, change support functions of DRT to
expect a QVariantMap instead of a QWebElement. This matches the exposed function
in the controller, which takes 'document' and not 'document.documentElement'.

And now that Element -> QWebElement is a perfect match, we must use QWebElement
instead of QVariantMap, like in plainText().

  • WebCoreSupport/DumpRenderTreeSupportQt.h:
  • WebCoreSupport/DumpRenderTreeSupportQt.cpp: (DumpRenderTreeSupportQt::plainText): Fix to use QWebElement instead of QVariantMap when getting the startContainer. Also use QVariantMap directly, bridge will do conversion for us now.

(getCoreDocumentFromVariantMap): Extracts the WebCore::Document* from the
QVariantMap that Qt bridge gives us when 'document' is passed from JS.

(DumpRenderTreeSupportQt::nodesFromRect): Use helper function.

  • tests/qwebframe/tst_qwebframe.cpp: Splitted the test domCycles() into two different tests. In practice, the original test just checked whether we could create a QVariantMap representing 'document' without infinite looping due to cycles in the DOM. This was more evident before since we haven't a conversion from JSElement to QWebElement, but from JSElement to QVariantMap.

(tst_QWebFrame::documentHasDocumentElement): Evaluates 'document' and extracts
'documentElement' from it. Compares to QWebFrame::documentElement().

(tst_QWebFrame::documentAllHasDocumentElement): Look inside 'document.all' for
the documentElement.

(tst_QWebFrame::overloadedSlots): Remove expected failure and fix wrong comment.

2011-06-14 Caio Marcelo de Oliveira Filho <​caio.oliveira@openbossa.org>

Reviewed by Andreas Kling.

[Qt] tst_QWebFrame::overloadedSlots() fails
​https://bugs.webkit.org/show_bug.cgi?id=37319

Since we don't implictly convert 'document' object to QWebElement
in metacalls anymore, change the controller to expect QVariantMap instead.

The method plainText() was updated to use QVariantMap as well to let the bridge
do the conversion directly for us.

  • DumpRenderTree/qt/LayoutTestControllerQt.cpp: (LayoutTestController::nodesFromRect):
  • DumpRenderTree/qt/LayoutTestControllerQt.h:
  • DumpRenderTree/qt/PlainTextControllerQt.cpp: (PlainTextController::plainText):
  • DumpRenderTree/qt/PlainTextControllerQt.h:
Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r88794 r88796  
     12011-06-14  Caio Marcelo de Oliveira Filho  <caio.oliveira@openbossa.org>
     2
     3        Reviewed by Andreas Kling.
     4
     5        [Qt] tst_QWebFrame::overloadedSlots() fails
     6        https://bugs.webkit.org/show_bug.cgi?id=37319
     7
     8        Increase the likeness that JSElements are converted to QWebElements. When hinted
     9        with QWebElement metatype, we qualify the conversion from JSElement as a "perfect
     10        match".
     11
     12        The test was failing because the wrong slot was called, since the QWebElement
     13        match was taken as equal to the others and not chosen when the metacall happened.
     14
     15        We also remove the implicit conversion between JSDocument (which is not an
     16        element) to QWebElement. The conversion only worked for calling slots, while
     17        without hint it returned a QVariantMap (as can be seen in domCycles test). It was
     18        added for supporting DRT, but since this change we can use it as QVariantMap and
     19        get the value for "documentElement".
     20
     21        This patch is based on Noam Rosenthal original patch in the same bug.
     22
     23        * bridge/qt/qt_runtime.cpp:
     24        (JSC::Bindings::hintForRealType):
     25        Extracted function to choose the metatype hint based on the JSValue type.  Add
     26        QWebElement metatype as a hint for JSElement objects, this way if no hint is
     27        provided, JSElement objects will always be converted to QWebElements.
     28
     29        (JSC::Bindings::convertValueToQVariant):
     30        Use previous function. Identify the conversion between JSElement to QWebElement
     31        as a "perfect match" (dist = 0). And remove the implicit conversion when the hint
     32        is QWebElement metatype and we have a JSDocument.  Changed from JSHTMLElement to
     33        JSElement to cover the 'documentElement'.
     34
    1352011-06-14  Andreas Kling  <kling@webkit.org>
    236

  • trunk/Source/WebCore/bridge/qt/qt_runtime.cpp

    r88635 r88796  
    183183}
    184184
     185static QMetaType::Type hintForRealType(JSRealType type, JSObject* object)
     186{
     187    switch (type) {
     188    case Number:
     189        return QMetaType::Double;
     190    case Boolean:
     191        return QMetaType::Bool;
     192    case String:
     193        return QMetaType::QString;
     194    case Date:
     195        return QMetaType::QDateTime;
     196    case RegExp:
     197        return QMetaType::QRegExp;
     198    case Object:
     199        if (object->inherits(&NumberObject::s_info))
     200            return QMetaType::Double;
     201        if (object->inherits(&BooleanObject::s_info))
     202            return QMetaType::Bool;
     203        if (object->inherits(&JSElement::s_info))
     204            return static_cast<QMetaType::Type>(qMetaTypeId<QWebElement>());
     205        return QMetaType::QVariantMap;
     206    case QObj:
     207        return QMetaType::QObjectStar;
     208    case JSByteArray:
     209        return QMetaType::QByteArray;
     210    case Array:
     211    case RTArray:
     212        return QMetaType::QVariantList;
     213    }
     214    return QMetaType::QString;
     215}
     216
    185217QVariant convertValueToQVariant(ExecState*, JSValue, QMetaType::Type, int*, HashSet<JSObject*>*, int);
    186218
    … …  
    242274    JSLock lock(SilenceAssertionsOnly);
    243275    JSRealType type = valueRealType(exec, value);
    244     if (hint == QMetaType::Void) {
    245         switch(type) {
    246             case Number:
    247                 hint = QMetaType::Double;
    248                 break;
    249             case Boolean:
    250                 hint = QMetaType::Bool;
    251                 break;
    252             case String:
    253             default:
    254                 hint = QMetaType::QString;
    255                 break;
    256             case Date:
    257                 hint = QMetaType::QDateTime;
    258                 break;
    259             case RegExp:
    260                 hint = QMetaType::QRegExp;
    261                 break;
    262             case Object:
    263                 if (object->inherits(&NumberObject::s_info))
    264                     hint = QMetaType::Double;
    265                 else if (object->inherits(&BooleanObject::s_info))
    266                     hint = QMetaType::Bool;
    267                 else
    268                     hint = QMetaType::QVariantMap;
    269                 break;
    270             case QObj:
    271                 hint = QMetaType::QObjectStar;
    272                 break;
    273             case JSByteArray:
    274                 hint = QMetaType::QByteArray;
    275                 break;
    276             case Array:
    277             case RTArray:
    278                 hint = QMetaType::QVariantList;
    279                 break;
    280         }
    281     }
     276    if (hint == QMetaType::Void)
     277        hint = hintForRealType(type, object);
    282278
    283279    qConvDebug() << "convertValueToQVariant: jstype is " << type << ", hint is" << hint;
    … …  
    779775                ret = QtPixmapInstance::variantFromObject(object, static_cast<QMetaType::Type>(hint));
    780776            } else if (hint == (QMetaType::Type) qMetaTypeId<QWebElement>()) {
    781                 if (object && object->inherits(&JSHTMLElement::s_info))
    782                     ret = QVariant::fromValue<QWebElement>(QtWebElementRuntime::create((static_cast<JSHTMLElement*>(object))->impl()));
    783                 else if (object && object->inherits(&JSDocument::s_info))
    784                     ret = QVariant::fromValue<QWebElement>(QtWebElementRuntime::create((static_cast<JSDocument*>(object))->impl()->documentElement()));
    785                 else
    786                     ret = QVariant::fromValue<QWebElement>(QWebElement());
     777                if (object && object->inherits(&JSElement::s_info)) {
     778                    ret = QVariant::fromValue<QWebElement>(QtWebElementRuntime::create((static_cast<JSElement*>(object))->impl()));
     779                    dist = 0;
     780                    // Allow other objects to reach this one. This won't cause our algorithm to
     781                    // loop since when we find an Element we do not recurse.
     782                    visitedObjects->remove(object);
     783                    break;
     784                }
     785                ret = QVariant::fromValue<QWebElement>(QWebElement());
    787786            } else if (hint == (QMetaType::Type) qMetaTypeId<QDRTNode>()) {
    788787                if (object && object->inherits(&JSNode::s_info))

  • trunk/Source/WebKit/qt/ChangeLog

    r88795 r88796  
     12011-06-14  Caio Marcelo de Oliveira Filho  <caio.oliveira@openbossa.org>
     2
     3        Reviewed by Andreas Kling.
     4
     5        [Qt] tst_QWebFrame::overloadedSlots() fails
     6        https://bugs.webkit.org/show_bug.cgi?id=37319
     7
     8        Since the implicit conversion was removed, change support functions of DRT to
     9        expect a QVariantMap instead of a QWebElement. This matches the exposed function
     10        in the controller, which takes 'document' and not 'document.documentElement'.
     11
     12        And now that Element -> QWebElement is a perfect match, we must use QWebElement
     13        instead of QVariantMap, like in plainText().
     14
     15        * WebCoreSupport/DumpRenderTreeSupportQt.h:
     16        * WebCoreSupport/DumpRenderTreeSupportQt.cpp:
     17        (DumpRenderTreeSupportQt::plainText): Fix to use QWebElement instead of
     18        QVariantMap when getting the startContainer. Also use QVariantMap directly,
     19        bridge will do conversion for us now.
     20
     21        (getCoreDocumentFromVariantMap): Extracts the WebCore::Document* from the
     22        QVariantMap that Qt bridge gives us when 'document' is passed from JS.
     23
     24        (DumpRenderTreeSupportQt::nodesFromRect): Use helper function.
     25
     26        * tests/qwebframe/tst_qwebframe.cpp: Splitted the test domCycles() into two
     27        different tests. In practice, the original test just checked whether we could
     28        create a QVariantMap representing 'document' without infinite looping due to
     29        cycles in the DOM. This was more evident before since we haven't a conversion
     30        from JSElement to QWebElement, but from JSElement to QVariantMap.
     31
     32        (tst_QWebFrame::documentHasDocumentElement): Evaluates 'document' and extracts
     33        'documentElement' from it. Compares to QWebFrame::documentElement().
     34
     35        (tst_QWebFrame::documentAllHasDocumentElement): Look inside 'document.all' for
     36        the documentElement.
     37
     38        (tst_QWebFrame::overloadedSlots): Remove expected failure and fix wrong comment.
     39
    1402011-06-14  Andreas Kling  <kling@webkit.org>
    241

  • trunk/Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp

    r88446 r88796  
    975975}
    976976
    977 QString DumpRenderTreeSupportQt::plainText(const QVariant& range)
    978 {
    979     QMap<QString, QVariant> map = range.toMap();
    980     QVariant startContainer  = map.value(QLatin1String("startContainer"));
    981     map = startContainer.toMap();
    982 
    983     return map.value(QLatin1String("innerText")).toString();
    984 }
    985 
    986 QVariantList DumpRenderTreeSupportQt::nodesFromRect(const QWebElement& document, int x, int y, unsigned top, unsigned right, unsigned bottom, unsigned left, bool ignoreClipping)
     977QString DumpRenderTreeSupportQt::plainText(const QVariantMap& range)
     978{
     979    QVariant v = range.value(QLatin1String("startContainer"));
     980    ASSERT(v.isValid());
     981    QWebElement startContainer = qvariant_cast<QWebElement>(v);
     982    return startContainer.toPlainText();
     983}
     984
     985WebCore::Document* DumpRenderTreeSupportQt::getCoreDocumentFromVariantMap(const QVariantMap& document)
     986{
     987    QVariant v = document.value(QLatin1String("documentElement"));
     988    ASSERT(v.isValid());
     989    QWebElement documentElement = qvariant_cast<QWebElement>(v);
     990
     991    WebCore::Element* element = documentElement.m_element;
     992    if (!element)
     993        return 0;
     994
     995    return element->document();
     996}
     997
     998QVariantList DumpRenderTreeSupportQt::nodesFromRect(const QVariantMap& document, int x, int y, unsigned top, unsigned right, unsigned bottom, unsigned left, bool ignoreClipping)
    987999{
    9881000    QVariantList res;
    989     WebCore::Element* webElement = document.m_element;
    990     if (!webElement)
    991         return res;
    992 
    993     Document* doc = webElement->document();
     1001    Document* doc = getCoreDocumentFromVariantMap(document);
    9941002    if (!doc)
    9951003        return res;

  • trunk/Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.h

    r88446 r88796  
    2828
    2929namespace WebCore {
     30class Document;
    3031class Text;
    3132class Node;
    … …  
    161162    static QString markerTextForListItem(const QWebElement& listItem);
    162163    static QVariantMap computedStyleIncludingVisitedInfo(const QWebElement& element);
    163     static QString plainText(const QVariant& rng);
     164    static QString plainText(const QVariantMap& range);
    164165
    165166    static void dumpFrameLoader(bool b);
    … …  
    199200    static void scalePageBy(QWebFrame*, float scale, const QPoint& origin);
    200201
    201     static QVariantList nodesFromRect(const QWebElement& document, int x, int y, unsigned top, unsigned right, unsigned bottom, unsigned left, bool ignoreClipping);
     202    static QVariantList nodesFromRect(const QVariantMap& document, int x, int y, unsigned top, unsigned right, unsigned bottom, unsigned left, bool ignoreClipping);
    202203    static QString responseMimeType(QWebFrame*);
    203204    static void clearOpener(QWebFrame*);
    … …  
    219220
    220221    static void injectInternalsObject(QWebFrame*);
     222
     223private:
     224    static WebCore::Document* getCoreDocumentFromVariantMap(const QVariantMap& document);
    221225};
    222226

  • trunk/Source/WebKit/qt/tests/qwebframe/tst_qwebframe.cpp

    r87818 r88796  
    613613    void progressSignal();
    614614    void urlChange();
    615     void domCycles();
     615    void documentHasDocumentElement();
     616    void documentAllHasDocumentElement();
    616617    void requestedUrl();
    617618    void requestedUrlAfterSetAndLoadFailures();
    … …  
    20052006    */
    20062007
    2007     // should pick myOverloadedSlot(QRegExp)
     2008    // should pick myOverloadedSlot(QWebElement)
    20082009    m_myObject->resetQtFunctionInvoked();
    20092010    evalJS("myObject.myOverloadedSlot(document.body)");
    2010     QEXPECT_FAIL("", "https://bugs.webkit.org/show_bug.cgi?id=37319", Continue);
    20112011    QCOMPARE(m_myObject->qtFunctionInvoked(), 36);
    20122012
    … …  
    22922292
    22932293
    2294 void tst_QWebFrame::domCycles()
    2295 {
    2296     m_view->setHtml("<html><body>");
    2297     QVariant v = m_page->mainFrame()->evaluateJavaScript("document");
    2298     QVERIFY(v.type() == QVariant::Map);
     2294void tst_QWebFrame::documentHasDocumentElement()
     2295{
     2296    m_view->setHtml("<html><body></body></html>");
     2297    QVariant docVariant = m_page->mainFrame()->evaluateJavaScript("document");
     2298    QVERIFY(docVariant.isValid());
     2299    QCOMPARE(docVariant.type(), QVariant::Map);
     2300    QVariantMap document = docVariant.toMap();
     2301
     2302    QVariant docElementVariant = document.value("documentElement");
     2303    QVERIFY(docElementVariant.isValid());
     2304    QCOMPARE(docElementVariant.userType(), qMetaTypeId<QWebElement>());
     2305    QWebElement documentElement = qvariant_cast<QWebElement>(docElementVariant);
     2306
     2307    QVERIFY(!documentElement.isNull());
     2308    QCOMPARE(documentElement, m_page->mainFrame()->documentElement());
     2309}
     2310
     2311void tst_QWebFrame::documentAllHasDocumentElement()
     2312{
     2313    m_view->setHtml("<html><body></body></html>");
     2314    QVariant docVariant = m_page->mainFrame()->evaluateJavaScript("document");
     2315    QVariantMap document = docVariant.toMap();
     2316
     2317    QVariant allVariant = document.value("all");
     2318    QVERIFY(allVariant.isValid());
     2319    QCOMPARE(allVariant.type(), QVariant::Map);
     2320    QVariantMap all = allVariant.toMap();
     2321
     2322    bool foundDocumentElement = false;
     2323    foreach (QVariant v, all.values()) {
     2324        if (v.userType() != qMetaTypeId<QWebElement>())
     2325            continue;
     2326        QWebElement e = qvariant_cast<QWebElement>(v);
     2327        if (e == m_page->mainFrame()->documentElement()) {
     2328            foundDocumentElement = true;
     2329            break;
     2330        }
     2331    }
     2332
     2333    QVERIFY(foundDocumentElement);
    22992334}
    23002335

  • trunk/Tools/ChangeLog

    r88791 r88796  
     12011-06-14  Caio Marcelo de Oliveira Filho  <caio.oliveira@openbossa.org>
     2
     3        Reviewed by Andreas Kling.
     4
     5        [Qt] tst_QWebFrame::overloadedSlots() fails
     6        https://bugs.webkit.org/show_bug.cgi?id=37319
     7
     8        Since we don't implictly convert 'document' object to QWebElement
     9        in metacalls anymore, change the controller to expect QVariantMap instead.
     10
     11        The method plainText() was updated to use QVariantMap as well to let the bridge
     12        do the conversion directly for us.
     13
     14        * DumpRenderTree/qt/LayoutTestControllerQt.cpp:
     15        (LayoutTestController::nodesFromRect):
     16        * DumpRenderTree/qt/LayoutTestControllerQt.h:
     17        * DumpRenderTree/qt/PlainTextControllerQt.cpp:
     18        (PlainTextController::plainText):
     19        * DumpRenderTree/qt/PlainTextControllerQt.h:
     20
    1212011-06-14  Andras Becsi  <abecsi@webkit.org>
    222

  • trunk/Tools/DumpRenderTree/qt/LayoutTestControllerQt.cpp

    r88626 r88796  
    910910}
    911911
    912 QVariantList LayoutTestController::nodesFromRect(const QWebElement& document, int x, int y, unsigned top, unsigned right, unsigned bottom, unsigned left, bool ignoreClipping)
     912QVariantList LayoutTestController::nodesFromRect(const QVariantMap& document, int x, int y, unsigned top, unsigned right, unsigned bottom, unsigned left, bool ignoreClipping)
    913913{
    914914    return DumpRenderTreeSupportQt::nodesFromRect(document, x, y, top, right, bottom, left, ignoreClipping);

  • trunk/Tools/DumpRenderTree/qt/LayoutTestControllerQt.h

    r88626 r88796  
    237237    bool hasSpellingMarker(int from, int length);
    238238
    239     QVariantList nodesFromRect(const QWebElement& document, int x, int y, unsigned top, unsigned right, unsigned bottom, unsigned left, bool ignoreClipping);
     239    QVariantList nodesFromRect(const QVariantMap& document, int x, int y, unsigned top, unsigned right, unsigned bottom, unsigned left, bool ignoreClipping);
    240240
    241241    void addURLToRedirect(const QString& origin, const QString& destination);

  • trunk/Tools/DumpRenderTree/qt/PlainTextControllerQt.cpp

    r82489 r88796  
    3939}
    4040
    41 QString PlainTextController::plainText(const QVariant& range)
     41QString PlainTextController::plainText(const QVariantMap& range)
    4242{
    4343    return DumpRenderTreeSupportQt::plainText(range);

  • trunk/Tools/DumpRenderTree/qt/PlainTextControllerQt.h

    r68825 r88796  
    4242
    4343public slots:
    44     QString plainText(const QVariant& range);
     44    QString plainText(const QVariantMap& range);
    4545};
    4646
Note: See TracChangeset for help on using the changeset viewer.