Changeset 228461 in webkit

Timestamp:

Feb 13, 2018 10:57:35 PM (6 years ago)

Author:

jmarcell@apple.com

Message:

Cherry-pick r228430. rdar://problem/37518683

Location:

branches/safari-605-branch

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/FrameLoader.cpp (modified) (1 diff)
• Source/WebCore/loader/NavigationScheduler.cpp (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (modified) (4 diffs)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm (added)

branches/safari-605-branch/Source/WebCore/ChangeLog

@@ +1 @@
+2018-02-13  Jason Marcell  <jmarcell@apple.com>
+
+        Cherry-pick r228430. rdar://problem/37518683
+
+    2018-02-13  Chris Dumez  <cdumez@apple.com>
+
+            REGRESSION (r228299): Broke reader mode in Safari
+            https://bugs.webkit.org/show_bug.cgi?id=182697
+            <rdar://problem/37399012>
+
+            Reviewed by Ryosuke Niwa.
+
+            Rework the fix for r228299 to be more targeted. I moved the policy check
+            cencelation from FrameLoader::stopLoading() to NavigationScheduler::schedule()
+            when a pending load is cancelled by another load. I have verified that the
+            sites fixed by r228299 still work with this more limited change. However,
+            reader mode is now working again.
+
+            The issue seems to be that we tell CFNetwork to continue with the load after
+            receiving the response, even if the client has not responded to the
+            decidePolicyForNavigationResponse delegate yet. As a result, CFNetwork sends
+            us the resource data and we may commit the provisional load before receiving
+            the policy response from the client. When the provisional load is committed,
+            we call FrameLoader::stopLoading() which after r228299 cancelled pending
+            policy checks. Because we did not wait for the policy check response to
+            commit the load, we would cancel it which would make the load fail.
+
+            The real fix here would be to make not tell CFNetwork to continue until after
+            we've received the policy delegate response. However, this is a larger and
+            riskier change at this point. I will follow-up on this issue.
+
+            Covered by new API test.
+
+            * loader/FrameLoader.cpp:
+            (WebCore::FrameLoader::stopLoading):
+            * loader/NavigationScheduler.cpp:
+            (WebCore::NavigationScheduler::schedule):
+
 2018-02-13  Jason Marcell  <jmarcell@apple.com>
 

branches/safari-605-branch/Source/WebCore/loader/FrameLoader.cpp

@@ -489 +489 @@
     }
 
-    policyChecker().stopCheck();
-
     // FIXME: This will cancel redirection timer, which really needs to be restarted when restoring the frame from b/f cache.
     m_frame.navigationScheduler().cancel();

branches/safari-605-branch/Source/WebCore/loader/NavigationScheduler.cpp

@@ -51 +51 @@
 #include "NavigationDisabler.h"
 #include "Page.h"
+#include "PolicyChecker.h"
 #include "ScriptController.h"
 #include "UserGestureIndicator.h"
@@ -521 +522 @@
         if (DocumentLoader* provisionalDocumentLoader = m_frame.loader().provisionalDocumentLoader())
             provisionalDocumentLoader->stopLoading();
+        m_frame.loader().policyChecker().stopCheck();
         m_frame.loader().stopLoading(UnloadEventPolicyUnloadAndPageHide);
     }

branches/safari-605-branch/Tools/ChangeLog

@@ +1 @@
+2018-02-13  Jason Marcell  <jmarcell@apple.com>
+
+        Cherry-pick r228430. rdar://problem/37518683
+
+    2018-02-13  Chris Dumez  <cdumez@apple.com>
+
+            REGRESSION (r228299): Broke reader mode in Safari
+            https://bugs.webkit.org/show_bug.cgi?id=182697
+            <rdar://problem/37399012>
+
+            Reviewed by Ryosuke Niwa.
+
+            Add API test coverage for responding asynchronously to the decidePolicyForNavigationResponse
+            delegate.
+
+            * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+            * TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm: Added.
+            (-[TestAsyncNavigationDelegate webView:didFinishNavigation:]):
+            (-[TestAsyncNavigationDelegate webView:didFailNavigation:withError:]):
+            (-[TestAsyncNavigationDelegate webView:didFailProvisionalNavigation:withError:]):
+            (-[TestAsyncNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
+            (-[TestAsyncNavigationDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
+            (TestWebKitAPI::TEST):
+
 2018-02-12  Jason Marcell  <jmarcell@apple.com>
 

branches/safari-605-branch/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

@@ -531 +531 @@
                 7CEFA9661AC0B9E200B910FD /* _WKUserContentExtensionStore.mm in Sources */ = {isa = PBXBuildFile; fileRef = 7CEFA9641AC0B9E200B910FD /* _WKUserContentExtensionStore.mm */; };
                 7CFBCAE51743238F00B2BFCF /* WillLoad_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 7CFBCAE31743238E00B2BFCF /* WillLoad_Bundle.cpp */; };
+                834138C7203261CA00F26960 /* AsyncPolicyForNavigationResponse.mm in Sources */ = {isa = PBXBuildFile; fileRef = 834138C6203261B900F26960 /* AsyncPolicyForNavigationResponse.mm */; };
                 8349D3C21DB96DDE004A9F65 /* ContextMenuDownload.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8349D3C11DB96DDA004A9F65 /* ContextMenuDownload.mm */; };
                 8349D3C41DB9728E004A9F65 /* link-with-download-attribute.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 8349D3C31DB9724F004A9F65 /* link-with-download-attribute.html */; };
@@ -1518 +1519 @@
                 7CFBCAE31743238E00B2BFCF /* WillLoad_Bundle.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WillLoad_Bundle.cpp; sourceTree = "<group>"; };
                 81B50192140F232300D9EB58 /* StringBuilder.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = StringBuilder.cpp; sourceTree = "<group>"; };
+                834138C6203261B900F26960 /* AsyncPolicyForNavigationResponse.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = AsyncPolicyForNavigationResponse.mm; sourceTree = "<group>"; };
                 8349D3C11DB96DDA004A9F65 /* ContextMenuDownload.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ContextMenuDownload.mm; sourceTree = "<group>"; };
                 8349D3C31DB9724F004A9F65 /* link-with-download-attribute.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "link-with-download-attribute.html"; sourceTree = "<group>"; };
@@ -2054 +2056 @@
                                 2DE71AFD1D49C0BD00904094 /* AnimatedResize.mm */,
                                 63F668201F97C3AA0032EE51 /* ApplicationManifest.mm */,
+                                834138C6203261B900F26960 /* AsyncPolicyForNavigationResponse.mm */,
                                 754CEC801F6722DC00D0039A /* AutoFillAvailable.mm */,
                                 2DD355351BD08378005DF4A7 /* AutoLayoutIntegration.mm */,
@@ -3339 +3342 @@
                                 63F668221F97F7F90032EE51 /* ApplicationManifest.mm in Sources */,
                                 6354F4D11F7C3AB500D89DF3 /* ApplicationManifestParser.cpp in Sources */,
+                                834138C7203261CA00F26960 /* AsyncPolicyForNavigationResponse.mm in Sources */,
                                 7CCE7EB41A411A7E00447C4C /* AttributedString.mm in Sources */,
                                 CDC8E48D1BC5CB4500594FEC /* AudioSessionCategoryIOS.mm in Sources */,