[webkit-reviews] review granted: [Bug 86474] [Qt] Add infra for testing double-tap to zoom functionality etc : [Attachment 141945] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Kenneth Rohde Christiansen
<kenneth at webkit.org>'s request for review:
Bug 86474: [Qt] Add infra for testing double-tap to zoom functionality etc
https://bugs.webkit.org/show_bug.cgi?id=86474

Attachment 141945: Patch
https://bugs.webkit.org/attachment.cgi?id=141945&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=141945&action=review


r=me, I like the step of moving the viewport info stuff into something that can
be re-used for unit testing. But it doesn't belong into experimental (because
we know this experiment is never going to succeed to become public API) and it
should only be instantiated when run as part of the unit tests.

I suggest private C++ API to expose (and thus lazily create) the testing object
and then make it available in TestWebView.

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:114
> +    emit d->viewportItem->experimental()->test()->contentsSizeChanged();

So even "production" code instantiates the test object and emits signals.
Wouldn't it be better to avoid that?

> Source/WebKit2/UIProcess/API/qt/qwebkittest.cpp:53
> +    // if (delay > 0)
> +    //     QTest::qWait(delay);

Forgot to remove this? :)