Bug 265990 - [GTK][WPE] Random incorrect image displayed as the background of a div
Summary: [GTK][WPE] Random incorrect image displayed as the background of a div
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Miguel Gomez
URL:
Keywords:
: 256442 (view as bug list)
Depends on:
Blocks:
 
Reported: 2023-12-07 02:18 PST by Miguel Gomez
Modified: 2023-12-25 08:33 PST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Miguel Gomez 2023-12-07 02:18:56 PST
This can only be reproduced in situations where the MemoryCache if filled and MemoryCache::pruneLiveResourcesToSize() gets called. In order to easily force this, you can edit the function calculateMemoryCacheSizes in CacheModel.cpp so that for the PrimaryBrowser case, the cache capacity is set to 16MB.
Then, loading https://widgets.metrological.com/lightning/liberty/2e3c4fc22f0d35e3eb7fdb47eb7d4658#app:com.gametree.tv.Solitaire will take you to a page with a keypad dialog that is showing an incorrect background image for the dialog (it should be a gray background, but some other image will be shown).

What happens is:

- During a layerFlush, the background images are painted into a buffer that is later passed to the composition, where it's uploaded to a texture to be painted. There are cases where the same image is used as the background of several layers, so instead of painting the image in several different buffers, the CompositingCoordinator keeps a hashMap of ImageBackingStore instances, which contain the rendered buffer for that image, indexed by the nativeImageID (the pointer to the cairo surface that's rendered there). Thanks to this, the layers using the same image as background will have references to the same ImageBackingStore instead of having several buffers with the same content.

- The CompositingCoordinator keeps the ImageBackingStore instances cached until it's the only one with references to them, which means they are not used anymore for the composition.

- In between layer flushes, there's the possibility that the MemoryCache is filled and MemoryCache::pruneLiveResourcesToSize() is called. This may delete the decoded frames for some of the images that we're using. If this happens, when we try to get the nativeImageID of an Image, a new frame will be allocated with a new cairo surface, so we will get a new nativeImageID during the next layerFlush. 

- This causes the ids used by the CompositingCoordinator to index the ImageBackingStore hashMap to become obsolete, as the content is in a different cairo surface now, and the nativeImageID is different. So when we try to get the ImageBackingStore for a nativeImageID, it won't be in the hashMap and a new one will be created for that nativeImageID.

- This wouldn't be a problem if it wasn't because, after calling MemoryCache::pruneLiveResourcesToSize(), new cairo surfaces allocated for the images may be allocated in the same address than previous ones. This is what happens in the example provided. The new nativeImageID gotten for one of the images is the same that another image had before the call to MemoryCache::pruneLiveResourcesToSize(). Due to this, instead of getting a new ImageBackingStore from the CompositingCoordinator, we're getting one that was used by a different image before, which has the wrong image. So when we render that layer, we're using an incorrect ImageBackingStore and the content shown is not appropriate.

After giving a look, I think the best way to fix this is to clear the CompositingCoordinator's hashMap after each layer flush, as can't guarantee that the nativeImageIDs that it holds are valid for subsequent layer flushes if MemoryCache::pruneLiveResourcesToSize() is called.
Comment 1 Miguel Gomez 2023-12-07 02:27:34 PST
Ah, in order to reproduce the problem with the test provided, the browser needs to be maximized before loading the page, so the MemoryCache gets filled with resources.
Comment 2 Miguel Gomez 2023-12-07 02:31:20 PST
Pull request: https://github.com/WebKit/WebKit/pull/21432
Comment 3 Miguel Gomez 2023-12-11 04:59:05 PST
After talking to zdobersek, I'll send a new PR for this with a new approach that gives cairo surfaces an unique ID using cairo_surface_set_user_data. This way we can detect when a new cairo suface has been allocated and update the ImageBackingStores accordingly.
Comment 4 EWS 2023-12-13 15:37:43 PST
Committed 272009@main (fcd69249c493): <https://commits.webkit.org/272009@main>

Reviewed commits have been landed. Closing PR #21432 and removing active labels.
Comment 5 Michael Catanzaro 2023-12-25 08:33:27 PST
*** Bug 256442 has been marked as a duplicate of this bug. ***