WebKit Bugzilla
Attachment 341049 Details for
Bug 185895
: REGRESSION(r232081): Broke two API tests (Requested by bfulgham_ on #webkit).
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
ROLLOUT of r232081
bug-185895-20180522201624.patch (text/plain), 20.93 KB, created by
WebKit Commit Bot
on 2018-05-22 17:16:25 PDT
(
hide
)
Description:
ROLLOUT of r232081
Filename:
MIME Type:
Creator:
WebKit Commit Bot
Created:
2018-05-22 17:16:25 PDT
Size:
20.93 KB
patch
obsolete
>Subversion Revision: 232091 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index a34892f5d1575bcf2bd0188d4765899a3c555e40..40a00c9927694ae057b0670154bc85fe511ebd29 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,16 @@ >+2018-05-22 Commit Queue <commit-queue@webkit.org> >+ >+ 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 >+ > 2018-05-22 Dean Jackson <dino@apple.com> > > Optimized path zoom animation needs a valid UIImage and CGRect >diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp >index 7ed4f64c921181e48af9bd603259eed55d1b883a..9e09bd35e0d34fc74a4907b92692ce023264d20d 100644 >--- a/Source/WebCore/loader/DocumentLoader.cpp >+++ b/Source/WebCore/loader/DocumentLoader.cpp >@@ -1,5 +1,5 @@ > /* >- * Copyright (C) 2006-2018 Apple Inc. All rights reserved. >+ * Copyright (C) 2006-2017 Apple Inc. All rights reserved. > * Copyright (C) 2011 Google Inc. All rights reserved. > * > * Redistribution and use in source and binary forms, with or without >@@ -640,7 +640,7 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc > if (!didReceiveRedirectResponse && shouldContinue != ShouldContinue::ForSuspension) > return completionHandler(WTFMove(newRequest)); > >- auto navigationPolicyCompletionHandler = [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) mutable { >+ auto navigationPolicyCompletionHandler = [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, FormState*, ShouldContinue shouldContinue) mutable { > m_waitingForNavigationPolicy = false; > switch (shouldContinue) { > case ShouldContinue::ForSuspension: >diff --git a/Source/WebCore/loader/FormState.cpp b/Source/WebCore/loader/FormState.cpp >index f8700b1ad3831a3d43163081a8f0c062b6ca06f4..9fa8bdd3e1cc36ab11ccde5f3019652894bfd05b 100644 >--- a/Source/WebCore/loader/FormState.cpp >+++ b/Source/WebCore/loader/FormState.cpp >@@ -1,5 +1,5 @@ > /* >- * Copyright (C) 2006-2018 Apple Inc. All rights reserved. >+ * Copyright (C) 2006-2017 Apple Inc. All rights reserved. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions >@@ -52,7 +52,7 @@ Ref<FormState> FormState::create(HTMLFormElement& form, StringPairVector&& textF > void FormState::willDetachPage() > { > // Beartrap for <rdar://problem/37579354> >- RELEASE_ASSERT(hasOneRef()); >+ RELEASE_ASSERT_NOT_REACHED(); > } > > } >diff --git a/Source/WebCore/loader/FormState.h b/Source/WebCore/loader/FormState.h >index 77584329af16424650c00db8a793d9d955f5050f..2634a97b1236c976234e50da7d5c20397cdcc2fb 100644 >--- a/Source/WebCore/loader/FormState.h >+++ b/Source/WebCore/loader/FormState.h >@@ -1,5 +1,5 @@ > /* >- * Copyright (C) 2006-2018 Apple Inc. All rights reserved. >+ * Copyright (C) 2006-2017 Apple Inc. All rights reserved. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions >@@ -29,7 +29,6 @@ > #pragma once > > #include "FrameDestructionObserver.h" >-#include <wtf/WeakPtr.h> > #include <wtf/text/WTFString.h> > > namespace WebCore { >@@ -50,8 +49,6 @@ public: > Document& sourceDocument() const { return m_sourceDocument; } > FormSubmissionTrigger formSubmissionTrigger() const { return m_formSubmissionTrigger; } > >- auto& weakPtrFactory() const { return m_weakFactory; } >- > private: > FormState(HTMLFormElement&, StringPairVector&& textFieldValues, Document&, FormSubmissionTrigger); > void willDetachPage() override; >@@ -60,7 +57,6 @@ private: > StringPairVector m_textFieldValues; > Ref<Document> m_sourceDocument; > FormSubmissionTrigger m_formSubmissionTrigger; >- WeakPtrFactory<FormState> m_weakFactory; > }; > > } // namespace WebCore >diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp >index 89d5dc91216fc642b0dfff500300d1e8c9d17e7b..30d00253cde8a74837b0d475c0f17351f5aa8af8 100644 >--- a/Source/WebCore/loader/FrameLoader.cpp >+++ b/Source/WebCore/loader/FrameLoader.cpp >@@ -1,5 +1,5 @@ > /* >- * Copyright (C) 2006-2018 Apple Inc. All rights reserved. >+ * Copyright (C) 2006-2016 Apple Inc. All rights reserved. > * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies) > * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/) > * Copyright (C) 2008 Alp Toker <alp@atoker.com> >@@ -1228,7 +1228,7 @@ void FrameLoader::loadFrameRequest(FrameLoadRequest&& request, Event* event, For > else > loadType = FrameLoadType::Standard; > >- auto completionHandler = [this, protectedFrame = makeRef(m_frame), formState = makeWeakPtr(formState), frameName = request.frameName()] { >+ auto completionHandler = [this, protectedFrame = makeRef(m_frame), formState = makeRefPtr(formState), frameName = request.frameName()] { > // FIXME: It's possible this targetFrame will not be the same frame that was targeted by the actual > // load if frame names have changed. > Frame* sourceFrame = formState ? formState->sourceDocument().frame() : &m_frame; >@@ -1338,8 +1338,8 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref > > if (!targetFrame && !frameName.isEmpty()) { > action = action.copyWithShouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicyToApply(m_frame, frameLoadRequest)); >- 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) { >- continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy); >+ 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) { >+ continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy); > completionHandler(); > }); > return; >@@ -1358,7 +1358,7 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref > oldDocumentLoader->setLastCheckedRequest(ResourceRequest()); > policyChecker().stopCheck(); > policyChecker().setLoadType(newLoadType); >- policyChecker().checkNavigationPolicy(WTFMove(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) { >+ policyChecker().checkNavigationPolicy(WTFMove(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) { > continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes); > }, PolicyDecisionMode::Synchronous); > return; >@@ -1419,8 +1419,8 @@ void FrameLoader::load(FrameLoadRequest&& request) > > if (request.shouldCheckNewWindowPolicy()) { > NavigationAction action { request.requester(), request.resourceRequest(), InitiatedByMainFrame::Unknown, NavigationType::Other, request.shouldOpenExternalURLsPolicy() }; >- 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) { >- continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress); >+ policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request.resourceRequest()), nullptr, request.frameName(), [this] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) { >+ continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress); > }); > > return; >@@ -1533,7 +1533,7 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t > oldDocumentLoader->setTriggeringAction(action); > oldDocumentLoader->setLastCheckedRequest(ResourceRequest()); > policyChecker().stopCheck(); >- policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) { >+ policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) { > continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes); > }, PolicyDecisionMode::Synchronous); > return; >@@ -1567,8 +1567,8 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t > return; > } > >- 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) { >- continueLoadAfterNavigationPolicy(request, formState.get(), shouldContinue, allowNavigationToInvalidURL); >+ 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) { >+ continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL); > completionHandler(); > }); > } >@@ -2861,13 +2861,13 @@ void FrameLoader::loadPostRequest(FrameLoadRequest&& request, const String& refe > > if (!frameName.isEmpty()) { > // The search for a target frame is done earlier in the case of form submission. >- if (auto* targetFrame = formState ? nullptr : findFrameForNavigation(frameName)) { >- targetFrame->loader().loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, formState, allowNavigationToInvalidURL, WTFMove(completionHandler)); >+ if (Frame* targetFrame = formState ? 0 : findFrameForNavigation(frameName)) { >+ targetFrame->loader().loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL, WTFMove(completionHandler)); > return; > } > >- 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) { >- continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy); >+ 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) { >+ continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy); > completionHandler(); > }); > return; >@@ -2875,7 +2875,7 @@ void FrameLoader::loadPostRequest(FrameLoadRequest&& request, const String& refe > > // must grab this now, since this load may stop the previous load and clear this flag > bool isRedirect = m_quickRedirectComing; >- loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, formState, allowNavigationToInvalidURL, [this, isRedirect, protectedFrame = makeRef(m_frame), completionHandler = WTFMove(completionHandler)] { >+ loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL, [this, isRedirect, protectedFrame = makeRef(m_frame), completionHandler = WTFMove(completionHandler)] { > if (isRedirect) { > m_quickRedirectComing = false; > if (m_provisionalDocumentLoader) >diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp >index 1be70e14e7388ad8276e85cfaa27e093b2997285..b6690ea9665a1515c97a63adc9407732b9819127 100644 >--- a/Source/WebCore/loader/PolicyChecker.cpp >+++ b/Source/WebCore/loader/PolicyChecker.cpp >@@ -1,5 +1,5 @@ > /* >- * Copyright (C) 2006-2018 Apple Inc. All rights reserved. >+ * Copyright (C) 2006-2016 Apple Inc. All rights reserved. > * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies) > * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/) > * >@@ -109,7 +109,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec > // Don't ask more than once for the same request or if we are loading an empty URL. > // This avoids confusion on the part of the client. > if (equalIgnoringHeaderFields(request, loader->lastCheckedRequest()) || (!request.isNull() && request.url().isEmpty())) { >- function(ResourceRequest(request), { }, ShouldContinue::Yes); >+ function(ResourceRequest(request), nullptr, ShouldContinue::Yes); > loader->setLastCheckedRequest(WTFMove(request)); > return; > } >@@ -124,7 +124,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec > #endif > if (isBackForwardLoadType(m_loadType)) > m_loadType = FrameLoadType::Reload; >- function(WTFMove(request), { }, shouldContinue ? ShouldContinue::Yes : ShouldContinue::No); >+ function(WTFMove(request), nullptr, shouldContinue ? ShouldContinue::Yes : ShouldContinue::No); > return; > } > >@@ -134,7 +134,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec > // reveal that the frame was blocked. This way, it looks like any other cross-origin page load. > m_frame.ownerElement()->dispatchEvent(Event::create(eventNames().loadEvent, false, false)); > } >- function(WTFMove(request), { }, ShouldContinue::No); >+ function(WTFMove(request), nullptr, ShouldContinue::No); > return; > } > >@@ -147,7 +147,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec > #if USE(QUICK_LOOK) > // Always allow QuickLook-generated URLs based on the protocol scheme. > if (!request.isNull() && isQuickLookPreviewURL(request.url())) >- return function(WTFMove(request), makeWeakPtr(formState), ShouldContinue::Yes); >+ return function(WTFMove(request), formState, ShouldContinue::Yes); > #endif > > #if ENABLE(CONTENT_FILTERING) >@@ -168,7 +168,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec > > m_delegateIsDecidingNavigationPolicy = true; > String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute(); >- 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 { >+ 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 { > m_delegateIsDecidingNavigationPolicy = false; > > switch (policyAction) { >@@ -185,7 +185,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec > handleUnimplementablePolicy(m_frame.loader().client().cannotShowURLError(request)); > return function({ }, nullptr, ShouldContinue::No); > } >- return function(WTFMove(request), WTFMove(formState), ShouldContinue::Yes); >+ return function(WTFMove(request), formState.get(), ShouldContinue::Yes); > } > ASSERT_NOT_REACHED(); > }); >@@ -201,7 +201,7 @@ void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, Re > > auto blobURLLifetimeExtension = extendBlobURLLifetimeIfNecessary(request); > >- 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 { >+ 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 { > switch (policyAction) { > case PolicyAction::Download: > frame->loader().client().startDownload(request); >@@ -213,7 +213,7 @@ void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, Re > // It is invalid to get a "Suspend" policy for new windows, as the old document is not going away. > RELEASE_ASSERT_NOT_REACHED(); > case PolicyAction::Use: >- function(request, WTFMove(formState), frameName, navigationAction, ShouldContinue::Yes); >+ function(request, formState.get(), frameName, navigationAction, ShouldContinue::Yes); > return; > } > ASSERT_NOT_REACHED(); >diff --git a/Source/WebCore/loader/PolicyChecker.h b/Source/WebCore/loader/PolicyChecker.h >index 6f112119146c48b4eb4e9b02681dc0e9b2af56f0..c329ec43b81e35e8b73abe6e601d13d4250852af 100644 >--- a/Source/WebCore/loader/PolicyChecker.h >+++ b/Source/WebCore/loader/PolicyChecker.h >@@ -1,5 +1,5 @@ > /* >- * Copyright (C) 2006-2018 Apple Inc. All rights reserved. >+ * Copyright (C) 2006-2016 Apple Inc. All rights reserved. > * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/) > * > * Redistribution and use in source and binary forms, with or without >@@ -31,7 +31,6 @@ > > #include "FrameLoaderTypes.h" > #include "ResourceRequest.h" >-#include <wtf/WeakPtr.h> > #include <wtf/text/WTFString.h> > > #if ENABLE(CONTENT_FILTERING) >@@ -60,8 +59,8 @@ enum class ShouldContinue { > > enum class PolicyDecisionMode { Synchronous, Asynchronous }; > >-using NewWindowPolicyDecisionFunction = CompletionHandler<void(const ResourceRequest&, WeakPtr<FormState>&&, const String& frameName, const NavigationAction&, ShouldContinue)>; >-using NavigationPolicyDecisionFunction = CompletionHandler<void(ResourceRequest&&, WeakPtr<FormState>&&, ShouldContinue)>; >+using NewWindowPolicyDecisionFunction = CompletionHandler<void(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, ShouldContinue)>; >+using NavigationPolicyDecisionFunction = CompletionHandler<void(ResourceRequest&&, FormState*, ShouldContinue)>; > > class PolicyChecker { > WTF_MAKE_NONCOPYABLE(PolicyChecker);
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=341049">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 185895
: 341049