Skip to content

Commit

Permalink
Remove redundant redirectResponse information in navigation action me…
Browse files Browse the repository at this point in the history
…ssage

https://bugs.webkit.org/show_bug.cgi?id=259864

Reviewed by Wenson Hsieh.

We are sending the response, and separately we are sending a bool indicating if the response is null.
This is unnecessary redundant information.

Also, I'm going to need the whole response on NavigationActionData for #16383
and this is a little piece of that change that is easy to separate into a non-behavior-changing PR.

* Source/WebKit/Shared/NavigationActionData.h:
* Source/WebKit/Shared/NavigationActionData.serialization.in:
* Source/WebKit/UIProcess/API/APINavigation.h:
(API::Navigation::currentRequestIsRedirect const):
* Source/WebKit/UIProcess/API/APINavigationAction.h:
* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync):
(WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionSync):
* Source/WebKit/UIProcess/ProvisionalPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationActionAsync):
(WebKit::WebPageProxy::decidePolicyForNavigationActionAsyncShared):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
(WebKit::WebPageProxy::decidePolicyForNavigationActionSync):
(WebKit::WebPageProxy::decidePolicyForNavigationActionSyncShared):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::createWindow):
* Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp:
(WebKit::WebLocalFrameLoaderClient::didSameDocumentNavigationForFrameViaJSHistoryAPI):
(WebKit::WebLocalFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):

Canonical link: https://commits.webkit.org/266620@main
  • Loading branch information
achristensen07 committed Aug 6, 2023
1 parent 8b7b217 commit a68a8b8
Show file tree
Hide file tree
Showing 12 changed files with 37 additions and 35 deletions.
3 changes: 2 additions & 1 deletion Source/WebKit/Shared/NavigationActionData.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <WebCore/FloatPoint.h>
#include <WebCore/FrameLoaderTypes.h>
#include <WebCore/PrivateClickMeasurement.h>
#include <WebCore/ResourceResponse.h>
#include <WebCore/SecurityOriginData.h>

