|Summary:||[GTK] Initial view loading is slow|
|Product:||WebKit||Reporter:||Cédric Bellegarde <cedric.bellegarde>|
|Severity:||Normal||CC:||achristensen, bugs-noreply, cdumez, cgarcia, mike, svillar, thorton|
Description Cédric Bellegarde 2019-09-04 01:46:13 PDT
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.
Comment 1 Michael Gratton 2019-09-04 15:57:42 PDT
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?
Comment 2 Michael Gratton 2019-09-04 23:43:25 PDT
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?
Comment 3 Cédric Bellegarde 2019-09-05 02:18:11 PDT
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.
Comment 4 Carlos Garcia Campos 2019-09-05 02:25:19 PDT
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.
Comment 5 Cédric Bellegarde 2019-09-05 02:41:47 PDT
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
Comment 6 Carlos Garcia Campos 2019-09-05 03:50:36 PDT
(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?
Comment 7 Cédric Bellegarde 2019-09-05 04:35:35 PDT
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()
Comment 8 Carlos Garcia Campos 2019-09-05 04:47:48 PDT
Why are you using an idle source to load the uri?
Comment 9 Cédric Bellegarde 2019-09-05 04:48:34 PDT
Oops, uploaded wrong version, was just some testing. You can remove the idle_add().
Comment 10 Carlos Garcia Campos 2019-09-05 04:52:28 PDT
I can't reproduce it anyway with the test case.
Comment 11 Carlos Garcia Campos 2019-09-05 04:55:10 PDT
Ok, I can reproduce it now, without the idle
Comment 12 Carlos Garcia Campos 2019-09-06 05:01:45 PDT
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.
Comment 14 Carlos Garcia Campos 2019-09-06 05:40:31 PDT
Created attachment 378184 [details] Rebased patch
Comment 15 Cédric Bellegarde 2019-09-07 05:48:28 PDT
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).
Comment 16 Carlos Garcia Campos 2019-09-09 01:24:00 PDT
(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
Comment 17 Cédric Bellegarde 2019-09-09 01:39:23 PDT
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.
Comment 18 Sergio Villar Senin 2019-09-09 04:14:35 PDT
Comment on attachment 378184 [details] Rebased patch Looks good from my side. Let's wait for the ack of an owner
Comment 19 Carlos Garcia Campos 2019-09-11 03:45:49 PDT
(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?
Comment 20 Carlos Garcia Campos 2019-09-16 00:53:58 PDT
Ping owners, please.
Comment 21 Chris Dumez 2019-09-16 08:25:55 PDT
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.
Comment 22 Carlos Garcia Campos 2019-09-17 01:41:49 PDT
(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.
Comment 23 Carlos Garcia Campos 2019-09-17 01:58:08 PDT
Created attachment 378950 [details] Patch for landing