[GTK] Initial view loading is slow

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Cédric Bellegarde]

Version: webkit2gtk3-2.25.92

Tested on Fedora 31 and ArchLinux.

WebKit2 2.25.92 initial view loading is slow (epiphany, eolie, geary, ...)

It is really visible in Geary (3.33.92 on Fedora and 3.32.2 on ArchLinux) where loading a mail hangs for some seconds.

Looks like a regression.

[Michael Gratton]

Confirming, also an issue with 2.25.4 in current Ubuntu Eoan and the current nightly flatpak build.

It's potentially quite visible in Geary because the WebKitWebView is a child of a GtkRevealer, which animates the height of the WebView. So maybe a layout/rendering issue?

But also potentially a delay on WebKitWebContext construction, since Geary currently constructs a new one for each conversation.

I'm thinking maybe the latter since I can't seem to see the delay opening a new tab in Ephy, but can catch it opening a new incognito window, and even new tabs in incognito windows - notice how the text caret in the location bar freezes blinking for a second when opening a new incognito tab?

[Michael Gratton]

Actually, seems I mis-remembered how it works, Geary actually constructs a single shared WebContext at startup and uses that, so perhaps the delay is in spawning the first WebProcess for a context?

[Cédric Bellegarde]

Here what happens in Eolie:

First test:
- Launch eolie => load google.com
- Open a new tab => slow => load google.com
- Click on any link in google page
- Open a new tab => fast => load google.com

So, doing some navigation seems to fix the issue.

Second test:
- Modify Eolie to use a new context for every view
- Doing some navigation does not fix the issue


So, looks it's an issue with WebKitWebContext initial state.

[Carlos Garcia Campos]

I don't see any delay in ephy incognito windows. We used to spawn the web process when the web page was created, but now it's delayed until the first load. That might be the cause, but I can't reproduce it.

[Cédric Bellegarde]

Can reproduce with epihany here: https://youtu.be/xr0OmMqq8g8

Fedora 31 with epiphany-3.33.92-1.fc31.x86_64

When I click on new tab button, there is a one second lag (like in Eolie)

Launching stable epiphany from flathub => No lag

[Carlos Garcia Campos]

(In reply to Cédric Bellegarde from comment #5)
> Can reproduce with epihany here: https://youtu.be/xr0OmMqq8g8

It doesn't happen here :-(

> Fedora 31 with epiphany-3.33.92-1.fc31.x86_64
> 
> When I click on new tab button, there is a one second lag (like in Eolie)
> 
> Launching stable epiphany from flathub => No lag

Does it happen with MiniBrowser too?

[Cédric Bellegarde]

Created attachment 378075 [details]
Python browser with issue

MiniBrowser does not load an URI by default so impossible to reproduce.

Here a small python example.

Click on WebKitGTK button => lag
Click on Empty button => no log

So, looks related to load_uri()

[Carlos Garcia Campos]

Why are you using an idle source to load the uri?

[Cédric Bellegarde]

Oops, uploaded wrong version, was just some testing. You can remove the idle_add().

[Carlos Garcia Campos]

I can't reproduce it anyway with the test case.

[Carlos Garcia Campos]

Ok, I can reproduce it now, without the idle

[Carlos Garcia Campos]

The problem is that now we are always calling DrawingAreaProxy::waitForBackingStoreUpdateOnNextPaint() after a new process is launched and we used to do that only when launching a new process after a crash. This makes m_hasReceivedFirstUpdate useless, because it's always set to true right after a process is launched. Then, we wait up to half a second (which is usually the case for the initial load) until the first update. We only want to do that when recovering from a crash or when swapping processes to avoid flashing effect.

[Carlos Garcia Campos]

Created attachment 378183 [details]
Patch

[Carlos Garcia Campos]

Created attachment 378184 [details]
Rebased patch

[Cédric Bellegarde]

Can confirm it fixed the issue in Eolie.

For Geary, it's really better but there is a small delay when revealing a message (was not present with WebKitGTK 2.24).

[Carlos Garcia Campos]

(In reply to Cédric Bellegarde from comment #15)
> Can confirm it fixed the issue in Eolie.
> 
> For Geary, it's really better but there is a small delay when revealing a
> message (was not present with WebKitGTK 2.24).

I would need a way to reproduce it

[Cédric Bellegarde]

Created attachment 378356 [details]
Example using a Gtk revealer to show webview

Ok, does not happen with this simple example so issue looks to be in Geary.

[Sergio Villar Senin]

Comment on attachment 378184 [details]
Rebased patch

Looks good from my side. Let's wait for the ack of an owner

[Carlos Garcia Campos]

(In reply to Sergio Villar Senin from comment #18)
> Comment on attachment 378184 [details]
> Rebased patch
> 
> Looks good from my side. Let's wait for the ack of an owner

Chris?

[Carlos Garcia Campos]

Ping owners, please.

[Chris Dumez]

Comment on attachment 378184 [details]
Rebased patch

View in context: https://bugs.webkit.org/attachment.cgi?id=378184&action=review

> Source/WebKit/UIProcess/WebPageProxy.h:1747
> +        InitialProcess,

You may need to make sure the current process is the dummy one to determine that it is really the initial process launch.

> Source/WebKit/UIProcess/WebPageProxy.h:1749
> +        Reload

I think this should be "Crash", reloading is not a reason for launching a new process unless the previous process has crashed.

> Source/WebKit/UIProcess/WebPageProxy.h:1752
> +    void launchProcess(const WebCore::RegistrableDomain&, ProcessLaunchReason = ProcessLaunchReason::InitialProcess);

Please do not specify a default parameter value and force call sites to specify the reason.

[Carlos Garcia Campos]

(In reply to Chris Dumez from comment #21)
> Comment on attachment 378184 [details]
> Rebased patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=378184&action=review
> 
> > Source/WebKit/UIProcess/WebPageProxy.h:1747
> > +        InitialProcess,
> 
> You may need to make sure the current process is the dummy one to determine
> that it is really the initial process launch.

Well, it doesn't matter here if it's a delayed process launch.

> > Source/WebKit/UIProcess/WebPageProxy.h:1749
> > +        Reload
> 
> I think this should be "Crash", reloading is not a reason for launching a
> new process unless the previous process has crashed.

Ok, makes sense, I followed the logic of WebPageProxy::launchProcessForReload

> > Source/WebKit/UIProcess/WebPageProxy.h:1752
> > +    void launchProcess(const WebCore::RegistrableDomain&, ProcessLaunchReason = ProcessLaunchReason::InitialProcess);
> 
> Please do not specify a default parameter value and force call sites to
> specify the reason.

Ok.

[Carlos Garcia Campos]

Created attachment 378950 [details]
Patch for landing

[Carlos Garcia Campos]

Committed r249953: <https://trac.webkit.org/changeset/249953>