Skip to content

Commit

Permalink
Remove redundant FrameIdentifier parameter in decidePolicyForNavigati…
Browse files Browse the repository at this point in the history
…onAction message

https://bugs.webkit.org/show_bug.cgi?id=259705
rdar://113227343

Reviewed by Brady Eidson.

FrameInfoData always has the same frame identifier.
I also removed a redundant message check.
I also did similar redundant parameters in decidePolicyForNewWindowAction,
decidePolicyForNavigationActionSync, and decidePolicyForResponse

In order to still have a FrameIdentifier from a Document after it is removed from
the frame tree and to keep the layout test working at http/tests/navigation/window-open-redirect-and-remove-opener.html
I store the FrameIdentifier on the Document and use that when finding the opener's FrameIdentifier.

* Source/WebKit/Shared/FrameInfoData.h:
* Source/WebKit/Shared/FrameInfoData.serialization.in:
* Source/WebKit/UIProcess/API/APIFrameInfo.cpp:
(API::FrameInfo::handle const):
* Source/WebKit/UIProcess/Cocoa/NavigationState.mm:
(WebKit::createErrorWithRecoveryAttempter):
* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync):
* Source/WebKit/UIProcess/ProvisionalPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationActionAsync):
(WebKit::WebPageProxy::decidePolicyForNavigationActionAsyncShared):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
(WebKit::WebPageProxy::createNewPage):
(WebKit::WebPageProxy::requestGeolocationPermissionForFrame):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):

Canonical link: https://commits.webkit.org/266514@main
  • Loading branch information
achristensen07 committed Aug 2, 2023
1 parent 83a6562 commit ef9aa42
Show file tree
Hide file tree
Showing 18 changed files with 87 additions and 100 deletions.
3 changes: 2 additions & 1 deletion Source/WebCore/dom/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,7 @@ Document::Document(LocalFrame* frame, const Settings& settings, const URL& url,
#endif
, m_isSynthesized(constructionFlags.contains(ConstructionFlag::Synthesized))
, m_isNonRenderedPlaceholder(constructionFlags.contains(ConstructionFlag::NonRenderedPlaceholder))
, m_frameIdentifier(frame ? std::optional(frame->frameID()) : std::nullopt)
{
addToDocumentsMap();

Expand Down Expand Up @@ -8605,7 +8606,7 @@ std::optional<PageIdentifier> Document::pageID() const

std::optional<FrameIdentifier> Document::frameID() const
{
return m_frame ? std::optional<FrameIdentifier>(m_frame->loader().frameID()) : std::nullopt;
return m_frameIdentifier;
}

void Document::registerArticleElement(Element& article)
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/dom/Document.h
Original file line number Diff line number Diff line change
Expand Up @@ -2422,6 +2422,7 @@ class Document
static bool hasEverCreatedAnAXObjectCache;

RefPtr<ResizeObserver> m_resizeObserverForContainIntrinsicSize;
const std::optional<FrameIdentifier> m_frameIdentifier;
};

Element* eventTargetElementForDocument(Document*);
Expand Down
20 changes: 2 additions & 18 deletions Source/WebCore/loader/NavigationRequester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,35 +26,19 @@
#include "config.h"
#include "NavigationRequester.h"

#include "ContentSecurityPolicy.h"
#include "Document.h"
#include "FrameDestructionObserverInlines.h"
#include "FrameLoader.h"
#include "LocalFrame.h"

namespace WebCore {

static std::optional<GlobalFrameIdentifier> createGlobalFrameIdentifier(const Document& document)
{
auto* frame = document.frame();
if (!frame)
return std::nullopt;

auto pageID = document.pageID();
if (!pageID)
return std::nullopt;

return GlobalFrameIdentifier { *pageID, frame->frameID() };
}

NavigationRequester NavigationRequester::from(Document& document)
{
return {
document.url(),
document.securityOrigin(),
document.topOrigin(),
document.policyContainer(),
createGlobalFrameIdentifier(document)
document.frameID(),
document.pageID()
};
}

Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/loader/NavigationRequester.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ struct NavigationRequester {
Ref<SecurityOrigin> securityOrigin;
Ref<SecurityOrigin> topOrigin;
PolicyContainer policyContainer;
std::optional<GlobalFrameIdentifier> globalFrameIdentifier;
std::optional<FrameIdentifier> frameID;
std::optional<PageIdentifier> pageID;
};

} // namespace WebCore
2 changes: 1 addition & 1 deletion Source/WebKit/Shared/FrameInfoData.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ struct FrameInfoData {
WebCore::ResourceRequest request;
WebCore::SecurityOriginData securityOrigin;
String frameName;
std::optional<WebCore::FrameIdentifier> frameID;
WebCore::FrameIdentifier frameID;
std::optional<WebCore::FrameIdentifier> parentFrameID;
ProcessID processID;
};
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/Shared/FrameInfoData.serialization.in
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ struct WebKit::FrameInfoData {
WebCore::ResourceRequest request
WebCore::SecurityOriginData securityOrigin
String frameName
std::optional<WebCore::FrameIdentifier> frameID
WebCore::FrameIdentifier frameID
std::optional<WebCore::FrameIdentifier> parentFrameID
WTF::ProcessID processID;
}
3 changes: 2 additions & 1 deletion Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in
Original file line number Diff line number Diff line change
Expand Up @@ -4308,7 +4308,8 @@ struct WebCore::NavigationRequester {
Ref<WebCore::SecurityOrigin> securityOrigin;
Ref<WebCore::SecurityOrigin> topOrigin;
WebCore::PolicyContainer policyContainer;
std::optional<WebCore::GlobalFrameIdentifier> globalFrameIdentifier;
std::optional<WebCore::FrameIdentifier> frameID;
std::optional<WebCore::PageIdentifier> pageID;
};

struct WebCore::PolicyContainer {
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/API/APIFrameInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ FrameInfo::~FrameInfo() = default;

Ref<FrameHandle> FrameInfo::handle() const
{
return FrameHandle::create(m_data.frameID ? *m_data.frameID : WebCore::FrameIdentifier { });
return FrameHandle::create(m_data.frameID);
}

RefPtr<FrameHandle> FrameInfo::parentFrameHandle() const
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/Cocoa/NavigationState.mm
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,7 @@ static void tryInterceptNavigation(Ref<API::NavigationAction>&& navigationAction

static RetainPtr<NSError> createErrorWithRecoveryAttempter(WKWebView *webView, const FrameInfoData& frameInfo, NSError *originalError, bool isHTTPSOnlyError = false)
{
auto frameHandle = API::FrameHandle::create(frameInfo.frameID ? *frameInfo.frameID : FrameIdentifier { });
auto frameHandle = API::FrameHandle::create(frameInfo.frameID);

auto recoveryAttempter = adoptNS([[WKReloadFrameErrorRecoveryAttempter alloc] initWithWebView:webView frameHandle:wrapper(frameHandle.get()) urlString:originalError.userInfo[NSURLErrorFailingURLStringErrorKey]]);

Expand Down
20 changes: 10 additions & 10 deletions Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -390,20 +390,20 @@ void ProvisionalPageProxy::didChangeProvisionalURLForFrame(FrameIdentifier frame
m_page->didChangeProvisionalURLForFrameShared(m_process.copyRef(), frameID, navigationID, WTFMove(url));
}

void ProvisionalPageProxy::decidePolicyForNavigationActionAsync(FrameIdentifier frameID, FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier identifier, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, uint64_t listenerID)
void ProvisionalPageProxy::decidePolicyForNavigationActionAsync(FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier identifier, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, uint64_t listenerID)
{
if (!validateInput(frameID, navigationID))
if (!validateInput(frameInfo.frameID, navigationID))
return;

m_page->decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), m_webPageID, frameID, WTFMove(frameInfo), identifier, navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), listenerID);
m_page->decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), m_webPageID, WTFMove(frameInfo), identifier, navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), listenerID);
}

