Changeset 222472 in webkit


Ignore:
Timestamp:
Sep 25, 2017 3:33:19 PM (7 years ago)
Author:
achristensen@apple.com
Message:

Stop using PolicyCallback
https://bugs.webkit.org/show_bug.cgi?id=176088

Reviewed by Andy Estes.

Source/WebCore:

This is what lambdas are for. No change in behavior.
Cancelling the callback is now the responsibility of the FrameLoaderClient, to whom we have given the lambda.
That is why there are changes in WebKit and WebKitLegacy where the FrameLoaderClient::cancelPolicyCheck finds the lambda it has stored.

  • CMakeLists.txt:
  • WebCore.xcodeproj/project.pbxproj:
  • loader/DocumentLoader.cpp:
  • loader/FrameLoader.cpp:
  • loader/FrameLoaderClient.h:
  • loader/PolicyCallback.cpp: Removed.
  • loader/PolicyCallback.h: Removed.
  • loader/PolicyChecker.cpp:

(WebCore::PolicyChecker::checkNavigationPolicy):
(WebCore::PolicyChecker::checkNewWindowPolicy):
(WebCore::PolicyChecker::stopCheck):
(WebCore::PolicyChecker::continueAfterNavigationPolicy): Deleted.

  • loader/PolicyChecker.h:

(WebCore::PolicyChecker::setSuggestedFilename): Deleted.

Source/WebKit:

  • WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
  • WebProcess/WebPage/WebFrame.cpp:

(WebKit::WebFrame::invalidatePolicyListener):

  • WebProcess/WebPage/WebFrame.h:
  • WebProcess/WebPage/WebInspector.cpp:

Source/WebKitLegacy/mac:

  • WebCoreSupport/WebFrameLoaderClient.mm:

(-[WebFramePolicyListener invalidate]):
(-[WebFramePolicyListener receivedPolicyDecision:]):

