Bug 164180 – [GTK] No way to safely use webkit_web_view_get_snapshot() as pages are not rendered when LOAD_FINISHED is called

• Status: NEW
• Component: WebKitGTK

[Michael Catanzaro]

Currently there is no guarantee that pages are actually rendered at the time that LOAD_FINISHED is emitted, and there is no signal to indicate that a page has been rendered, so there is never any safe time to use webkit_web_view_get_snapshot(). In Epiphany we use a hardcoded timeout (0.2 or 0.5 seconds or so) to decide when to call the snapshot API as a workaround for this. Otherwise there is a chance that the returned snapshot would just be a white background.

Preferably LOAD_FINISHED would not be emitted until the page has actually been rendered. Alternatively, we could add a new signal to indicate when it is safe to take a snapshot.

[Carlos Garcia Campos]

I don't think we should change the semantics of LOAD_FINISHED, we should probably expose APILoaderClient::didReachLayoutMilestone, and the applications could wait for DidFirstVisuallyNonEmptyLayout or DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the milestones.

[Carlos Alberto Lopez Perez]

(In reply to comment #1)
> I don't think we should change the semantics of LOAD_FINISHED, we should
> probably expose APILoaderClient::didReachLayoutMilestone, and the
> applications could wait for DidFirstVisuallyNonEmptyLayout or
> DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the
> milestones.

Why you think we should not change when LOAD_FINISHED triggers?
What is the use case of having a LOAD_FINISHED signal that doesn't trigger always when the page is fully rendered??

[Carlos Garcia Campos]

(In reply to comment #2)
> (In reply to comment #1)
> > I don't think we should change the semantics of LOAD_FINISHED, we should
> > probably expose APILoaderClient::didReachLayoutMilestone, and the
> > applications could wait for DidFirstVisuallyNonEmptyLayout or
> > DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the
> > milestones.
> 
> Why you think we should not change when LOAD_FINISHED triggers?
> What is the use case of having a LOAD_FINISHED signal that doesn't trigger
> always when the page is fully rendered??

Because load finished is not about rendering but about networking, the page finished loading, but not necessarily rendering. That's why there are other callbacks more related to rendering like the ones I mentioned before.

[Carlos Alberto Lopez Perez]

(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > I don't think we should change the semantics of LOAD_FINISHED, we should
> > > probably expose APILoaderClient::didReachLayoutMilestone, and the
> > > applications could wait for DidFirstVisuallyNonEmptyLayout or
> > > DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the
> > > milestones.
> > 
> > Why you think we should not change when LOAD_FINISHED triggers?
> > What is the use case of having a LOAD_FINISHED signal that doesn't trigger
> > always when the page is fully rendered??
> 
> Because load finished is not about rendering but about networking, the page
> finished loading, but not necessarily rendering. That's why there are other
> callbacks more related to rendering like the ones I mentioned before.

I think that this is an internal implementation detail that don't necessarily has to be exposed in our API

Quoting our docs:

" WEBKIT_LOAD_FINISHED This state means that everything that was required to display the page has been loaded. "

https://webkitgtk.org/reference/webkitgtk/stable/WebKitWebFrame.html#WebKitLoadStatus

And after reading that twice, I would expect that when LOAD_FINISHED triggers the page is always fully rendered.

Adding a new state is an option, but I think is a bad idea to do it, if we only add it for the sake of exposing the internal implementation.

So, what is the use case for an application to have both WEBKIT_LOAD_FINISHED and WEBKIT_LOAD_FINISHED_AND_RENDERED_COMPLETED ?

[Carlos Alberto Lopez Perez]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (In reply to comment #1)
> > > > I don't think we should change the semantics of LOAD_FINISHED, we should
> > > > probably expose APILoaderClient::didReachLayoutMilestone, and the
> > > > applications could wait for DidFirstVisuallyNonEmptyLayout or
> > > > DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the
> > > > milestones.
> > > 
> > > Why you think we should not change when LOAD_FINISHED triggers?
> > > What is the use case of having a LOAD_FINISHED signal that doesn't trigger
> > > always when the page is fully rendered??
> > 
> > Because load finished is not about rendering but about networking, the page
> > finished loading, but not necessarily rendering. That's why there are other
> > callbacks more related to rendering like the ones I mentioned before.
> 
> I think that this is an internal implementation detail that don't
> necessarily has to be exposed in our API
> 
> Quoting our docs:
> 
> " WEBKIT_LOAD_FINISHED This state means that everything that was required to
> display the page has been loaded. "
> 
> https://webkitgtk.org/reference/webkitgtk/stable/WebKitWebFrame.
> html#WebKitLoadStatus

Sorry, I got the doc from the webkit1 API by mistake. The one for WK2:

WEBKIT_LOAD_FINISHED Load completed. All resources are done loading or there was an error during the load operation.

https://webkitgtk.org/reference/webkit2gtk/stable/WebKitWebView.html#WebKitLoadEvent

My understanding of what that means remains the same.

[Carlos Garcia Campos]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (In reply to comment #1)
> > > > I don't think we should change the semantics of LOAD_FINISHED, we should
> > > > probably expose APILoaderClient::didReachLayoutMilestone, and the
> > > > applications could wait for DidFirstVisuallyNonEmptyLayout or
> > > > DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the
> > > > milestones.
> > > 
> > > Why you think we should not change when LOAD_FINISHED triggers?
> > > What is the use case of having a LOAD_FINISHED signal that doesn't trigger
> > > always when the page is fully rendered??
> > 
> > Because load finished is not about rendering but about networking, the page
> > finished loading, but not necessarily rendering. That's why there are other
> > callbacks more related to rendering like the ones I mentioned before.
> 
> I think that this is an internal implementation detail that don't
> necessarily has to be exposed in our API
> 
> Quoting our docs:
> 
> " WEBKIT_LOAD_FINISHED This state means that everything that was required to
> display the page has been loaded. "

Everything required to display doesn't mean it has displayed eferything, only that the resources are there, but you have to parse, layout, paint, etc.

> https://webkitgtk.org/reference/webkitgtk/stable/WebKitWebFrame.
> html#WebKitLoadStatus
> 
> And after reading that twice, I would expect that when LOAD_FINISHED
> triggers the page is always fully rendered.
> 
> Adding a new state is an option, but I think is a bad idea to do it, if we
> only add it for the sake of exposing the internal implementation.
> 
> So, what is the use case for an application to have both
> WEBKIT_LOAD_FINISHED and WEBKIT_LOAD_FINISHED_AND_RENDERED_COMPLETED ?

I don't think it's possible to know when a page has finished rendering, you can know if something has rendered, or if there was a layout, etc. But, if page has dynamic contents or animations. Load events has to do with the load, what happens in WebCore/loader, not with the layout or render. If we want to expose notifications about those I would not add new load events, but a different signal or whatever

[Tomas Popela]

I didn't know about that bug and I created bug 168830. The thing is basically the same. We should fix webkit_web_view_get_snapshot() and printing to internally wait until the layout has finished if possible. As Michael said there is a nasty workaround for webkit_web_view_get_snapshot(), but that one could not be applied for the situation in the bug 168830.

[Tomas Popela]

*** Bug 168830 has been marked as a duplicate of this bug. ***

[Michael Catanzaro]

Same problem exists with zoom, there's no way for the browser to know the right time to call webkit_web_view_set_zoom_level().

[Adrian Perez]

When porting the GIMP's “web-page” plug-in to use modern WebKitGTK+
we have hit this issue as well, as per the comments at:

   https://gitlab.gnome.org/GNOME/gimp/merge_requests/15

It seems that most tools out there are artificially adding a timer
delay before attempting to save snapshots, like for example the
“webkit-capture” Python script which I have used sometimes:

   https://github.com/nud/webkit-capture/blob/e67858eef6265050ffa36644e2ac4b00862538fc/webkit-capture#L158-L160

Having this solved would be nice. I agree that conceptually it does
not make sense to use the WebKitWebView::load-changed signal to notify
about layout completion, because this one is about *network* activity.
Wouldn't it be acceptable to have a new WebKitWebView::layout-completed
signal? Adding a signal would not break the API/ABI, which would remain
backwards-compatible.

[Michael Catanzaro]

(In reply to Adrian Perez from comment #10)
> Wouldn't it be acceptable to have a new WebKitWebView::layout-completed
> signal? Adding a signal would not break the API/ABI, which would remain
> backwards-compatible.

That might be safer than exposing all the details of APILoaderClient::didReachLayoutMilestone.