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

Multiple iframes in same isolated process results in one of them not being drawn. #20120

Merged

Conversation

mattwoodrow
Copy link
Contributor

@mattwoodrow mattwoodrow commented Nov 7, 2023

bb4da53

Multiple iframes in same isolated process results in one of them not being drawn.
https://bugs.webkit.org/show_bug.cgi?id=264356
<rdar://118034981>

Reviewed by Alex Christensen.

RemoteLayerBackingStoreCollection::willCommitLayerTree collects all the backing stores that were unreachable,
and includes that list in the transaction so that the UI process can ensure the buffer references are removed.

This changes moves that call until we've finished building all the transactions, so that were not considering
buffers only used in latter transactions to be unreachable for the first transaction.

It also adds a new 'willBuildTransaction' function. The set of backing stores that need display is a property
of the transaction (where we call prepareBuffersForDisplay, and then paint them), so we want to clear this
state per-transaction, not per rendering update.

* LayoutTests/http/tests/site-isolation/double-iframe-expected.html: Added.
* LayoutTests/http/tests/site-isolation/double-iframe.html: Added.
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStoreCollection.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStoreCollection.mm:
(WebKit::RemoteLayerBackingStoreCollection::willFlushLayers):
(WebKit::RemoteLayerBackingStoreCollection::willBuildTransaction):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::updateRendering):

Canonical link: https://commits.webkit.org/270399@main

127406d

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
❌ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 watch ⏳ 🛠 jsc-mips
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim ⏳ 🧪 jsc-mips-tests

@mattwoodrow mattwoodrow self-assigned this Nov 7, 2023
@mattwoodrow mattwoodrow added the Layout and Rendering For bugs with layout and rendering of Web pages. label Nov 7, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 7, 2023
@achristensen07
Copy link
Contributor

This is great, skipping test on iOS in #20176

@achristensen07 achristensen07 added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Nov 8, 2023
webkit-commit-queue pushed a commit to achristensen07/WebKit that referenced this pull request Nov 8, 2023
https://bugs.webkit.org/show_bug.cgi?id=264422

Unreviewed.

WebKit#20120 added a new test that fails on iOS
because of some unrelated drawing issues that also affect other site-isolation
tests.  Skip this new test like we do the others until the underlying issue is
fixed.

* LayoutTests/platform/ios/TestExpectations:

Canonical link: https://commits.webkit.org/270398@main
…being drawn.

https://bugs.webkit.org/show_bug.cgi?id=264356
<rdar://118034981>

Reviewed by Alex Christensen.

RemoteLayerBackingStoreCollection::willCommitLayerTree collects all the backing stores that were unreachable,
and includes that list in the transaction so that the UI process can ensure the buffer references are removed.

This changes moves that call until we've finished building all the transactions, so that were not considering
buffers only used in latter transactions to be unreachable for the first transaction.

It also adds a new 'willBuildTransaction' function. The set of backing stores that need display is a property
of the transaction (where we call prepareBuffersForDisplay, and then paint them), so we want to clear this
state per-transaction, not per rendering update.

* LayoutTests/http/tests/site-isolation/double-iframe-expected.html: Added.
* LayoutTests/http/tests/site-isolation/double-iframe.html: Added.
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStoreCollection.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStoreCollection.mm:
(WebKit::RemoteLayerBackingStoreCollection::willFlushLayers):
(WebKit::RemoteLayerBackingStoreCollection::willBuildTransaction):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::updateRendering):

Canonical link: https://commits.webkit.org/270399@main
@webkit-commit-queue
Copy link
Collaborator

Committed 270399@main (bb4da53): https://commits.webkit.org/270399@main

Reviewed commits have been landed. Closing PR #20120 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit bb4da53 into WebKit:main Nov 8, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
5 participants