Attachment 343516 Details for Bug 187008 – Patch

[patch] Patch

bug-187008-20180625110414.patch (text/plain), 3.22 KB, created by Brent Fulgham on 2018-06-25 11:04:14 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Brent Fulgham

Created: 2018-06-25 11:04:14 PDT

Size: 3.22 KB

patch

obsolete

>Subversion Revision: 233083
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c2a64044d161bf4173d5acd1217a40f8f3eb7de6..1e4bfc2202bbd8301858d3ddaa3f860d492a51d4 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-06-25  Brent Fulgham  <bfulgham@apple.com>
>+
>+        REGRESSION(r229722): WebKitLegacy clients can crash when loading alternate page
>+        https://bugs.webkit.org/show_bug.cgi?id=187008
>+        
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The new call to 'clearProvisionalLoadForPolicyCheck' added in r229722 broke loading
>+        behavior in WebKitLegacy.
>+
>+        1. We can now enter 'cancelPolicyCheckIfNeeded' without a Frame loader, in what appears
>+           to be a recursive call during the load cancellation (the 'm_waitingForContentPolicy'
>+           and 'm_waitingForNavigationPolicy' have already been nulled). It seems like we should
>+           return early here, or perhaps just move the RELEASE_ASSERT inside the case where we
>+           have an active policy check happening.
>+
>+        2. We also enter FrameLoader::checkContentPolicy without an active document loader. We
>+           should recognize this case and handle it, rather than trying to dereference a nullptr
>+           document loader.
>+
>+        * loader/DocumentLoader.cpp:
>+        (WebCore::DocumentLoader::cancelPolicyCheckIfNeeded): Move the RELEASE_ASSERT inside the
>+        conditional where the frameLoader is actually used.
>+        * loader/FrameLoader.cpp:
>+        (WebCore::FrameLoader::checkContentPolicy): Recognize that the activeDocumentLoader may
>+        be nullptr at this point, and take appropriate action (rather than crashing).
>+
> 2018-06-22  Thibault Saunier  <tsaunier@igalia.com>
> 
>         [GStreamer] Avoid sending SELECT_STREAM events when nothing changed
>diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
>index 30775bc8377e1159eb0e79721cd0087c8d517f33..954961494cd71174d0e38f6fedcdeb20a62bfcb9 100644
>--- a/Source/WebCore/loader/DocumentLoader.cpp
>+++ b/Source/WebCore/loader/DocumentLoader.cpp
>@@ -1813,9 +1813,8 @@ void DocumentLoader::loadMainResource(ResourceRequest&& request)
> 
> void DocumentLoader::cancelPolicyCheckIfNeeded()
> {
>-    RELEASE_ASSERT(frameLoader());
>-
>     if (m_waitingForContentPolicy || m_waitingForNavigationPolicy) {
>+        RELEASE_ASSERT(frameLoader());
>         frameLoader()->policyChecker().stopCheck();
>         m_waitingForContentPolicy = false;
>         m_waitingForNavigationPolicy = false;
>diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
>index 9d726ab48f40d8f8cff18df0e2c2896b25249fbd..d3dea8afe462e60b2c851222915a6c37696dbbb3 100644
>--- a/Source/WebCore/loader/FrameLoader.cpp
>+++ b/Source/WebCore/loader/FrameLoader.cpp
>@@ -362,6 +362,12 @@ void FrameLoader::setDefersLoading(bool defers)
> 
> void FrameLoader::checkContentPolicy(const ResourceResponse& response, ContentPolicyDecisionFunction&& function)
> {
>+    if (!activeDocumentLoader()) {
>+        // Load was cancelled
>+        function(PolicyAction::Ignore);
>+        return;
>+    }
>+
>     client().dispatchDecidePolicyForResponse(response, activeDocumentLoader()->request(), WTFMove(function));
> }
>

Flags:

cdumez: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 187008: 343516