Bug 94949 - [Qt][WK2] REGRESSION(r126067): It made qmltests::DoubleTapToZoom::test_basic() fail
Summary: [Qt][WK2] REGRESSION(r126067): It made qmltests::DoubleTapToZoom::test_basic(...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Simon Hausmann
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 70236 94493
  Show dependency treegraph
 
Reported: 2012-08-24 10:15 PDT by Csaba Osztrogonác
Modified: 2012-08-27 02:59 PDT (History)
14 users (show)

See Also:


Attachments
Patch (1.80 KB, patch)
2012-08-27 02:03 PDT, Simon Hausmann
kenneth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2012-08-24 10:15:32 PDT
After https://trac.webkit.org/changeset/126067 qmltests::DoubleTapToZoom::test_basic()
Qt-WK2 API test started to fail regularly. (not always, but in 90% of runs)
I got it with manual bisecting. It doesn't fail on r126066, but it fails
almost always on r126067.

FAIL!  : qmltests::DoubleTapToZoom::test_basic() Uncaught exception: Cannot read property 'width' of undefined
   Loc: [(0)]

You can reproduce the bug if you run this binary:
$ WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/qmltests/tst_qmltests_WebView
Comment 1 Csaba Osztrogonác 2012-08-24 10:23:36 PDT
cc JSC/QML experts, maybe you know what is it and how to fix it
Comment 2 Chris Dumez 2012-08-24 10:52:45 PDT
It appears this QML attempts to serialize a ClientRect object (unhandled host object), which is not supported according to spec (http://www.whatwg.org/specs/web-apps/current-work/multipage/common-dom-interfaces.html#safe-passing-of-structured-data).

As a consequence, a DataCloneError is being thrown.

tst_doubleTapToZoom.qml code:
function elementRect(id) {
            resultSpy.clear();
            var result;

             webView.experimental.evaluateJavaScript(
                "document.getElementById('" + id + "').getBoundingClientRect();",
                function(rect) { webView.resultReceived(); result = rect });
            resultSpy.wait();
            return result;
        }
Comment 3 Kenneth Rohde Christiansen 2012-08-24 10:54:22 PDT
I haven't looked at this in details, but Chris made the serialization more compliant with the specs and thus more strict.

 getBoundingClientRect(); returns ClientRect object (http://www.w3.org/TR/cssom-view/#clientrect) so it is not serializable and it has to be turned into an object literal (Object object) before serialization.

This following line in Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_doubleTapToZoom.qml

function(rect) { webView.resultReceived(); result = rect });

needs to be changed to

function(rect) { webView.resultReceived(); result = { width: rect.width, height: rect.height } });

or similar
Comment 4 Kenneth Rohde Christiansen 2012-08-24 10:57:54 PDT
Btw you need to do this before serialization!

webView.experimental.evaluateJavaScript(
    "var clientRect = document.getElementById('" + id + "').getBoundingClientRect();" +
    "return { width: clientRect.width, height: clientRect.height }",
    function(rect) { webView.resultReceived(); result = rect });
Comment 5 Simon Hausmann 2012-08-27 02:03:38 PDT
Created attachment 160674 [details]
Patch
Comment 6 Kenneth Rohde Christiansen 2012-08-27 02:12:05 PDT
Comment on attachment 160674 [details]
Patch

Funny I thought about this yesterday... I should have mentioned to just use the JSON methods, and here we go :-)
Comment 7 Simon Hausmann 2012-08-27 02:56:55 PDT
I still wonder if for the QML API we should automatically imply a JSON conversion or instead keep the current implementation and document that the return value will be cloned according to the html cloning algo.
Comment 8 Simon Hausmann 2012-08-27 02:59:42 PDT
Committed r126740: <http://trac.webkit.org/changeset/126740>