Bug 38921 – [Qt] emit initialLayoutCompleted signal from FrameLoaderClientQt::dispatchDidFirstVisuallyNonEmptyLayout

• Component: New Bugs
• Status: CLOSED FIXED

[Antonio Gomes]

In QWebFrame docs we have:

(..)
/*!
    \fn void QWebFrame::initialLayoutCompleted()

    This signal is emitted when the frame is laid out the first time.
    This is the first time you will see contents displayed on the frame.

    \note A frame can be laid out multiple times.
*/
(..)

However FrameLoaderClient::dispatchDidFirstLayout (who emits initialLayoutCompleted) can be called more than once and even when there is no content in fact "drawn". One example of this is when ones changes the viewport size (QWebPage::setViewportSize) before any page load, as in following snippet:

void tst_QGraphicsWebView::crashOnViewlessWebPages()
{
  QGraphicsScene scene;
  QGraphicsView view(&scene);

  QGraphicsWebView* webView = new QGraphicsWebView;
  WebPage* page = new WebPage;
  webView->setPage(page);
  page->webView = webView;
  scene.addItem(webView);

  connect(page->mainFrame(), SIGNAL(initialLayoutCompleted()), page, SLOT(aborting()));

  view.setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
  view.resize(600, 480);
  webView->resize(view.geometry().size());
  QCoreApplication::processEvents();
  view.show();

  page->mainFrame()->setHtml(QString("data:text/html,<h1>blah"));
  (...)
}

"initialLayoutCompleted" signal is emitted (and WebPage::aborting slot is called) even before page->mainFrame()->setHtml is executed because there is a webView->resize (which triggers qwebpage::setViewportSize and FrameView::layout())

In r39385 FrameLoaderClient::dispatchDidFirstVisuallyNonEmptyLayout was introduced, and I think it is the best place to emit initialLayoutCompleted signal. Its documentation says: 

"Add new FrameLoaderClient method to indicate the first visually non-empty layout based on an heuristic.  Right now that heuristic
is the first layout after an image, text or plugin has been added to the render tree, but I can imagine it becoming much smarter."

and it matches more to qwebframe docs.

[Kenneth Rohde Christiansen]

It is only called once, but once per page. "about:blank" also counts as a page.

That in mind, I still think it makes sense using the new dispatch method instead.

[Antonio Gomes]

Created attachment 55728 [details]
(committed in r59163, reviewed by kenneth) patch v1

[Antonio Gomes]

(In reply to comment #1)
> It is only called once, but once per page. "about:blank" also counts as a page.

To be precise, it is one per frame.

[Kenneth Rohde Christiansen]

Comment on attachment 55728 [details]
(committed in r59163, reviewed by kenneth) patch v1

LGTM

[Antonio Gomes]

Comment on attachment 55728 [details]
(committed in r59163, reviewed by kenneth) patch v1

Clearing flags on attachment: 55728

Committed r59163: <http://trac.webkit.org/changeset/59163>

[Simon Hausmann]

Revision r59163 cherry-picked into qtwebkit-2.0 with commit 0a778f7ce0f8c905339b884030b48d93d5b90b7c

[Early Warning System Bot]

Comment on attachment 55728 [details]
(committed in r59163, reviewed by kenneth) patch v1

Attachment 92515 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8592040