search

Igalia / wolvic

A fast and secure browser for standalone virtual-reality and augmented-reality headsets.
https://wolvic.org
Mozilla Public License 2.0
796 stars 100 forks source link

Remove reliance of onFirstContentfulPaint #1207

Closed HollowMan6 closed 6 months ago

HollowMan6 commented 8 months ago

There seems to be some bugs/breaking changes (?) related to onFirstContentfulPaint in Gecko 121.0.1, this PR tries to mitigate this issue by setFirstPaint each time when we open a new window/tab, and remove mFirstContentfulPaint check when we captureBitmap, so that the bitmap in tabs list can be properly shown.

com igalia wolvic-20240127-162024

svillar commented 8 months ago

I've some questions

  1. When does it happen on the first ever launch of Wolvic?
  2. What's the problem with OnFirstContentfulPaint? That we no longer receive the event?
HollowMan6 commented 8 months ago
  1. When does it happen on the first ever launch of Wolvic?

Just open Wolvic and you will see an empty page like the PR description. We can only see the page content after we press the reload button/home page button (because of the caches. If you enable Bypass cache on reload, then it will keep the empty page even you press the reload button).

com igalia wolvic-20240129-191156

You can more reliably reproduce this by disabling restore tabs and windows after restart in privacy & security settings, then we will have this issue for every launch.

  1. What's the problem with OnFirstContentfulPaint? That we no longer receive the event?

Yes, but actually it only happens on some of the pages when you make them as home page (Wolvic's default one is included), https://www.google.com seems not included, so it looks like a bug in Gecko 121 (maybe somehow they accidentally introduced a timeout that stops sending OnFirstContentfulPaint if the page loads too long in Gecko?)

I'm sure it's a bug not caused at Wolvic side since you can still reproduce the same situation even if you revert to the parent commit of https://github.com/Igalia/wolvic/commit/89f1df3236283ce704cf12aeb8ac8383fdbc60e2 while still use Gecko 121 substitution.

But in any case, I don't think we need to rely on OnFirstContentfulPaint here to display, so here I just remove those reliance.

svillar commented 8 months ago
  1. What's the problem with OnFirstContentfulPaint? That we no longer receive the event?

Yes, but actually it only happens on some of the pages when you make them as home page (Wolvic's default one is included), https://www.google.com seems not included, so it looks like a bug in Gecko 121 (maybe somehow they accidentally introduced a timeout that stops sending OnFirstContentfulPaint if the page loads too long in Gecko?)

Just to confirm, do you mean that the event is not received for some pages? Wolvic's default one can be likely fully cached so it looks to me that the event is not received if we load the page from cache?

HollowMan6 commented 8 months ago
  1. What's the problem with OnFirstContentfulPaint? That we no longer receive the event?

Yes, but actually it only happens on some of the pages when you make them as home page (Wolvic's default one is included), https://www.google.com seems not included, so it looks like a bug in Gecko 121 (maybe somehow they accidentally introduced a timeout that stops sending OnFirstContentfulPaint if the page loads too long in Gecko?)

Just to confirm, do you mean that the event is not received for some pages? Wolvic's default one can be likely fully cached so it looks to me that the event is not received if we load the page from cache?

I believe we will lose the cache each time we restart Wolvic (with restore tabs and windows after restart disabled), so it's actually the case that we don't receive the event on some pages without cache. We still receive the event when we load with cache.

svillar commented 8 months ago
  1. What's the problem with OnFirstContentfulPaint? That we no longer receive the event?

Yes, but actually it only happens on some of the pages when you make them as home page (Wolvic's default one is included), https://www.google.com seems not included, so it looks like a bug in Gecko 121 (maybe somehow they accidentally introduced a timeout that stops sending OnFirstContentfulPaint if the page loads too long in Gecko?)

Just to confirm, do you mean that the event is not received for some pages? Wolvic's default one can be likely fully cached so it looks to me that the event is not received if we load the page from cache?

I believe we will lose the cache each time we restart Wolvic (with restore tabs and windows after restart disabled), so it's actually the case that we don't receive the event on some pages without cache.

No the HTML cache of the web engine has nothing to do with the restore tabs feature from Wolvic. The restore session is just a file saved by wolvic with information about which tabs are opened. The HTML cache is persistent and it's managed by the engine.

HollowMan6 commented 8 months ago

No the HTML cache of the web engine has nothing to do with the restore tabs feature from Wolvic. The restore session is just a file saved by wolvic with information about which tabs are opened.

Actually I mean, we directly save SessionState into windows_state.json, which works like some kind of cache already, it's not about HTML cache. https://github.com/Igalia/wolvic/blob/b62cb7e7143880336a294837683f1ab7b64b6e80/app/src/common/shared/com/igalia/wolvic/ui/widgets/Windows.java#L219-L231

The HTML cache is persistent and it's managed by the engine.

Since we failed to have the OnFirstContentfulPaint event without caches, I do believe we lose the cache somehow after the restart in Wolvic, otherwise, it won't explain , with restore tabs and windows after restart disabled, why we still can't have the home page displayed even after the fresh launch, while it will still work when we click the reload button. Also in https://github.com/Igalia/wolvic/issues/353#issuecomment-1716482605, we believe it's caused by cache, and it will work again after we restart wolvic.

Maybe we have other bugs here in Wolvic related to cache persistence as well? Sounds similar to #1199 (although that should be related to cookies)

svillar commented 8 months ago

I've uploaded an alternative solution to #1217. It'd be nice to test it to check whether it fixes the issues we've seen.

The key here, is that Wolvic's start page is fully cacheable, which is nice because we can show it blazingly fast, but the counterpart is that it does not emit FCP. Actually the problem is what you mentioned, we should not rely on FCP at all, but that would involve larger changes that we can eventually make in the future.

HollowMan6 commented 7 months ago

Let's convert this into draft for a moment https://github.com/Igalia/wolvic/pull/1217#pullrequestreview-1857078052

HollowMan6 commented 6 months ago

Close this for now. Feel free to re-open if anyone wants to continue working on this.