Bug 246330 – [GTK][WPE] Add heartbeat mechanism to check for web process responsiveness

• Status: NEW
• Component: WebKitGTK

[Przemyslaw Gorszkowski]

The idea bases on: https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/866/ 

The new api method should be named webkit_web_view_check_web_process_responsiveness and if the process is not responsive, the appropriate signal should be emitted.

[Michael Catanzaro]

This already exists, WebKitWebView:is-web-process-responsive. You can connect to the notify::is-web-process-responsive if you need a signal. But you know that already. I see you're trying to work around a problem with that property by adding a new method webkit_web_view_is_web_process_responsive_async() that forces the UI process to immediately check if the web process is responsive. But it's quite confusing if the method does something different than returning the value of the corresponding property.

It seems your problem is that if zero messages being sent between UI process and web process, then unresponsiveness will not be detected. I did not realize it was ever possible to wind up in such a situation, but since you're hitting it, how about adding a heartbeat message? That would solve your problem without requiring any API changes, right?

P.S. What are you planning to do with webkit_web_view_get_web_process_identifier()? Is webkit_web_view_terminate_web_process() not good enough for what you want to do? My worry is that developers will be confused when the pid changes after a process swap.

[Miguel Gomez]

With the system that we currently have working, we can only detect that the process has became unresponsive while there are API calls or user input. For the cases when there's none of those (like when watching a video without interacting with the page, for example), we can't detect if the process becomes unresponsive.

We tried to talk to Apple to add an internal timer that would ping every 10 seconds if there was no API call or user event (the heartbeat as you mention), but they didn't show any interest on it, so it's a problem that still needs to be addressed.

That's why we're trying to add this new API method so the application can manually ping the webprocess. Don't pay too much attention to the PR that the idea is based on, as the implementation will be different. The idea is just to add a new API method webkit_web_view_check_web_process_responsiveness() without any parameter. This method will internally ping the webprocess, and that ping will already activate the is-web-process-responsive signal if the webprocess doesn't answer. We're not adding anything else. We don't need it to be async (the reply will come as the signal), and we don't need webkit_web_view_get_web_process_identifier() as we already have webkit_web_view_terminate_web_process(), as you mention.

[Michael Catanzaro]

Well the proposal seems OK, but... seems even better, and easier, to implement a heartbeat, unless there is a really good reason not to? Expecting applications to handle this manually doesn't seem very friendly. Besides, as long as WebKitWebView:is-web-process-responsive exists, it ought to return accurate results, right? And if you don't add new API, then you don't have to test the API that you didn't add, and you also skip the new API discussion/approval. I'm curious what other reviewers think.

It's probably best to have a heartbeat at the WebProcessProxy level, but if Apple doesn't want it there (why not?), then putting it in the API layer sounds OK. It could be as simple as (a) take whatever work you've done already, (b) make it private instead of public, (c) call it every few seconds on a timer.

For GTK, we could additionally restrict it so that it only runs if the web view is focused, so we're not wasting battery life checking web views that aren't visible. (I guess WPE has no way to check for focus, though.)

[Przemyslaw Gorszkowski]

Thanks Michael for your suggestions, I think that idea with "heartbeat" mechanism on our side sounds better than expecting such mechanism on application side. I am wondering how and when this mechanism should be started and stopped on API level (WebKitWebViewGtk.cpp, WbKitWebViewWpe.cpp), do you have any suggestions? I am thinking to use for that ResponsivenessTimer - or something similar.

[Michael Catanzaro]

(In reply to Przemyslaw Gorszkowski from comment #4)
> I am wondering how and when this mechanism should be
> started and stopped on API level (WebKitWebViewGtk.cpp,
> WbKitWebViewWpe.cpp), do you have any suggestions? I am thinking to use for
> that ResponsivenessTimer - or something similar.

It seems ResponsivenessTimer is how WebKitWebView:is-web-process-responsive is already implemented. Each AuxiliaryProcessProxy has a ResponsivenessTimer. WebProcessProxy inherits from AuxiliaryProcessProxy and uses that to call NavigationClient::processDidBecomeUnresponsive. And WebKitWebView installs WebKitNavigationClient, which calls webkitWebViewSetIsWebProcessResponsive.

So there's already a ResponsivenessTimer to use for this. I don't think we need a second one. Just need to make sure that AuxiliaryProcessProxy's timer is actually running. The code that decides whether to run the ResponsivenessTimer is scattered throughout WebPageProxy.cpp. It looks like the code runs the timer when (a) loading a page, or (b) processing user input, and stops the timer (a) to avoid it running during nested run loops, (b) after the user input has been processed, and also (c) in WebPageProxy::dispatchActivityStateChange. This design makes sense based on the assumption that if the user is not doing anything, it doesn't matter whether the process is responsive, so it's OK to check responsiveness only in response to user input. Now, if a video is playing, no user interaction is expected, but you still need to know if the process hangs so you can restart it and resume the video. On a desktop or phone, it's probably OK to fail to detect if the video has frozen, because the user will undoubtedly click or press when it freezes and that will start the timer and detect the problem. But on your devices, that assumption does not work: you need to restart even if there is no user interaction, yes?

So, this turned out to be more complicated than I was expecting. Maybe your original proposal really is the simplest option: just add a new API that will immediately update the is-web-process-responsive property without requiring user interaction, and call it regularly from your application, exactly as you originally proposed. Another option would be to change the WebPageProxy to have two different modes: the current mode, and a new mode that always checks responsiveness instead of only checking during user input. Problem is, if the view is not visible, that wastes battery life, so we'd need to add a way for WebKitWebView to tell WebPageProxy whether it is visible or not... or the API user could be responsible for handling that by telling the responsiveness timer when to start and stop. But then that requires *two* new API functions, one to start the timer and one to stop it! Finally, we could convince Apple to just run the timer always, but if it's not needed for devices where easy user input is expected, maybe we shouldn't?

Perhaps I was wrong to dismiss your original proposal so quickly. I wonder what other WPE/GTK reviewers would prefer.

[Miguel Gomez]

The good part of adding the new API is that the change is simple and it's up to the application whether it wants to perform the ping or not.

On the other hand, the heartbeat idea makes things simpler for the app as it doesn't have to do anything. On a fast look, I think this could be implemented for GTK and WPE by adding a timer to the glib WebKitWebView that calls WebProcessProxy::isResponsive every some seconds. That should be fairly simple. We could even add a setting for enabling and disabling this timer if we want to. 

Regarding visibility, I'm not sure whether the visibility flag is accessible from WebKitWebView, but they can be taken into consideration to start/stop the timer as needed (not sure whether the visibility flags are accessible from WebKitWebView, maybe we need to work with WebKitWebViewGTK and WebKitWebViewWPE).

I initially thought of the new API to fix this, but if the heartbeat is not hard to implement I think it would be a better approach.

[Michael Catanzaro]

(In reply to Miguel Gomez from comment #6)
> Regarding visibility, I'm not sure whether the visibility flag is accessible
> from WebKitWebView,

It is, that's gtk_widget_is_visible(). (For WPE, I guess we have to assume everything is always visible, unless libwpe has API for that.)

[Carlos Garcia Campos]

WebPageProxy::isViewVisible()

[Adrian Perez]

(In reply to Michael Catanzaro from comment #7)
> (In reply to Miguel Gomez from comment #6)
> > Regarding visibility, I'm not sure whether the visibility flag is accessible
> > from WebKitWebView,
> 
> It is, that's gtk_widget_is_visible(). (For WPE, I guess we have to assume
> everything is always visible, unless libwpe has API for that.)

Applications which embed WPE web views can use the wpe_view_activity_state
flags, see:

  https://github.com/WebPlatformForEmbedded/libwpe/blob/1.14.0/include/wpe/view-backend.h#L157-L190

[Adrian Perez]

(In reply to Miguel Gomez from comment #6)

> The good part of adding the new API is that the change is simple and it's up
> to the application whether it wants to perform the ping or not.

OTOH adding new API means that then we have to maintain it 😅️

[Carlos Garcia Campos]

Maybe we can start the heartbeat timer when media plays start and stop it when it finishes. We could do it in WebPageProxy::updatePlayingMediaDidChange

[Przemyslaw Gorszkowski]

(In reply to Carlos Garcia Campos from comment #11)
> Maybe we can start the heartbeat timer when media plays start and stop it
> when it finishes. We could do it in WebPageProxy::updatePlayingMediaDidChange

That is what I was thinking of after Michael comment(2022-10-14 05:55:14 PDT). The question is: does this cover all cases for which want to have this heartbeat process responsiveness mechanism.

[Miguel Gomez]

I don't think enabling the timer just when the video plays is enough. There can be situations where there's no user interaction but no video is playing, and we should check for responsiveness in that case too. Also, I'm not sure whether pausing/resuming a video would trigger WebPageProxy::updatePlayingMediaDidChange.

Ideally we should intercept changes in the activity state flags, and disable the timer only when the page is not visible or not isInWindow.

[Miguel Gomez]

(In reply to Miguel Gomez from comment #13)
> I don't think enabling the timer just when the video plays is enough. There
> can be situations where there's no user interaction but no video is playing,
> and we should check for responsiveness in that case too. Also, I'm not sure
> whether pausing/resuming a video would trigger
> WebPageProxy::updatePlayingMediaDidChange.
> 
> Ideally we should intercept changes in the activity state flags, and disable
> the timer only when the page is not visible or not isInWindow.

On a second thought, there's the possibility that the browser is playing video while the window is not visible, so that's a case we should handle as well.

[Michael Catanzaro]

(In reply to Carlos Garcia Campos from comment #11)
> Maybe we can start the heartbeat timer when media plays start and stop it
> when it finishes. We could do it in WebPageProxy::updatePlayingMediaDidChange

But note there are a lot of reasons why WebPageProxy needs to temporarily disable its ResponsivenessTimer, and it doesn't restart the timer again afterwards. It's really built on the assumption that it's testing responsiveness to a specific user input. So it would need to use a separate ResponsivenessTimer.

[Przemyslaw Gorszkowski]

Pull request: https://github.com/WebKit/WebKit/pull/5581

[Philippe Normand]

Is this related/dupe of bug 224862?