Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Synthesize FCP for cached pages" #1641

Merged
merged 2 commits into from
Nov 22, 2024
Merged

Conversation

svillar
Copy link
Member

@svillar svillar commented Nov 20, 2024

This reverts commit aae8014.

This workaround was added after the update to Gecko 121. The issues with first contentful paint are not here anymore so we can remove it and avoid synthesizing FCP events at the wrong time which were causing issues when taking screenshots of the web contents.

When those generated FCPs were issued, the Gecko compositor was not in a ready state to generate a screenshot and thus an exception was generated. Gecko was not handling it properly so it was affecting subsequent JNI calls causing crashes and hangs in Gecko.

Fixes #1638

This reverts commit aae8014.

This workaround was added after the update to Gecko 121. The issues
with first contentful paint are not here anymore so we can remove
it and avoid synthesizing FCP events at the wrong time which were
causing issues when taking screenshots of the web contents.

When those generated FCPs were issued, the Gecko compositor was
not in a ready state to generate a screenshot and thus an exception
was generated. Gecko was not handling it properly so it was
affecting subsequent JNI calls causing crashes and hangs in Gecko.

Fixes #1638
Copy link
Member

@javifernandez javifernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change fixes the issue, thanks.

However, since it removed the white screen shown while the webcontent is being loaded, it gives now the impression of slowness or unresponsiveness. Maybe we can do something about it in a follow up patch.

@svillar
Copy link
Member Author

svillar commented Nov 20, 2024

Hmm I've tried it with slow connections and the issue that the original patch was fixing reappeared, i.e., starting Wolvic sometimes results in an empty web view.

The FCP event is not emitted by GeckoView if the page is loaded from the cache
(either HTTP or back/forward). As a result many things are not properly set up
in Wolvic, like the current active session, resulting in many cases in web
contents not visible on app start.

In those cases we should generate and emit it to get things working.
@svillar svillar merged commit ec4a8b7 into main Nov 22, 2024
22 checks passed
@svillar svillar deleted the capture_image_exception branch November 22, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Meta] No web contents when adding new windows in v72
2 participants