Add test SPI to see the entire frame trees in all processes for site isolation #13679
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
achristensen07
added
the
WebKit Process Model
|
EWS run on previous version of this PR (hash c5a0621)
|
pascoej
approved these changes
May 10, 2023
| @@ -31,8 +31,11 @@ | |||
|
|
|||
| namespace WebKit { | |||
|
|
|||
| enum class FrameType : bool { Local, Remote }; | |||
There was a problem hiding this comment.
Can we reuse the enum class FrameType in Frame.h?
There was a problem hiding this comment.
We could, but then we'd need to include Frame.h in many more places, which would hurt our build time.
youennf
approved these changes
May 10, 2023
| class FrameTreeCallbackAggregator : public RefCounted<FrameTreeCallbackAggregator> { | ||
| public: | ||
| static Ref<FrameTreeCallbackAggregator> create(CompletionHandler<void(Vector<FrameTreeNodeData>&&)>&& completionHandler, size_t processCount) { return adoptRef(*new FrameTreeCallbackAggregator(WTFMove(completionHandler), processCount)); } | ||
| void addFrameTree(FrameTreeNodeData&& data) { m_data.uncheckedAppend(WTFMove(data)); } |
There was a problem hiding this comment.
Can we make it private? It seems uncheckedAppend as a public method is not great.
There was a problem hiding this comment.
No, it needs to be public. But I can make the append a checked append call.
| }; | ||
|
|
||
| auto& internals = this->internals(); | ||
| size_t processCount = !!m_mainFrame + internals.domainToSubframePageProxyMap.size(); |
There was a problem hiding this comment.
If there is no main frame, why should there be any sub frame?
There was a problem hiding this comment.
There wouldn't be. I'll make an early return if m_mainFrame is null.
| size_t processCount = !!m_mainFrame + internals.domainToSubframePageProxyMap.size(); | ||
| auto aggregator = FrameTreeCallbackAggregator::create(WTFMove(completionHandler), processCount); | ||
| if (m_mainFrame) { | ||
| m_mainFrame->process().sendWithAsyncReply(Messages::WebPage::GetFrameTree(), [aggregator] (FrameTreeNodeData&& data) { |
| }, webPageID()); | ||
| } | ||
| for (auto& subframePageProxy : internals.domainToSubframePageProxyMap.values()) { | ||
| subframePageProxy->process().sendWithAsyncReply(Messages::WebPage::GetFrameTree(), [aggregator] (FrameTreeNodeData&& data) { |
| debugPrint(child, indentation + 2); | ||
| } | ||
|
|
||
| void debugPrint(const ExpectedFrameTree& node, size_t indentation = 0) |
There was a problem hiding this comment.
Are all these debug prints routines really needed?
There was a problem hiding this comment.
They were quite useful for looking for what went wrong. I guess they don't need to be checked in.
| Util::run(&done); | ||
| checkFrameTreesInProcesses(webView.get(), { | ||
| { "https://example.com"_s, | ||
| { { RemoteFrame } } |
There was a problem hiding this comment.
I would expect the top level main frame to be local
There was a problem hiding this comment.
It is. This is its child.
achristensen07
force-pushed
the
eng/Add-test-SPI-to-see-the-entire-frame-trees-in-all-processes-for-site-isolation
branch
from
c5a0621
to
a53600d
Compare
|
EWS run on previous version of this PR (hash a53600d) |
webkit-ews-buildbot
added
the
merging-blocked
achristensen07
force-pushed
the
eng/Add-test-SPI-to-see-the-entire-frame-trees-in-all-processes-for-site-isolation
branch
from
a53600d
to
6c383b3
Compare
|
EWS run on current version of this PR (hash 6c383b3)
|
achristensen07
added
merge-queue
…isolation https://bugs.webkit.org/show_bug.cgi?id=256574 rdar://109134755 Reviewed by J Pascoe and Youenn Fablet. WKWebView._frames gets info about the frame tree from whatever process each frame is in. In order to add tests that verifies the whole frame tree state in multiple process during operations like frame destruction, we need to get the whole frame tree state from each process and inspect all of it. This introduces such SPI and uses it for tests. * Source/WebKit/Shared/FrameInfoData.h: * Source/WebKit/Shared/FrameInfoData.serialization.in: * Source/WebKit/UIProcess/API/APIFrameInfo.h: * Source/WebKit/UIProcess/API/APIFrameTreeNode.h: * Source/WebKit/UIProcess/API/Cocoa/WKFrameInfo.mm: (-[WKFrameInfo _isLocalFrame]): * Source/WebKit/UIProcess/API/Cocoa/WKFrameInfoPrivate.h: * Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm: (-[WKWebView _frames:]): (-[WKWebView _frameTrees:]): * Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h: * Source/WebKit/UIProcess/API/Cocoa/_WKFrameTreeNode.h: * Source/WebKit/UIProcess/API/Cocoa/_WKFrameTreeNode.mm: (-[_WKFrameTreeNode info]): (-[_WKFrameTreeNode _isLocalFrame]): * Source/WebKit/UIProcess/Cocoa/UIDelegate.mm: (WebKit::UIDelegate::UIClient::promptForDisplayCapturePermission): (WebKit::UIDelegate::UIClient::decidePolicyForUserMediaPermissionRequest): * Source/WebKit/UIProcess/ProvisionalPageProxy.cpp: (WebKit::ProvisionalPageProxy::cancel): * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::getAllFrameTrees): * Source/WebKit/UIProcess/WebPageProxy.h: * Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp: (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): * Source/WebKit/WebProcess/WebPage/WebFrame.cpp: (WebKit::WebFrame::info const): (WebKit::WebFrame::frameTreeData const): * Source/WebKit/WebProcess/WebPage/WebFrame.h: * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::getFrameTree): * Source/WebKit/WebProcess/WebPage/WebPage.h: * Source/WebKit/WebProcess/WebPage/WebPage.messages.in: * Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm: (TestWebKitAPI::nullTerminatedIndentation): (TestWebKitAPI::debugPrint): (TestWebKitAPI::frameTreesMatch): (TestWebKitAPI::checkFrameTreesInProcesses): (TestWebKitAPI::TEST): Canonical link: https://commits.webkit.org/263912@main
webkit-commit-queue
force-pushed
the
eng/Add-test-SPI-to-see-the-entire-frame-trees-in-all-processes-for-site-isolation
branch
from
6c383b3
to
538b503
Compare
|
Committed 263912@main (538b503): https://commits.webkit.org/263912@main Reviewed commits have been landed. Closing PR #13679 and removing active labels. |
webkit-commit-queue
removed
the
unsafe-merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
538b503
6c383b3
🛠 wincairo🧪 wpe-wk2🧪 api-mac🧪 ios-wk2-wpt🧪 mac-wk1🧪 gtk-wk2🧪 mac-wk2🧪 api-gtk🧪 mac-AS-debug-wk2🛠 tv-sim🛠 🧪 merge🛠 watch-sim