⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 236480 in webkit


Ignore:
Timestamp:
Sep 25, 2018, 3:17:51 PM (8 years ago)
Author:
Chris Dumez
Message:

Revert some of the changes in r236471
​https://bugs.webkit.org/show_bug.cgi?id=189973

Reviewed by Alex Christensen.

Revert some of the changes in r236471 as they should not be needed. In particular,
it should not be possible for the DecidePolicyForNavigationActionSync IPC to get
processed *before* the DidCreateMainFrame / DidCreateSubframe ones because those
use IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply. They are thus
processed early when necessary, the same way as synchronous IPC messages.

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::didCreateMainFrame):
(WebKit::WebPageProxy::didCreateSubframe):
(WebKit::WebPageProxy::decidePolicyForNavigationActionAsync):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
(WebKit::WebPageProxy::decidePolicyForNavigationActionSync):

  • UIProcess/WebPageProxy.h:
  • UIProcess/WebPageProxy.messages.in:
  • WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:

(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):

  • WebProcess/WebPage/WebFrame.cpp:

(WebKit::WebFrame::createWithCoreMainFrame):
(WebKit::WebFrame::createSubframe):

Location:
trunk/Source/WebKit
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r236477 r236480  
     12018-09-25  Chris Dumez  <cdumez@apple.com>
     2
     3        Revert some of the changes in r236471
     4        https://bugs.webkit.org/show_bug.cgi?id=189973
     5
     6        Reviewed by Alex Christensen.
     7
     8        Revert some of the changes in r236471 as they should not be needed. In particular,
     9        it should not be possible for the DecidePolicyForNavigationActionSync IPC to get
     10        processed *before* the DidCreateMainFrame / DidCreateSubframe ones because those
     11        use IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply. They are thus
     12        processed early when necessary, the same way as synchronous IPC messages.
     13
     14        * UIProcess/WebPageProxy.cpp:
     15        (WebKit::WebPageProxy::didCreateMainFrame):
     16        (WebKit::WebPageProxy::didCreateSubframe):
     17        (WebKit::WebPageProxy::decidePolicyForNavigationActionAsync):
     18        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
     19        (WebKit::WebPageProxy::decidePolicyForNavigationActionSync):
     20        * UIProcess/WebPageProxy.h:
     21        * UIProcess/WebPageProxy.messages.in:
     22        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
     23        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
     24        * WebProcess/WebPage/WebFrame.cpp:
     25        (WebKit::WebFrame::createWithCoreMainFrame):
     26        (WebKit::WebFrame::createSubframe):
     27
    1282018-09-25  Sihui Liu  <sihui_liu@apple.com>
    229

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r236471 r236480  
    33583358void WebPageProxy::didCreateMainFrame(uint64_t frameID)
    33593359{
    3360     if (m_mainFrame && m_mainFrame->frameID() == frameID)
    3361         return;
    3362 
    33633360    PageClientProtector protector(pageClient());
    33643361
    … …  
    33833380
    33843381    MESSAGE_CHECK(m_mainFrame);
    3385 
    3386     if (m_process->webFrame(frameID))
    3387         return;
    3388 
    33893382    MESSAGE_CHECK(m_process->canCreateFrame(frameID));
    33903383   
    … …  
    40043997void WebPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, uint64_t listenerID)
    40053998{
    4006     auto* frame = m_process->webFrame(frameID);
    4007     MESSAGE_CHECK(frame);
    4008 
    4009     decidePolicyForNavigationAction(*frame, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, PolicyDecisionSender::create([this, protectedThis = makeRef(*this), frameID, listenerID] (auto... args) {
     3999    decidePolicyForNavigationAction(frameID, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, PolicyDecisionSender::create([this, protectedThis = makeRef(*this), frameID, listenerID] (auto... args) {
    40104000        m_process->send(Messages::WebPage::DidReceivePolicyDecision(frameID, listenerID, args...), m_pageID);
    40114001    }));
    40124002}
    40134003
    4014 void WebPageProxy::decidePolicyForNavigationAction(WebFrameProxy& frame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&& sender)
     4004void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&& sender)
    40154005{
    40164006    LOG(Loading, "WebPageProxy::decidePolicyForNavigationAction - Original URL %s, current target URL %s", originalRequest.url().string().utf8().data(), request.url().string().utf8().data());
    … …  
    40244014        m_pageLoadState.clearPendingAPIRequestURL(transaction);
    40254015
     4016    auto* frame = m_process->webFrame(frameID);
     4017    MESSAGE_CHECK(frame);
    40264018    MESSAGE_CHECK_URL(request.url());
    40274019    MESSAGE_CHECK_URL(originalRequest.url());
    … …  
    40604052
    40614053#if ENABLE(CONTENT_FILTERING)
    4062     if (frame.didHandleContentFilterUnblockNavigation(request))
     4054    if (frame->didHandleContentFilterUnblockNavigation(request))
    40634055        return receivedPolicyDecision(PolicyAction::Ignore, m_navigationState->navigation(newNavigationID), std::nullopt, WTFMove(sender));
    40644056#else
    … …  
    40664058#endif
    40674059
    4068     auto listener = makeRef(frame.setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(frame), sender = WTFMove(sender), navigation] (WebCore::PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&&) mutable {
     4060    auto listener = makeRef(frame->setUpPolicyListenerProxy([this, protectedThis = makeRef(*this), frame = makeRef(*frame), sender = WTFMove(sender), navigation] (WebCore::PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, Vector<Ref<SafeBrowsingResult>>&&) mutable {
    40694061        // FIXME: do something with the SafeBrowsingResults.
    40704062        receivedNavigationPolicyDecision(policyAction, navigation.get(), processSwapRequestedByClient, frame, policies, WTFMove(sender));
    … …  
    40734065        beginSafeBrowsingCheck(request.url(), listener);
    40744066
    4075     API::Navigation* mainFrameNavigation = frame.isMainFrame() ? navigation.get() : nullptr;
     4067    API::Navigation* mainFrameNavigation = frame->isMainFrame() ? navigation.get() : nullptr;
    40764068    WebFrameProxy* originatingFrame = m_process->webFrame(originatingFrameInfoData.frameID);
    40774069
    40784070    if (auto* resourceLoadStatisticsStore = websiteDataStore().resourceLoadStatistics())
    4079         resourceLoadStatisticsStore->logFrameNavigation(frame, URL(URL(), m_pageLoadState.url()), request, redirectResponse.url());
     4071        resourceLoadStatisticsStore->logFrameNavigation(*frame, URL(URL(), m_pageLoadState.url()), request, redirectResponse.url());
    40804072
    40814073    if (m_policyClient)
    4082         m_policyClient->decidePolicyForNavigationAction(*this, &frame, WTFMove(navigationActionData), originatingFrame, originalRequest, WTFMove(request), WTFMove(listener), m_process->transformHandlesToObjects(userData.object()).get());
     4074        m_policyClient->decidePolicyForNavigationAction(*this, frame, WTFMove(navigationActionData), originatingFrame, originalRequest, WTFMove(request), WTFMove(listener), m_process->transformHandlesToObjects(userData.object()).get());
    40834075    else {
    4084         auto destinationFrameInfo = API::FrameInfo::create(frame, frameSecurityOrigin.securityOrigin());
     4076        auto destinationFrameInfo = API::FrameInfo::create(*frame, frameSecurityOrigin.securityOrigin());
    40854077        RefPtr<API::FrameInfo> sourceFrameInfo;
    4086         if (!fromAPI && originatingFrame == &frame)
     4078        if (!fromAPI && originatingFrame == frame)
    40874079            sourceFrameInfo = destinationFrameInfo.copyRef();
    40884080        else if (!fromAPI)
    … …  
    41004092}
    41014093
    4102 void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
     4094void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
    41034095{
    41044096    auto sender = PolicyDecisionSender::create(WTFMove(reply));
    4105 
    4106     auto* frame = m_process->webFrame(frameID);
    4107     if (!frame) {
    4108         // This synchronous IPC message was processed before the asynchronous DidCreateMainFrame / DidCreateSubframe one so we do not know about this frameID yet.
    4109         if (isMainFrame)
    4110             didCreateMainFrame(frameID);
    4111         else
    4112             didCreateSubframe(frameID);
    4113 
    4114         frame = m_process->webFrame(frameID);
    4115         RELEASE_ASSERT(frame);
    4116     }
    41174097   
    4118     decidePolicyForNavigationAction(*frame, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, sender.copyRef());
     4098    decidePolicyForNavigationAction(frameID, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, sender.copyRef());
    41194099
    41204100    // If the client did not respond synchronously, proceed with the load.

  • trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r236471 r236480  
    14351435    void didDestroyNavigation(uint64_t navigationID);
    14361436
    1437     void decidePolicyForNavigationAction(WebFrameProxy&, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&&);
     1437    void decidePolicyForNavigationAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&&);
    14381438    void decidePolicyForNavigationActionAsync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, uint64_t listenerID);
    1439     void decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
     1439    void decidePolicyForNavigationActionSync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
    14401440    void decidePolicyForNewWindowAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, NavigationActionData&&, WebCore::ResourceRequest&&, const String& frameName, uint64_t listenerID, const UserData&);
    14411441    void decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&);

  • trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in

    r236471 r236480  
    109109    DecidePolicyForResponse(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, WebCore::ResourceResponse response, WebCore::ResourceRequest request, bool canShowMIMEType, uint64_t listenerID, WebKit::UserData userData)
    110110    DecidePolicyForNavigationActionAsync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, uint64_t listenerID)
    111     DecidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck) -> (enum WebCore::PolicyAction policyAction, uint64_t newNavigationID, WebKit::DownloadID downloadID, std::optional<WebKit::WebsitePoliciesData> websitePolicies) Delayed
     111    DecidePolicyForNavigationActionSync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck) -> (enum WebCore::PolicyAction policyAction, uint64_t newNavigationID, WebKit::DownloadID downloadID, std::optional<WebKit::WebsitePoliciesData> websitePolicies) Delayed
    112112    DecidePolicyForNewWindowAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, struct WebKit::NavigationActionData navigationActionData, WebCore::ResourceRequest request, String frameName, uint64_t listenerID, WebKit::UserData userData)
    113113    UnableToImplementPolicy(uint64_t frameID, WebCore::ResourceError error, WebKit::UserData userData)

  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp

    r236471 r236480  
    895895        std::optional<WebsitePoliciesData> websitePolicies;
    896896
    897         if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->frameID(), m_frame->isMainFrame(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), shouldSkipSafeBrowsingCheck), Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(policyAction, newNavigationID, downloadID, websitePolicies))) {
     897        if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), shouldSkipSafeBrowsingCheck), Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(policyAction, newNavigationID, downloadID, websitePolicies))) {
    898898            m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
    899899            return;

  • trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp

    r235562 r236480  
    112112{
    113113    auto frame = create(std::unique_ptr<WebFrameLoaderClient>(static_cast<WebFrameLoaderClient*>(&coreFrame->loader().client())));
     114    // DispatchMessageEvenWhenWaitingForSyncReply SendOption is needed to ensure that this IPC always gets received before the DecidePolicyForNavigationSync synchronous
     115    // IPC for this frame.
    114116    page->send(Messages::WebPageProxy::DidCreateMainFrame(frame->frameID()), page->pageID(), IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
    115117
    … …  
    123125{
    124126    auto frame = create(std::make_unique<WebFrameLoaderClient>());
     127    // DispatchMessageEvenWhenWaitingForSyncReply SendOption is needed to ensure that this IPC always gets received before the DecidePolicyForNavigationSync synchronous
     128    // IPC for this frame.
    125129    page->send(Messages::WebPageProxy::DidCreateSubframe(frame->frameID()), page->pageID(), IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
    126130
Note: See TracChangeset for help on using the changeset viewer.