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

Begin drawing pixels to the screen from site isolated iframes #9204

Conversation

achristensen07
Copy link
Contributor

@achristensen07 achristensen07 commented Jan 27, 2023

90549ae

Begin drawing pixels to the screen from site isolated iframes
https://bugs.webkit.org/show_bug.cgi?id=251245
rdar://104726144

Reviewed by Simon Fraser.

If we have a remote frame, the parent process needs to make a layer but put nothing in it.
The iframe process's root layer needs to be the contents of the empty layer.
To make this happen, the journey starts in RenderLayerBacking::updateConfiguration where
we call GraphicsLayerCA::setContentsToPlatformLayerHost, which makes this layer a host for
content that will come from another process with a given identifier generated in the UI
process.  We need the iframe process to know it has a hosting context identifier
so we do this by passing the LayerHostingContextIdentifier to the WebPage.  If this is present,
then in RemoteLayerTreeContext::buildTransaction we set the layerHostingContextIdentifier on the
RemoteLayerTreeTransaction.  When the UI process sees a transaction with a layerHostingContextIdentifier
it has some special logic in RemoteLayerTreeDrawingAreaProxy::commitLayerTree to make its
"root layer" not a root layer of the whole page, but rather it sets its "root layer" to be a
sublayer of the parent process's empty layer which was intended for this purpose.

* LayoutTests/http/tests/site-isolation/basic-iframe-expected.html: Added.
* LayoutTests/http/tests/site-isolation/basic-iframe.html: Added.
* LayoutTests/http/tests/site-isolation/resources/green-background.html: Added.
* Source/WebCore/html/HTMLFrameOwnerElement.cpp:
(WebCore::HTMLFrameOwnerElement::disconnectContentFrame):
* Source/WebCore/html/HTMLFrameOwnerElement.h:
* Source/WebCore/page/RemoteFrameView.h:
(isType):
* Source/WebCore/platform/Widget.h:
(WebCore::Widget::isRemoteFrameView const):
* Source/WebCore/platform/graphics/GraphicsLayer.h:
(WebCore::GraphicsLayer::setRemoteFrame):
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateConfiguration):
* Source/WebCore/rendering/RenderWidget.cpp:
(WebCore::RenderWidget::requiresAcceleratedCompositing const):
(WebCore::RenderWidget::remoteFrame const):
* Source/WebCore/rendering/RenderWidget.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:
(WebKit::RemoteLayerTreeTransaction::setRemoteFrameIdentifier):
(WebKit::RemoteLayerTreeTransaction::remoteFrameIdentifier const):
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.mm:
(WebKit::RemoteLayerTreeTransaction::encode const):
(WebKit::RemoteLayerTreeTransaction::decode):
* Source/WebKit/Shared/WebPageCreationParameters.cpp:
(WebKit::WebPageCreationParameters::encode const):
(WebKit::WebPageCreationParameters::decode):
* Source/WebKit/Shared/WebPageCreationParameters.h:
* Source/WebKit/UIProcess/ProvisionalFrameProxy.cpp:
(WebKit::ProvisionalFrameProxy::ProvisionalFrameProxy):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.h:
(WebKit::RemoteLayerTreeDrawingAreaProxy::frameLayerMap):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.messages.in:
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
(WebKit::RemoteLayerTreeDrawingAreaProxy::layerWillBeUsedForRemoteFrame):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm:
(WebKit::RemoteLayerTreeHost::updateLayerTree):
* Source/WebKit/UIProcess/WebFrameProxy.h:
(WebKit::WebFrameProxy::parentFrame):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.cpp:
(WebKit::GraphicsLayerCARemote::setRemoteFrame):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.mm:
(WebKit::RemoteLayerTreeContext::buildTransaction):
* Source/WebKit/WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::didCommitLoadInAnotherProcess):
Add a call to scheduleInvalidateStyleAndLayerComposition or the logic in
RenderWidget::requiresAcceleratedCompositing won't be called until something else
invalidates style.  I found this initially by being frustrated that
requiresAcceleratedCompositing returning true didn't do anything, then noticing that
when I plugged my computer into power it produced a layer.  This call does a similar
kick of the iframe to tell it to make a layer now, when the LocalFrame is swapped
out with a RemoteFrame.
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
* Source/WebKit/WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::parentFrameIdentifier const):

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

1c37d8e

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe   πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk   πŸ›  wincairo
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac   πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ webkitpy βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1   πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@achristensen07 achristensen07 self-assigned this Jan 27, 2023
@achristensen07 achristensen07 added the WebKit Process Model Bugs related to WebKit's multi-process architecture label Jan 27, 2023
Source/WebCore/platform/graphics/GraphicsLayer.h Outdated Show resolved Hide resolved

