Changeset 232093 in webkit


Ignore:
Timestamp:
May 22, 2018 5:17:45 PM (6 years ago)
Author:
commit-queue@webkit.org
Message:

Unreviewed, rolling out r232081.
https://bugs.webkit.org/show_bug.cgi?id=185895

Broke two API tests (Requested by bfulgham_ on #webkit).

Reverted changeset:

"Avoid keeping FormState alive longer than necessary"
https://bugs.webkit.org/show_bug.cgi?id=185877
https://trac.webkit.org/changeset/232081

Location:
trunk/Source/WebCore
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r232090 r232093  
     12018-05-22  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r232081.
     4        https://bugs.webkit.org/show_bug.cgi?id=185895
     5
     6        Broke two API tests (Requested by bfulgham_ on #webkit).
     7
     8        Reverted changeset:
     9
     10        "Avoid keeping FormState alive longer than necessary"
     11        https://bugs.webkit.org/show_bug.cgi?id=185877
     12        https://trac.webkit.org/changeset/232081
     13
    1142018-05-22  Dean Jackson  <dino@apple.com>
    215
  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r232081 r232093  
    11/*
    2  * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
    33 * Copyright (C) 2011 Google Inc. All rights reserved.
    44 *
     
    641641        return completionHandler(WTFMove(newRequest));
    642642
    643     auto navigationPolicyCompletionHandler = [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) mutable {
     643    auto navigationPolicyCompletionHandler = [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, FormState*, ShouldContinue shouldContinue) mutable {
    644644        m_waitingForNavigationPolicy = false;
    645645        switch (shouldContinue) {
  • trunk/Source/WebCore/loader/FormState.cpp

    r232081 r232093  
    11/*
    2  * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    5353{
    5454    // Beartrap for <rdar://problem/37579354>
    55     RELEASE_ASSERT(hasOneRef());
     55    RELEASE_ASSERT_NOT_REACHED();
    5656}
    5757
  • trunk/Source/WebCore/loader/FormState.h

    r232081 r232093  
    11/*
    2  * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    3030
    3131#include "FrameDestructionObserver.h"
    32 #include <wtf/WeakPtr.h>
    3332#include <wtf/text/WTFString.h>
    3433
     
    5150    FormSubmissionTrigger formSubmissionTrigger() const { return m_formSubmissionTrigger; }
    5251
    53     auto& weakPtrFactory() const { return m_weakFactory; }
    54 
    5552private:
    5653    FormState(HTMLFormElement&, StringPairVector&& textFieldValues, Document&, FormSubmissionTrigger);
     
    6158    Ref<Document> m_sourceDocument;
    6259    FormSubmissionTrigger m_formSubmissionTrigger;
    63     WeakPtrFactory<FormState> m_weakFactory;
    6460};
    6561
  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r232090 r232093  
    11/*
    2  * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
    33 * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
    44 * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
     
    12291229        loadType = FrameLoadType::Standard;
    12301230
    1231     auto completionHandler = [this, protectedFrame = makeRef(m_frame), formState = makeWeakPtr(formState), frameName = request.frameName()] {
     1231    auto completionHandler = [this, protectedFrame = makeRef(m_frame), formState = makeRefPtr(formState), frameName = request.frameName()] {
    12321232        // FIXME: It's possible this targetFrame will not be the same frame that was targeted by the actual
    12331233        // load if frame names have changed.
     
    13391339    if (!targetFrame && !frameName.isEmpty()) {
    13401340        action = action.copyWithShouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicyToApply(m_frame, frameLoadRequest));
    1341         policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request), formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
    1342             continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
     1341        policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request), formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
     1342            continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
    13431343            completionHandler();
    13441344        });
     
    13591359        policyChecker().stopCheck();
    13601360        policyChecker().setLoadType(newLoadType);
    1361         policyChecker().checkNavigationPolicy(WTFMove(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
     1361        policyChecker().checkNavigationPolicy(WTFMove(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) {
    13621362            continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
    13631363        }, PolicyDecisionMode::Synchronous);
     
    14201420    if (request.shouldCheckNewWindowPolicy()) {
    14211421        NavigationAction action { request.requester(), request.resourceRequest(), InitiatedByMainFrame::Unknown, NavigationType::Other, request.shouldOpenExternalURLsPolicy() };
    1422         policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request.resourceRequest()), nullptr, request.frameName(), [this] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
    1423             continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
     1422        policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request.resourceRequest()), nullptr, request.frameName(), [this] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
     1423            continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
    14241424        });
    14251425
     
    15341534        oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
    15351535        policyChecker().stopCheck();
    1536         policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
     1536        policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) {
    15371537            continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
    15381538        }, PolicyDecisionMode::Synchronous);
     
    15681568    }
    15691569
    1570     policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, ShouldContinue shouldContinue) {
    1571         continueLoadAfterNavigationPolicy(request, formState.get(), shouldContinue, allowNavigationToInvalidURL);
     1570    policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, ShouldContinue shouldContinue) {
     1571        continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
    15721572        completionHandler();
    15731573    });
     
    28622862    if (!frameName.isEmpty()) {
    28632863        // The search for a target frame is done earlier in the case of form submission.
    2864         if (auto* targetFrame = formState ? nullptr : findFrameForNavigation(frameName)) {
    2865             targetFrame->loader().loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, formState, allowNavigationToInvalidURL, WTFMove(completionHandler));
     2864        if (Frame* targetFrame = formState ? 0 : findFrameForNavigation(frameName)) {
     2865            targetFrame->loader().loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL, WTFMove(completionHandler));
    28662866            return;
    28672867        }
    28682868
    2869         policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(workingResourceRequest), formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
    2870             continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
     2869        policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(workingResourceRequest), WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
     2870            continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
    28712871            completionHandler();
    28722872        });
     
    28762876    // must grab this now, since this load may stop the previous load and clear this flag
    28772877    bool isRedirect = m_quickRedirectComing;
    2878     loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, formState, allowNavigationToInvalidURL, [this, isRedirect, protectedFrame = makeRef(m_frame), completionHandler = WTFMove(completionHandler)] {
     2878    loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL, [this, isRedirect, protectedFrame = makeRef(m_frame), completionHandler = WTFMove(completionHandler)] {
    28792879        if (isRedirect) {
    28802880            m_quickRedirectComing = false;
  • trunk/Source/WebCore/loader/PolicyChecker.cpp

    r232081 r232093  
    11/*
    2  * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
    33 * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
    44 * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
     
    110110    // This avoids confusion on the part of the client.
    111111    if (equalIgnoringHeaderFields(request, loader->lastCheckedRequest()) || (!request.isNull() && request.url().isEmpty())) {
    112         function(ResourceRequest(request), { }, ShouldContinue::Yes);
     112        function(ResourceRequest(request), nullptr, ShouldContinue::Yes);
    113113        loader->setLastCheckedRequest(WTFMove(request));
    114114        return;
     
    125125        if (isBackForwardLoadType(m_loadType))
    126126            m_loadType = FrameLoadType::Reload;
    127         function(WTFMove(request), { }, shouldContinue ? ShouldContinue::Yes : ShouldContinue::No);
     127        function(WTFMove(request), nullptr, shouldContinue ? ShouldContinue::Yes : ShouldContinue::No);
    128128        return;
    129129    }
     
    135135            m_frame.ownerElement()->dispatchEvent(Event::create(eventNames().loadEvent, false, false));
    136136        }
    137         function(WTFMove(request), { }, ShouldContinue::No);
     137        function(WTFMove(request), nullptr, ShouldContinue::No);
    138138        return;
    139139    }
     
    148148    // Always allow QuickLook-generated URLs based on the protocol scheme.
    149149    if (!request.isNull() && isQuickLookPreviewURL(request.url()))
    150         return function(WTFMove(request), makeWeakPtr(formState), ShouldContinue::Yes);
     150        return function(WTFMove(request), formState, ShouldContinue::Yes);
    151151#endif
    152152
     
    169169    m_delegateIsDecidingNavigationPolicy = true;
    170170    String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
    171     m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, policyDecisionMode, [this, function = WTFMove(function), request = ResourceRequest(request), formState = makeWeakPtr(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
     171    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, policyDecisionMode, [this, function = WTFMove(function), request = ResourceRequest(request), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
    172172        m_delegateIsDecidingNavigationPolicy = false;
    173173
     
    186186                return function({ }, nullptr, ShouldContinue::No);
    187187            }
    188             return function(WTFMove(request), WTFMove(formState), ShouldContinue::Yes);
     188            return function(WTFMove(request), formState.get(), ShouldContinue::Yes);
    189189        }
    190190        ASSERT_NOT_REACHED();
     
    202202    auto blobURLLifetimeExtension = extendBlobURLLifetimeIfNecessary(request);
    203203
    204     m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState, frameName, [frame = makeRef(m_frame), request, formState = makeWeakPtr(formState), frameName, navigationAction, function = WTFMove(function), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
     204    m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState, frameName, [frame = makeRef(m_frame), request, formState = makeRefPtr(formState), frameName, navigationAction, function = WTFMove(function), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
    205205        switch (policyAction) {
    206206        case PolicyAction::Download:
     
    214214            RELEASE_ASSERT_NOT_REACHED();
    215215        case PolicyAction::Use:
    216             function(request, WTFMove(formState), frameName, navigationAction, ShouldContinue::Yes);
     216            function(request, formState.get(), frameName, navigationAction, ShouldContinue::Yes);
    217217            return;
    218218        }
  • trunk/Source/WebCore/loader/PolicyChecker.h

    r232081 r232093  
    11/*
    2  * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
    33 * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
    44 *
     
    3232#include "FrameLoaderTypes.h"
    3333#include "ResourceRequest.h"
    34 #include <wtf/WeakPtr.h>
    3534#include <wtf/text/WTFString.h>
    3635
     
    6160enum class PolicyDecisionMode { Synchronous, Asynchronous };
    6261
    63 using NewWindowPolicyDecisionFunction = CompletionHandler<void(const ResourceRequest&, WeakPtr<FormState>&&, const String& frameName, const NavigationAction&, ShouldContinue)>;
    64 using NavigationPolicyDecisionFunction = CompletionHandler<void(ResourceRequest&&, WeakPtr<FormState>&&, ShouldContinue)>;
     62using NewWindowPolicyDecisionFunction = CompletionHandler<void(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, ShouldContinue)>;
     63using NavigationPolicyDecisionFunction = CompletionHandler<void(ResourceRequest&&, FormState*, ShouldContinue)>;
    6564
    6665class PolicyChecker {
Note: See TracChangeset for help on using the changeset viewer.