void ProvisionalPageProxy::decidePolicyForResponse(FrameIdentifier frameID, FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier identifier, uint64_t navigationID, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& request, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID)
void ProvisionalPageProxy::decidePolicyForResponse(FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier identifier, uint64_t navigationID, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& request, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID)
{
if (!validateInput(frameID, navigationID))
if (!validateInput(frameInfo.frameID, navigationID))
return;

m_page->decidePolicyForResponseShared(m_process.copyRef(), m_webPageID, frameID, WTFMove(frameInfo), identifier, navigationID, response, request, canShowMIMEType, downloadAttribute, listenerID);
m_page->decidePolicyForResponseShared(m_process.copyRef(), m_webPageID, WTFMove(frameInfo), identifier, navigationID, response, request, canShowMIMEType, downloadAttribute, listenerID);
}

void ProvisionalPageProxy::didPerformServerRedirect(const String& sourceURLString, const String& destinationURLString, FrameIdentifier frameID)
Expand Down Expand Up @@ -432,23 +432,23 @@ void ProvisionalPageProxy::backForwardGoToItem(const WebCore::BackForwardItemIde
m_page->backForwardGoToItemShared(m_process.copyRef(), identifier, WTFMove(completionHandler));
}

void ProvisionalPageProxy::decidePolicyForNavigationActionSync(FrameIdentifier frameID, bool isMainFrame, FrameInfoData&& frameInfoData, WebCore::PolicyCheckIdentifier identifier,
void ProvisionalPageProxy::decidePolicyForNavigationActionSync(FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier identifier,
uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID,
const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse,
CompletionHandler<void(PolicyDecision&&)>&& reply)
{
if (!isMainFrame || (m_mainFrame && m_mainFrame->frameID() != frameID) || navigationID != m_navigationID) {
if (!frameInfo.isMainFrame || (m_mainFrame && m_mainFrame->frameID() != frameInfo.frameID) || navigationID != m_navigationID) {
reply(PolicyDecision { identifier, std::nullopt, WebCore::PolicyAction::Ignore, navigationID });
return;
}

if (!m_mainFrame) {
// This synchronous IPC message was processed before the asynchronous DidCreateMainFrame one so we do not know about this frameID yet.
didCreateMainFrame(frameID);
didCreateMainFrame(frameInfo.frameID);
}
ASSERT(m_mainFrame);

m_page->decidePolicyForNavigationActionSyncShared(m_process.copyRef(), m_webPageID, frameID, isMainFrame, WTFMove(frameInfoData), identifier, navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), WTFMove(reply));
m_page->decidePolicyForNavigationActionSyncShared(m_process.copyRef(), m_webPageID, WTFMove(frameInfo), identifier, navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), WTFMove(reply));
}