Location:
trunk/Source
Files:
2 deleted
15 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/CMakeLists.txt

    r222395 r222472  
    21172117    loader/NetscapePlugInStreamLoader.cpp
    21182118    loader/PingLoader.cpp
    2119     loader/PolicyCallback.cpp
    21202119    loader/PolicyChecker.cpp
    21212120    loader/ProgressTracker.cpp
  • trunk/Source/WebCore/ChangeLog

    r222471 r222472  
     12017-09-25  Alex Christensen  <achristensen@webkit.org>
     2
     3        Stop using PolicyCallback
     4        https://bugs.webkit.org/show_bug.cgi?id=176088
     5
     6        Reviewed by Andy Estes.
     7
     8        This is what lambdas are for. No change in behavior.
     9        Cancelling the callback is now the responsibility of the FrameLoaderClient, to whom we have given the lambda.
     10        That is why there are changes in WebKit and WebKitLegacy where the FrameLoaderClient::cancelPolicyCheck finds the lambda it has stored.
     11
     12        * CMakeLists.txt:
     13        * WebCore.xcodeproj/project.pbxproj:
     14        * loader/DocumentLoader.cpp:
     15        * loader/FrameLoader.cpp:
     16        * loader/FrameLoaderClient.h:
     17        * loader/PolicyCallback.cpp: Removed.
     18        * loader/PolicyCallback.h: Removed.
     19        * loader/PolicyChecker.cpp:
     20        (WebCore::PolicyChecker::checkNavigationPolicy):
     21        (WebCore::PolicyChecker::checkNewWindowPolicy):
     22        (WebCore::PolicyChecker::stopCheck):
     23        (WebCore::PolicyChecker::continueAfterNavigationPolicy): Deleted.
     24        * loader/PolicyChecker.h:
     25        (WebCore::PolicyChecker::setSuggestedFilename): Deleted.
     26
    1272017-09-25  Youenn Fablet  <youenn@apple.com>
    228
  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r222467 r222472  
    41064106                96ABA42314BCB80E00D56204 /* GraphicsContext3DOpenGLCommon.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 96ABA42214BCB80E00D56204 /* GraphicsContext3DOpenGLCommon.cpp */; };
    41074107                9703E1BF15DC4E37001F24C8 /* JSVoidCallback.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 97E9EC8B15DC492F004F2E71 /* JSVoidCallback.cpp */; };
    4108                 97059977107D975200A50A7C /* PolicyCallback.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 97059973107D975200A50A7C /* PolicyCallback.cpp */; };
    4109                 97059978107D975200A50A7C /* PolicyCallback.h in Headers */ = {isa = PBXBuildFile; fileRef = 97059974107D975200A50A7C /* PolicyCallback.h */; settings = {ATTRIBUTES = (Private, ); }; };
    41104108                97059979107D975200A50A7C /* PolicyChecker.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 97059975107D975200A50A7C /* PolicyChecker.cpp */; };
    41114109                9705997A107D975200A50A7C /* PolicyChecker.h in Headers */ = {isa = PBXBuildFile; fileRef = 97059976107D975200A50A7C /* PolicyChecker.h */; settings = {ATTRIBUTES = (Private, ); }; };
     
    1247112469                952076021F2675F9007D2AAB /* CallTracerTypes.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CallTracerTypes.h; sourceTree = "<group>"; };
    1247212470                96ABA42214BCB80E00D56204 /* GraphicsContext3DOpenGLCommon.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = GraphicsContext3DOpenGLCommon.cpp; sourceTree = "<group>"; };
    12473                 97059973107D975200A50A7C /* PolicyCallback.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PolicyCallback.cpp; sourceTree = "<group>"; };
    12474                 97059974107D975200A50A7C /* PolicyCallback.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PolicyCallback.h; sourceTree = "<group>"; };
    1247512471                97059975107D975200A50A7C /* PolicyChecker.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PolicyChecker.cpp; sourceTree = "<group>"; };
    1247612472                97059976107D975200A50A7C /* PolicyChecker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PolicyChecker.h; sourceTree = "<group>"; };
     
    2446424460                                D0FF2A5B11F8C45A007E74E0 /* PingLoader.cpp */,
    2446524461                                D0FF2A5C11F8C45A007E74E0 /* PingLoader.h */,
    24466                                 97059973107D975200A50A7C /* PolicyCallback.cpp */,
    24467                                 97059974107D975200A50A7C /* PolicyCallback.h */,
    2446824462                                97059975107D975200A50A7C /* PolicyChecker.cpp */,
    2446924463                                97059976107D975200A50A7C /* PolicyChecker.h */,
     
    2958829582                                3FF813A71DBA8640009BF001 /* PointerLockController.h in Headers */,
    2958929583                                84730D921248F0B300D3A9C9 /* PointLightSource.h in Headers */,
    29590                                 97059978107D975200A50A7C /* PolicyCallback.h in Headers */,
    2959129584                                9705997A107D975200A50A7C /* PolicyChecker.h in Headers */,
    2959229585                                FD45A957175D414C00C21EC8 /* PolygonShape.h in Headers */,
     
    3357933572                                5CFC4350192409E300A0D3B5 /* PointerLockController.cpp in Sources */,
    3358033573                                A1E1154613015C4E0054AC8C /* PointLightSource.cpp in Sources */,
    33581                                 97059977107D975200A50A7C /* PolicyCallback.cpp in Sources */,
    3358233574                                97059979107D975200A50A7C /* PolicyChecker.cpp in Sources */,
    3358333575                                FD45A959175D417100C21EC8 /* PolygonShape.cpp in Sources */,
  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r222467 r222472  
    7676#include "TextResourceDecoder.h"
    7777#include <wtf/Assertions.h>
     78#include <wtf/CompletionHandler.h>
    7879#include <wtf/NeverDestroyed.h>
    7980#include <wtf/Ref.h>
  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r222467 r222472  
    120120#include "WindowFeatures.h"
    121121#include "XMLDocumentParser.h"
     122#include <wtf/CompletionHandler.h>
    122123#include <wtf/CurrentTime.h>
    123124#include <wtf/Ref.h>
  • trunk/Source/WebCore/loader/FrameLoaderClient.h

    r222456 r222472  
    8585class Page;
    8686class PluginViewBase;
    87 class PolicyChecker;
    8887class PreviewLoaderClient;
    8988class ProtectionSpace;
  • trunk/Source/WebCore/loader/PolicyChecker.cpp

    r222456 r222472  
    4545#include "HTMLFrameOwnerElement.h"
    4646#include "HTMLPlugInElement.h"
     47#include <wtf/CompletionHandler.h>
    4748
    4849#if USE(QUICK_LOOK)
     
    7677}
    7778
    78 void PolicyChecker::checkNavigationPolicy(const ResourceRequest& newRequest, bool didReceiveRedirectResponse, NavigationPolicyDecisionFunction function)
     79void PolicyChecker::checkNavigationPolicy(const ResourceRequest& newRequest, bool didReceiveRedirectResponse, NavigationPolicyDecisionFunction&& function)
    7980{
    8081    checkNavigationPolicy(newRequest, didReceiveRedirectResponse, m_frame.loader().activeDocumentLoader(), nullptr, WTFMove(function));
    8182}
    8283
    83 void PolicyChecker::checkNavigationPolicy(const ResourceRequest& request, bool didReceiveRedirectResponse, DocumentLoader* loader, FormState* formState, NavigationPolicyDecisionFunction function)
     84void PolicyChecker::checkNavigationPolicy(const ResourceRequest& request, bool didReceiveRedirectResponse, DocumentLoader* loader, FormState* formState, NavigationPolicyDecisionFunction&& function)
    8485{
    8586    NavigationAction action = loader->triggeringAction();
     
    9293    // This avoids confusion on the part of the client.
    9394    if (equalIgnoringHeaderFields(request, loader->lastCheckedRequest()) || (!request.isNull() && request.url().isEmpty())) {
    94         function(request, 0, true);
     95        function(request, nullptr, true);
    9596        loader->setLastCheckedRequest(request);
    9697        return;
     
    107108        if (isBackForwardLoadType(m_loadType))
    108109            m_loadType = FrameLoadType::Reload;
    109         function(request, 0, shouldContinue);
     110        function(request, nullptr, shouldContinue);
    110111        return;
    111112    }
     
    117118            m_frame.ownerElement()->dispatchEvent(Event::create(eventNames().loadEvent, false, false));
    118119        }
    119         function(request, 0, false);
     120        function(request, nullptr, false);
    120121        return;
    121122    }
    122123
    123124    loader->setLastCheckedRequest(request);
    124 
    125     m_callback.set(request, formState, WTFMove(function));
    126125
    127126#if USE(QUICK_LOOK)
    128127    // Always allow QuickLook-generated URLs based on the protocol scheme.
    129     if (!request.isNull() && isQuickLookPreviewURL(request.url())) {
    130         continueAfterNavigationPolicy(PolicyUse);
    131         return;
    132     }
     128    if (!request.isNull() && isQuickLookPreviewURL(request.url()))
     129        return function(request, formState, true);
    133130#endif
    134131
     
    140137                frame->loader().reload();
    141138        });
    142         continueAfterNavigationPolicy(PolicyIgnore);
    143         return;
     139        return function({ }, nullptr, false);
    144140    }
    145141    m_contentFilterUnblockHandler = { };
     
    147143
    148144    m_delegateIsDecidingNavigationPolicy = true;
    149     m_suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
    150     m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, [this](PolicyAction action) {
    151         continueAfterNavigationPolicy(action);
     145    String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
     146    ResourceRequest requestCopy = request;
     147    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, [this, function = WTFMove(function), request = WTFMove(requestCopy), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename)](PolicyAction policyAction) mutable {
     148        switch (policyAction) {
     149        case PolicyDownload:
     150            m_frame.loader().setOriginalURLForDownloadRequest(request);
     151            m_frame.loader().client().startDownload(request, suggestedFilename);
     152            FALLTHROUGH;
     153        case PolicyIgnore:
     154            return function({ }, nullptr, false);
     155        case PolicyUse:
     156            if (!m_frame.loader().client().canHandleRequest(request)) {
     157                handleUnimplementablePolicy(m_frame.loader().client().cannotShowURLError(request));
     158                return function({ }, nullptr, false);
     159            }
     160            return function(request, formState.get(), true);
     161        }
     162        ASSERT_NOT_REACHED();
    152163    });
    153164    m_delegateIsDecidingNavigationPolicy = false;
    154165}
    155166
    156 void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, const ResourceRequest& request, FormState* formState, const String& frameName, NewWindowPolicyDecisionFunction function)
     167void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, const ResourceRequest& request, FormState* formState, const String& frameName, NewWindowPolicyDecisionFunction&& function)
    157168{
    158169    if (m_frame.document() && m_frame.document()->isSandboxed(SandboxPopups))
    159         return continueAfterNavigationPolicy(PolicyIgnore);
     170        return function({ }, nullptr, { }, { }, false);
    160171
    161172    if (!DOMWindow::allowPopUp(m_frame))
    162         return continueAfterNavigationPolicy(PolicyIgnore);
    163 
    164     m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState, frameName, [frame = makeRef(m_frame), request, formState = makeRefPtr(formState), frameName, navigationAction, function = WTFMove(function)](PolicyAction policyAction) {
     173        return function({ }, nullptr, { }, { }, false);
     174
     175    m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState, frameName, [frame = makeRef(m_frame), request, formState = makeRefPtr(formState), frameName, navigationAction, function = WTFMove(function)](PolicyAction policyAction) mutable {
    165176        switch (policyAction) {
    166177        case PolicyDownload:
     
    181192{
    182193    m_frame.loader().client().cancelPolicyCheck();
    183     PolicyCallback callback = WTFMove(m_callback);
    184     callback.cancel();
    185194}
    186195
     
    188197{
    189198    handleUnimplementablePolicy(m_frame.loader().client().cannotShowMIMETypeError(response));
    190 }
    191 
    192 void PolicyChecker::continueAfterNavigationPolicy(PolicyAction policy)
    193 {
    194     PolicyCallback callback = WTFMove(m_callback);
    195 
    196     bool shouldContinue = policy == PolicyUse;
    197 
    198     switch (policy) {
    199         case PolicyIgnore:
    200             callback.clearRequest();
    201             break;
    202         case PolicyDownload: {
    203             ResourceRequest request = callback.request();
    204             m_frame.loader().setOriginalURLForDownloadRequest(request);
    205             m_frame.loader().client().startDownload(request, m_suggestedFilename);
    206             callback.clearRequest();
    207             break;
    208         }
    209         case PolicyUse: {
    210             ResourceRequest request(callback.request());
    211 
    212             if (!m_frame.loader().client().canHandleRequest(request)) {
    213                 handleUnimplementablePolicy(m_frame.loader().client().cannotShowURLError(callback.request()));
    214                 callback.clearRequest();
    215                 shouldContinue = false;
    216             }
    217             break;
    218         }
    219     }
    220 
    221     callback.call(shouldContinue);
    222199}
    223200
  • trunk/Source/WebCore/loader/PolicyChecker.h

    r222456 r222472  
    3131
    3232#include "FrameLoaderTypes.h"
    33 #include "PolicyCallback.h"
    3433#include "ResourceRequest.h"
    3534#include <wtf/text/WTFString.h>
     
    3837#include "ContentFilterUnblockHandler.h"
    3938#endif
     39
     40namespace WTF {
     41template<typename> class CompletionHandler;
     42}
    4043
    4144namespace WebCore {
     
    4851class ResourceResponse;
    4952
    50 using NewWindowPolicyDecisionFunction = WTF::Function<void(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, bool shouldContinue)>;
     53using NewWindowPolicyDecisionFunction = WTF::CompletionHandler<void(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, bool shouldContinue)>;
     54using NavigationPolicyDecisionFunction = WTF::CompletionHandler<void(const ResourceRequest&, FormState*, bool shouldContinue)>;
    5155
    5256class PolicyChecker {
     
    5660    explicit PolicyChecker(Frame&);
    5761
    58     void checkNavigationPolicy(const ResourceRequest&, bool didReceiveRedirectResponse, DocumentLoader*, FormState*, NavigationPolicyDecisionFunction);
    59     void checkNavigationPolicy(const ResourceRequest&, bool didReceiveRedirectResponse, NavigationPolicyDecisionFunction);
    60     void checkNewWindowPolicy(NavigationAction&&, const ResourceRequest&, FormState*, const String& frameName, NewWindowPolicyDecisionFunction);
     62    void checkNavigationPolicy(const ResourceRequest&, bool didReceiveRedirectResponse, DocumentLoader*, FormState*, NavigationPolicyDecisionFunction&&);
     63    void checkNavigationPolicy(const ResourceRequest&, bool didReceiveRedirectResponse, NavigationPolicyDecisionFunction&&);
     64    void checkNewWindowPolicy(NavigationAction&&, const ResourceRequest&, FormState*, const String& frameName, NewWindowPolicyDecisionFunction&&);
    6165
    6266    void stopCheck();
     
    6670    FrameLoadType loadType() const { return m_loadType; }
    6771    void setLoadType(FrameLoadType loadType) { m_loadType = loadType; }
    68 
    69     void setSuggestedFilename(const String& suggestedFilename) { m_suggestedFilename = suggestedFilename; }
    7072
    7173    bool delegateIsDecidingNavigationPolicy() const { return m_delegateIsDecidingNavigationPolicy; }
     
    7779
    7880private:
    79     void continueAfterNavigationPolicy(PolicyAction);
    80 
    8181    void handleUnimplementablePolicy(const ResourceError&);
    8282
     
    9090    // on navigation action delegate callbacks.
    9191    FrameLoadType m_loadType;
    92     PolicyCallback m_callback;
    93     String m_suggestedFilename;
    9492
    9593#if ENABLE(CONTENT_FILTERING)
  • trunk/Source/WebKit/ChangeLog

    r222468 r222472  
     12017-09-25  Alex Christensen  <achristensen@webkit.org>
     2
     3        Stop using PolicyCallback
     4        https://bugs.webkit.org/show_bug.cgi?id=176088
     5
     6        Reviewed by Andy Estes.
     7
     8        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
     9        * WebProcess/WebPage/WebFrame.cpp:
     10        (WebKit::WebFrame::invalidatePolicyListener):
     11        * WebProcess/WebPage/WebFrame.h:
     12        * WebProcess/WebPage/WebInspector.cpp:
     13
    1142017-09-25  Chris Dumez  <cdumez@apple.com>
    215
  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp

    r222468 r222472  
    7878#include <WebCore/PluginData.h>
    7979#include <WebCore/PluginDocument.h>
     80#include <WebCore/PolicyChecker.h>
    8081#include <WebCore/ProgressTracker.h>
    8182#include <WebCore/ResourceError.h>
  • trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp

    r222468 r222472  
    241241    m_policyDownloadID = { };
    242242    m_policyListenerID = 0;
    243     m_policyFunction = nullptr;
     243    if (auto function = std::exchange(m_policyFunction, nullptr))
     244        function(PolicyIgnore);
    244245}
    245246
     
    255256        return;
    256257
    257     ASSERT(m_policyFunction);
     258    if (!m_policyFunction)
     259        return;
    258260
    259261    FramePolicyFunction function = WTFMove(m_policyFunction);
  • trunk/Source/WebKit/WebProcess/WebPage/WebFrame.h

    r222456 r222472  
    3434#include <WebCore/FrameLoaderClient.h>
    3535#include <WebCore/FrameLoaderTypes.h>
    36 #include <WebCore/PolicyChecker.h>
    3736#include <wtf/Forward.h>
     37#include <wtf/HashMap.h>
    3838#include <wtf/RefPtr.h>
    3939#include <wtf/RetainPtr.h>
  • trunk/Source/WebKit/WebProcess/WebPage/WebInspector.cpp

    r219407 r222472  
    4242#include <WebCore/InspectorPageAgent.h>
    4343#include <WebCore/MainFrame.h>
     44#include <WebCore/NavigationAction.h>
    4445#include <WebCore/NotImplemented.h>
    4546#include <WebCore/Page.h>
  • trunk/Source/WebKitLegacy/mac/ChangeLog

    r222465 r222472  
     12017-09-25  Alex Christensen  <achristensen@webkit.org>
     2
     3        Stop using PolicyCallback
     4        https://bugs.webkit.org/show_bug.cgi?id=176088
     5
     6        Reviewed by Andy Estes.
     7
     8        * WebCoreSupport/WebFrameLoaderClient.mm:
     9        (-[WebFramePolicyListener invalidate]):
     10        (-[WebFramePolicyListener receivedPolicyDecision:]):
     11
    1122017-09-25  Sam Weinig  <sam@webkit.org>
    213
  • trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm

    r222456 r222472  
    24102410{
    24112411    _frame = nullptr;
     2412    if (auto policyFunction = std::exchange(_policyFunction, nullptr))
     2413        policyFunction(PolicyIgnore);
    24122414}
    24132415
     
    24262428        return;
    24272429
    2428     FramePolicyFunction policyFunction = WTFMove(_policyFunction);
    2429     _policyFunction = nullptr;
    2430 
    2431     ASSERT(policyFunction);
    2432     policyFunction(action);
     2430    if (auto policyFunction = std::exchange(_policyFunction, nullptr))
     2431        policyFunction(action);
    24332432}
    24342433
Note: See TracChangeset for help on using the changeset viewer.