namespace IPC {
Expand All @@ -56,7 +57,7 @@ struct NavigationActionData {
WebCore::ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy { WebCore::ShouldOpenExternalURLsPolicy::ShouldNotAllow };
WTF::String downloadAttribute;
WebCore::FloatPoint clickLocationInRootViewCoordinates;
bool isRedirect { false };
WebCore::ResourceResponse redirectResponse;
bool treatAsSameOriginNavigation { false };
bool hasOpenedFrames { false };
bool openedByDOMWithOpener { false };
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/Shared/NavigationActionData.serialization.in
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ struct WebKit::NavigationActionData {
WebCore::ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy;
WTF::String downloadAttribute;
WebCore::FloatPoint clickLocationInRootViewCoordinates;
bool isRedirect;
WebCore::ResourceResponse redirectResponse;
bool treatAsSameOriginNavigation;
bool hasOpenedFrames;
bool openedByDOMWithOpener;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/API/APINavigation.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class Navigation : public ObjectImpl<Object::Type::Navigation> {
const WebCore::ResourceRequest& currentRequest() const { return m_currentRequest; }
std::optional<WebCore::ProcessIdentifier> currentRequestProcessIdentifier() const { return m_currentRequestProcessIdentifier; }

bool currentRequestIsRedirect() const { return m_lastNavigationAction.isRedirect; }
bool currentRequestIsRedirect() const { return !m_lastNavigationAction.redirectResponse.isNull(); }

WebKit::WebBackForwardListItem* targetItem() const { return m_targetItem.get(); }
WebKit::WebBackForwardListItem* fromItem() const { return m_fromItem.get(); }
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/API/APINavigationAction.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class NavigationAction final : public ObjectImpl<Object::Type::NavigationAction>
bool shouldOpenExternalSchemes() const { return m_navigationActionData.shouldOpenExternalURLsPolicy == WebCore::ShouldOpenExternalURLsPolicy::ShouldAllow || m_navigationActionData.shouldOpenExternalURLsPolicy == WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemesButNotAppLinks; }
bool shouldOpenAppLinks() const { return m_shouldOpenAppLinks && m_navigationActionData.shouldOpenExternalURLsPolicy == WebCore::ShouldOpenExternalURLsPolicy::ShouldAllow; }
bool shouldPerformDownload() const { return !m_navigationActionData.downloadAttribute.isNull(); }
bool isRedirect() const { return m_navigationActionData.isRedirect; }
bool isRedirect() const { return !m_navigationActionData.redirectResponse.isNull(); }
bool hasOpener() const { return m_navigationActionData.hasOpener; }
WebCore::ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy() const { return m_navigationActionData.shouldOpenExternalURLsPolicy; }

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

void ProvisionalPageProxy::decidePolicyForNavigationActionAsync(FrameInfoData&& frameInfo, 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&&)>&& completionHandler)
void ProvisionalPageProxy::decidePolicyForNavigationActionAsync(FrameInfoData&& frameInfo, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, CompletionHandler<void(PolicyDecision&&)>&& completionHandler)
{
if (!validateInput(frameInfo.frameID, navigationID))
return completionHandler({ });

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

void ProvisionalPageProxy::decidePolicyForResponse(FrameInfoData&& frameInfo, uint64_t navigationID, const WebCore::ResourceResponse& response, const WebCore::ResourceRequest& request, bool canShowMIMEType, const String& downloadAttribute, CompletionHandler<void(PolicyDecision&&)>&& completionHandler)
Expand Down Expand Up @@ -432,7 +432,7 @@ void ProvisionalPageProxy::backForwardGoToItem(const WebCore::BackForwardItemIde
m_page->backForwardGoToItemShared(m_process.copyRef(), identifier, WTFMove(completionHandler));
}

void ProvisionalPageProxy::decidePolicyForNavigationActionSync(FrameInfoData&& frameInfo, 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)
void ProvisionalPageProxy::decidePolicyForNavigationActionSync(FrameInfoData&& frameInfo, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, CompletionHandler<void(PolicyDecision&&)>&& reply)
{
if (!frameInfo.isMainFrame || (m_mainFrame && m_mainFrame->frameID() != frameInfo.frameID) || navigationID != m_navigationID) {
reply(PolicyDecision { std::nullopt, WebCore::PolicyAction::Ignore, navigationID });
Expand All @@ -445,7 +445,7 @@ void ProvisionalPageProxy::decidePolicyForNavigationActionSync(FrameInfoData&& f
}
ASSERT(m_mainFrame);

m_page->decidePolicyForNavigationActionSyncShared(m_process.copyRef(), m_webPageID, WTFMove(frameInfo), 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), navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(reply));
}

void ProvisionalPageProxy::logDiagnosticMessageFromWebProcess(const String& message, const String& description, WebCore::ShouldSample shouldSample)
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/ProvisionalPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ 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(FrameInfoData&&, 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 decidePolicyForNavigationActionAsync(FrameInfoData&&, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, CompletionHandler<void(PolicyDecision&&)>&&);
void decidePolicyForResponse(FrameInfoData&&, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, const String& downloadAttribute, CompletionHandler<void(PolicyDecision&&)>&&);
void didChangeProvisionalURLForFrame(WebCore::FrameIdentifier, uint64_t navigationID, URL&&);
void didPerformServerRedirect(const String& sourceURLString, const String& destinationURLString, WebCore::FrameIdentifier);
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(FrameInfoData&&, 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&&, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo, std::optional<WebPageProxyIdentifier> originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, CompletionHandler<void(PolicyDecision&&)>&&);
void backForwardAddItem(BackForwardListItemState&&);
void didDestroyNavigation(uint64_t navigationID);
#if USE(QUICK_LOOK)
Expand Down
Loading

0 comments on commit a68a8b8

Please sign in to comment.