void ProvisionalPageProxy::logDiagnosticMessageFromWebProcess(const String& message, const String& description, WebCore::ShouldSample shouldSample)
Expand Down
6 changes: 3 additions & 3 deletions Source/WebKit/UIProcess/ProvisionalPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
bool sendMessage(UniqueRef<IPC::Encoder>&&, OptionSet<IPC::SendOption>) final;
bool sendMessageWithAsyncReply(UniqueRef<IPC::Encoder>&&, AsyncReplyHandler, OptionSet<IPC::SendOption>) final;

void decidePolicyForNavigationActionAsync(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, uint64_t listenerID);
void decidePolicyForResponse(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID);
void decidePolicyForNavigationActionAsync(FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, uint64_t listenerID);
void decidePolicyForResponse(FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID);
void didChangeProvisionalURLForFrame(WebCore::FrameIdentifier, uint64_t navigationID, URL&&);
void didPerformServerRedirect(const String& sourceURLString, const String& destinationURLString, WebCore::FrameIdentifier);
void didReceiveServerRedirectForProvisionalLoadForFrame(WebCore::FrameIdentifier, uint64_t navigationID, WebCore::ResourceRequest&&, const UserData&);
Expand All @@ -151,7 +151,7 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
void logDiagnosticMessageWithValueDictionaryFromWebProcess(const String& message, const String& description, const WebCore::DiagnosticLoggingClient::ValueDictionary&, WebCore::ShouldSample);
void startURLSchemeTask(URLSchemeTaskParameters&&);
void backForwardGoToItem(const WebCore::BackForwardItemIdentifier&, CompletionHandler<void(const WebBackForwardListCounts&)>&&);
void decidePolicyForNavigationActionSync(WebCore::FrameIdentifier, bool isMainFrame, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, CompletionHandler<void(PolicyDecision&&)>&&);
void decidePolicyForNavigationActionSync(FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, CompletionHandler<void(PolicyDecision&&)>&&);
void backForwardAddItem(BackForwardListItemState&&);
void didDestroyNavigation(uint64_t navigationID);
#if USE(QUICK_LOOK)
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/RemotePageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ void RemotePageProxy::didReceiveMessage(IPC::Connection& connection, IPC::Decode
m_page->didReceiveMessage(connection, decoder);
}

void RemotePageProxy::decidePolicyForResponse(WebCore::FrameIdentifier frameID, FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier identifier, uint64_t navigationID, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& request, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID)
void RemotePageProxy::decidePolicyForResponse(FrameInfoData&& frameInfo, WebCore::PolicyCheckIdentifier identifier, uint64_t navigationID, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& request, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID)
{
if (m_page)
m_page->decidePolicyForResponseShared(m_process.copyRef(), m_page->webPageID(), frameID, WTFMove(frameInfo), identifier, navigationID, response, request, canShowMIMEType, downloadAttribute, listenerID);
m_page->decidePolicyForResponseShared(m_process.copyRef(), m_page->webPageID(), WTFMove(frameInfo), identifier, navigationID, response, request, canShowMIMEType, downloadAttribute, listenerID);
}

void RemotePageProxy::didCommitLoadForFrame(WebCore::FrameIdentifier frameID, FrameInfoData&& frameInfo, WebCore::ResourceRequest&& request, uint64_t navigationID, const String& mimeType, bool frameHasCustomContentProvider, WebCore::FrameLoadType frameLoadType, const WebCore::CertificateInfo& certificateInfo, bool usedLegacyTLS, bool privateRelayed, bool containsPluginDocument, WebCore::HasInsecureContent hasInsecureContent, WebCore::MouseEventPolicy mouseEventPolicy, const UserData& userData)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/RemotePageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class RemotePageProxy : public RefCounted<RemotePageProxy>, public IPC::MessageR
RemotePageProxy(WebPageProxy&, WebProcessProxy&, const WebCore::RegistrableDomain&, WebPageProxyMessageReceiverRegistration*);
void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
bool didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, UniqueRef<IPC::Encoder>&) final;
void decidePolicyForResponse(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID);
void decidePolicyForResponse(FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, const String& downloadAttribute, uint64_t listenerID);
void didCommitLoadForFrame(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::ResourceRequest&&, uint64_t navigationID, const String& mimeType, bool frameHasCustomContentProvider, WebCore::FrameLoadType, const WebCore::CertificateInfo&, bool usedLegacyTLS, bool privateRelayed, bool containsPluginDocument, WebCore::HasInsecureContent, WebCore::MouseEventPolicy, const UserData&);

const WebCore::PageIdentifier m_webPageID;
Expand Down
Loading

0 comments on commit ef9aa42

Please sign in to comment.