bool didUpdateEditorState = layerTreeTransaction.hasEditorState() && m_webPageProxy.updateEditorState(layerTreeTransaction.editorState(), WebPageProxy::ShouldMergeVisualEditorState::Yes);
bool didUpdateEditorState { false };
if (!layerTreeTransaction.remoteFrameIdentifier()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we want a totally separate entry point for transactions coming in for remote frames. Much of the code in this function assumes main frame.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's enough in common I think this approach is better for now, but not by a lot. That may not always be the case, though. If you feel strongly about it I can separate it now or later.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 27, 2023
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from 523c0d6 to fce503b Compare January 29, 2023 04:33
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from fce503b to ba37731 Compare January 30, 2023 21:26
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from ba37731 to 2947178 Compare January 30, 2023 21:46
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Jan 30, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 30, 2023
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from 2947178 to 91973fa Compare January 30, 2023 22:54
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Jan 30, 2023
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from 91973fa to 2907a71 Compare January 31, 2023 00:01
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from 2907a71 to f9b7c3d Compare January 31, 2023 00:41
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 31, 2023
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from f9b7c3d to a27ea03 Compare January 31, 2023 06:17
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Jan 31, 2023
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from a27ea03 to 420a5a8 Compare January 31, 2023 17:03
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from 420a5a8 to 4ebe17e Compare January 31, 2023 22:23
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from 4ebe17e to eab3f20 Compare January 31, 2023 22:25
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from eab3f20 to 1be0a9d Compare February 1, 2023 06:00
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 1, 2023
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Feb 1, 2023
@achristensen07 achristensen07 force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from 1be0a9d to 1c37d8e Compare February 7, 2023 00:54
@achristensen07 achristensen07 added the merge-queue Applied to send a pull request to merge-queue label Feb 7, 2023
https://bugs.webkit.org/show_bug.cgi?id=251245
rdar://104726144

Reviewed by Simon Fraser.

If we have a remote frame, the parent process needs to make a layer but put nothing in it.
The iframe process's root layer needs to be the contents of the empty layer.
To make this happen, the journey starts in RenderLayerBacking::updateConfiguration where
we call GraphicsLayerCA::setContentsToPlatformLayerHost, which makes this layer a host for
content that will come from another process with a given identifier generated in the UI
process.  We need the iframe process to know it has a hosting context identifier
so we do this by passing the LayerHostingContextIdentifier to the WebPage.  If this is present,
then in RemoteLayerTreeContext::buildTransaction we set the layerHostingContextIdentifier on the
RemoteLayerTreeTransaction.  When the UI process sees a transaction with a layerHostingContextIdentifier
it has some special logic in RemoteLayerTreeDrawingAreaProxy::commitLayerTree to make its
"root layer" not a root layer of the whole page, but rather it sets its "root layer" to be a
sublayer of the parent process's empty layer which was intended for this purpose.

* LayoutTests/http/tests/site-isolation/basic-iframe-expected.html: Added.
* LayoutTests/http/tests/site-isolation/basic-iframe.html: Added.
* LayoutTests/http/tests/site-isolation/resources/green-background.html: Added.
* Source/WebCore/html/HTMLFrameOwnerElement.cpp:
(WebCore::HTMLFrameOwnerElement::disconnectContentFrame):
* Source/WebCore/html/HTMLFrameOwnerElement.h:
* Source/WebCore/page/RemoteFrameView.h:
(isType):
* Source/WebCore/platform/Widget.h:
(WebCore::Widget::isRemoteFrameView const):
* Source/WebCore/platform/graphics/GraphicsLayer.h:
(WebCore::GraphicsLayer::setRemoteFrame):
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateConfiguration):
* Source/WebCore/rendering/RenderWidget.cpp:
(WebCore::RenderWidget::requiresAcceleratedCompositing const):
(WebCore::RenderWidget::remoteFrame const):
* Source/WebCore/rendering/RenderWidget.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:
(WebKit::RemoteLayerTreeTransaction::setRemoteFrameIdentifier):
(WebKit::RemoteLayerTreeTransaction::remoteFrameIdentifier const):
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.mm:
(WebKit::RemoteLayerTreeTransaction::encode const):
(WebKit::RemoteLayerTreeTransaction::decode):
* Source/WebKit/Shared/WebPageCreationParameters.cpp:
(WebKit::WebPageCreationParameters::encode const):
(WebKit::WebPageCreationParameters::decode):
* Source/WebKit/Shared/WebPageCreationParameters.h:
* Source/WebKit/UIProcess/ProvisionalFrameProxy.cpp:
(WebKit::ProvisionalFrameProxy::ProvisionalFrameProxy):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.h:
(WebKit::RemoteLayerTreeDrawingAreaProxy::frameLayerMap):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.messages.in:
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
(WebKit::RemoteLayerTreeDrawingAreaProxy::layerWillBeUsedForRemoteFrame):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm:
(WebKit::RemoteLayerTreeHost::updateLayerTree):
* Source/WebKit/UIProcess/WebFrameProxy.h:
(WebKit::WebFrameProxy::parentFrame):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.cpp:
(WebKit::GraphicsLayerCARemote::setRemoteFrame):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.mm:
(WebKit::RemoteLayerTreeContext::buildTransaction):
* Source/WebKit/WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::didCommitLoadInAnotherProcess):
Add a call to scheduleInvalidateStyleAndLayerComposition or the logic in
RenderWidget::requiresAcceleratedCompositing won't be called until something else
invalidates style.  I found this initially by being frustrated that
requiresAcceleratedCompositing returning true didn't do anything, then noticing that
when I plugged my computer into power it produced a layer.  This call does a similar
kick of the iframe to tell it to make a layer now, when the LocalFrame is swapped
out with a RemoteFrame.
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
* Source/WebKit/WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::parentFrameIdentifier const):

Canonical link: https://commits.webkit.org/259937@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Begin-drawing-pixels-to-the-screen-from-site-isolated-iframes branch from 1c37d8e to 90549ae Compare February 7, 2023 06:26
@webkit-commit-queue
Copy link
Collaborator

Committed 259937@main (90549ae): https://commits.webkit.org/259937@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 7, 2023
@webkit-early-warning-system webkit-early-warning-system merged commit 90549ae into WebKit:main Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit Process Model Bugs related to WebKit's multi-process architecture
Projects
None yet
5 participants