Bug 38921 - [Qt] emit initialLayoutCompleted signal from FrameLoaderClientQt::dispatchDidFirstVisuallyNonEmptyLayout
Summary: [Qt] emit initialLayoutCompleted signal from FrameLoaderClientQt::dispatchDid...
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Antonio Gomes
Keywords: Qt, QtTriaged
Depends on:
Blocks: 35784
  Show dependency treegraph
Reported: 2010-05-11 11:39 PDT by Antonio Gomes
Modified: 2011-05-05 18:04 PDT (History)
2 users (show)

See Also:

(committed in r59163, reviewed by kenneth) patch v1 (3.34 KB, patch)
2010-05-11 11:48 PDT, Antonio Gomes
webkit-ews: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Gomes 2010-05-11 11:39:28 PDT
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;
  page->webView = webView;

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

  view.setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
  view.resize(600, 480);


"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.
Comment 1 Kenneth Rohde Christiansen 2010-05-11 11:45:19 PDT
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.
Comment 2 Antonio Gomes 2010-05-11 11:48:42 PDT
Created attachment 55728 [details]
(committed in r59163, reviewed by kenneth) patch v1
Comment 3 Antonio Gomes 2010-05-11 11:50:37 PDT
(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.
Comment 4 Kenneth Rohde Christiansen 2010-05-11 11:53:21 PDT
Comment on attachment 55728 [details]
(committed in r59163, reviewed by kenneth) patch v1

Comment 5 Antonio Gomes 2010-05-11 12:21:59 PDT
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>
Comment 6 Simon Hausmann 2010-05-14 01:19:19 PDT
Revision r59163 cherry-picked into qtwebkit-2.0 with commit 0a778f7ce0f8c905339b884030b48d93d5b90b7c
Comment 7 Early Warning System Bot 2011-05-05 18:04:07 PDT
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