WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
201451
[GTK] Initial view loading is slow
https://bugs.webkit.org/show_bug.cgi?id=201451
Summary
[GTK] Initial view loading is slow
Cédric Bellegarde
Reported
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.
Attachments
Python browser with issue
(1.17 KB, text/x-python)
2019-09-05 04:35 PDT
,
Cédric Bellegarde
no flags
Details
Patch
(6.41 KB, patch)
2019-09-06 05:06 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Rebased patch
(6.34 KB, patch)
2019-09-06 05:40 PDT
,
Carlos Garcia Campos
svillar
: review+
cdumez
: commit-queue-
Details
Formatted Diff
Diff
Example using a Gtk revealer to show webview
(1.40 KB, text/x-python)
2019-09-09 01:39 PDT
,
Cédric Bellegarde
no flags
Details
Patch for landing
(10.38 KB, patch)
2019-09-17 01:58 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Michael Gratton
Comment 1
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?
Michael Gratton
Comment 2
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?
Cédric Bellegarde
Comment 3
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.
Carlos Garcia Campos
Comment 4
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.
Cédric Bellegarde
Comment 5
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
Carlos Garcia Campos
Comment 6
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?
Cédric Bellegarde
Comment 7
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()
Carlos Garcia Campos
Comment 8
2019-09-05 04:47:48 PDT
Why are you using an idle source to load the uri?
Cédric Bellegarde
Comment 9
2019-09-05 04:48:34 PDT
Oops, uploaded wrong version, was just some testing. You can remove the idle_add().
Carlos Garcia Campos
Comment 10
2019-09-05 04:52:28 PDT
I can't reproduce it anyway with the test case.
Carlos Garcia Campos
Comment 11
2019-09-05 04:55:10 PDT
Ok, I can reproduce it now, without the idle
Carlos Garcia Campos
Comment 12
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.
Carlos Garcia Campos
Comment 13
2019-09-06 05:06:39 PDT
Created
attachment 378183
[details]
Patch
Carlos Garcia Campos
Comment 14
2019-09-06 05:40:31 PDT
Created
attachment 378184
[details]
Rebased patch
Cédric Bellegarde
Comment 15
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).
Carlos Garcia Campos
Comment 16
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
Cédric Bellegarde
Comment 17
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.
Sergio Villar Senin
Comment 18
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
Carlos Garcia Campos
Comment 19
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?
Carlos Garcia Campos
Comment 20
2019-09-16 00:53:58 PDT
Ping owners, please.
Chris Dumez
Comment 21
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.
Carlos Garcia Campos
Comment 22
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.
Carlos Garcia Campos
Comment 23
2019-09-17 01:58:08 PDT
Created
attachment 378950
[details]
Patch for landing
Carlos Garcia Campos
Comment 24
2019-09-17 04:05:07 PDT
Committed
r249953
: <
https://trac.webkit.org/changeset/249953
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug