Title: [218050] trunk
- Revision
- 218050
- Author
- [email protected]
- Date
- 2017-06-10 00:25:11 -0700 (Sat, 10 Jun 2017)
Log Message
[QuickLook] PreviewLoader needs to check if its ResourceLoader has reached the terminal state before calling didReceiveResponse() and friends
https://bugs.webkit.org/show_bug.cgi?id=173190
<rdar://problem/31360659>
Reviewed by Brady Eidson.
Source/WebCore:
WebPreviewLoader's SubresourceLoader can reach a terminal state while data is loading from
QLPreviewConverter (the user can cancel, for instance). We can't call functions like
didReceiveResponse() in this state, because the loader no longer points to a CachedResource,
leading to null pointer dereferences.
Fix this in WebPreviewLoader by checking if the SubresourceLoader is in a terminal state
before calling didReceiveResponse(), didReceiveData(), didFinishLoading(), and didFail().
Fixes web process crashes in the QuickLook.CancelNavigationAfterResponse API test.
* loader/ios/PreviewLoader.mm:
(-[WebPreviewLoader _sendDidReceiveResponseIfNecessary]):
(-[WebPreviewLoader connection:didReceiveData:lengthReceived:]):
(-[WebPreviewLoader connectionDidFinishLoading:]):
(-[WebPreviewLoader connection:didFailWithError:]):
Tools:
Added a release assert that tries to create a proper test failure if the web process crashes.
* TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm:
(-[QuickLookDecidePolicyDelegate _webViewWebProcessDidCrash:]):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (218049 => 218050)
--- trunk/Source/WebCore/ChangeLog 2017-06-10 07:13:22 UTC (rev 218049)
+++ trunk/Source/WebCore/ChangeLog 2017-06-10 07:25:11 UTC (rev 218050)
@@ -1,3 +1,27 @@
+2017-06-10 Andy Estes <[email protected]>
+
+ [QuickLook] PreviewLoader needs to check if its ResourceLoader has reached the terminal state before calling didReceiveResponse() and friends
+ https://bugs.webkit.org/show_bug.cgi?id=173190
+ <rdar://problem/31360659>
+
+ Reviewed by Brady Eidson.
+
+ WebPreviewLoader's SubresourceLoader can reach a terminal state while data is loading from
+ QLPreviewConverter (the user can cancel, for instance). We can't call functions like
+ didReceiveResponse() in this state, because the loader no longer points to a CachedResource,
+ leading to null pointer dereferences.
+
+ Fix this in WebPreviewLoader by checking if the SubresourceLoader is in a terminal state
+ before calling didReceiveResponse(), didReceiveData(), didFinishLoading(), and didFail().
+
+ Fixes web process crashes in the QuickLook.CancelNavigationAfterResponse API test.
+
+ * loader/ios/PreviewLoader.mm:
+ (-[WebPreviewLoader _sendDidReceiveResponseIfNecessary]):
+ (-[WebPreviewLoader connection:didReceiveData:lengthReceived:]):
+ (-[WebPreviewLoader connectionDidFinishLoading:]):
+ (-[WebPreviewLoader connection:didFailWithError:]):
+
2017-06-09 Chris Dumez <[email protected]>
Use WTF::Function instead of std::function in indexeddb code
Modified: trunk/Source/WebCore/loader/ios/PreviewLoader.mm (218049 => 218050)
--- trunk/Source/WebCore/loader/ios/PreviewLoader.mm 2017-06-10 07:13:22 UTC (rev 218049)
+++ trunk/Source/WebCore/loader/ios/PreviewLoader.mm 2017-06-10 07:25:11 UTC (rev 218050)
@@ -117,6 +117,7 @@
- (void)_sendDidReceiveResponseIfNecessary
{
+ ASSERT(!_resourceLoader->reachedTerminalState());
if (_hasSentDidReceiveResponse)
return;
@@ -135,6 +136,9 @@
- (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data lengthReceived:(long long)lengthReceived
{
ASSERT_UNUSED(connection, !connection);
+ if (_resourceLoader->reachedTerminalState())
+ return;
+
[self _sendDidReceiveResponseIfNecessary];
// QuickLook code sends us a nil data at times. The check below is the same as the one in
@@ -146,6 +150,9 @@
- (void)connectionDidFinishLoading:(NSURLConnection *)connection
{
ASSERT_UNUSED(connection, !connection);
+ if (_resourceLoader->reachedTerminalState())
+ return;
+
ASSERT(_hasSentDidReceiveResponse);
NetworkLoadMetrics emptyMetrics;
@@ -160,6 +167,8 @@
- (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error
{
ASSERT_UNUSED(connection, !connection);
+ if (_resourceLoader->reachedTerminalState())
+ return;
if (!isQuickLookPasswordError(error)) {
[self _sendDidReceiveResponseIfNecessary];
Modified: trunk/Tools/ChangeLog (218049 => 218050)
--- trunk/Tools/ChangeLog 2017-06-10 07:13:22 UTC (rev 218049)
+++ trunk/Tools/ChangeLog 2017-06-10 07:25:11 UTC (rev 218050)
@@ -1,3 +1,16 @@
+2017-06-10 Andy Estes <[email protected]>
+
+ [QuickLook] PreviewLoader needs to check if its ResourceLoader has reached the terminal state before calling didReceiveResponse() and friends
+ https://bugs.webkit.org/show_bug.cgi?id=173190
+ <rdar://problem/31360659>
+
+ Reviewed by Brady Eidson.
+
+ Added a release assert that tries to create a proper test failure if the web process crashes.
+
+ * TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm:
+ (-[QuickLookDecidePolicyDelegate _webViewWebProcessDidCrash:]):
+
2017-06-09 Wenson Hsieh <[email protected]>
[iOS DnD] Add a hook to perform two-step drops in editable content
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm (218049 => 218050)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm 2017-06-10 07:13:22 UTC (rev 218049)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm 2017-06-10 07:25:11 UTC (rev 218050)
@@ -107,6 +107,11 @@
isDone = true;
}
+- (void)_webViewWebProcessDidCrash:(WKWebView *)webView
+{
+ RELEASE_ASSERT_NOT_REACHED();
+}
+
@end
@interface QuickLookFrameLoadDelegate : NSObject <WebFrameLoadDelegate>
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
