- Revision
- 121929
- Author
- benja...@webkit.org
- Date
- 2012-07-05 17:20:44 -0700 (Thu, 05 Jul 2012)
Log Message
Double release of resources if the load is canceled in a callback of ResourceLoader::didFinishLoading
https://bugs.webkit.org/show_bug.cgi?id=90431
Patch by Benjamin Poulain <bpoul...@apple.com> on 2012-07-05
Reviewed by Anders Carlsson.
Source/WebCore:
In ResourceLoader::didFinishLoadingOnePart(), we invoke didFinishLoad() on the WebKit client. If WebKit
causes the current frame to cancel the load synchronously, the resources are already freed when
ResourceLoader::didFinishLoadingOnePart() ends.
When ResourceLoader::didFinishLoading() subsequently invokes releaseResources(), we are releasing the
resources a second time.
This patch add a second check for cancellation after invoking ResourceLoader::didFinishLoadingOnePart() to
avoid such issues.
The previous check at the beginning of ResourceLoader::didFinishLoading() has been removed because it is
redundant with ResourceLoader::didFinishLoadingOnePart().
* loader/ResourceLoader.cpp:
(WebCore::ResourceLoader::didFinishLoading):
(WebCore::ResourceLoader::didFinishLoadingOnePart):
Tools:
Add a Mac API test.
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.html: Added.
* TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.mm: Added.
(-[CancelLoadFromResourceLoadDelegate webView:resource:didFinishLoadingFromDataSource:]):
(-[CancelLoadFromResourceLoadDelegateFrameLoadDelegate webView:didFinishLoadForFrame:]):
(TestWebKitAPI):
(TestWebKitAPI::TEST):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (121928 => 121929)
--- trunk/Source/WebCore/ChangeLog 2012-07-06 00:04:05 UTC (rev 121928)
+++ trunk/Source/WebCore/ChangeLog 2012-07-06 00:20:44 UTC (rev 121929)
@@ -1,3 +1,26 @@
+2012-07-05 Benjamin Poulain <bpoul...@apple.com>
+
+ Double release of resources if the load is canceled in a callback of ResourceLoader::didFinishLoading
+ https://bugs.webkit.org/show_bug.cgi?id=90431
+
+ Reviewed by Anders Carlsson.
+
+ In ResourceLoader::didFinishLoadingOnePart(), we invoke didFinishLoad() on the WebKit client. If WebKit
+ causes the current frame to cancel the load synchronously, the resources are already freed when
+ ResourceLoader::didFinishLoadingOnePart() ends.
+ When ResourceLoader::didFinishLoading() subsequently invokes releaseResources(), we are releasing the
+ resources a second time.
+
+ This patch add a second check for cancellation after invoking ResourceLoader::didFinishLoadingOnePart() to
+ avoid such issues.
+
+ The previous check at the beginning of ResourceLoader::didFinishLoading() has been removed because it is
+ redundant with ResourceLoader::didFinishLoadingOnePart().
+
+ * loader/ResourceLoader.cpp:
+ (WebCore::ResourceLoader::didFinishLoading):
+ (WebCore::ResourceLoader::didFinishLoadingOnePart):
+
2012-07-05 Simon Fraser <simon.fra...@apple.com>
Add a utility method for hasOverflowClip() or hasClip()
Modified: trunk/Source/WebCore/loader/ResourceLoader.cpp (121928 => 121929)
--- trunk/Source/WebCore/loader/ResourceLoader.cpp 2012-07-06 00:04:05 UTC (rev 121928)
+++ trunk/Source/WebCore/loader/ResourceLoader.cpp 2012-07-06 00:20:44 UTC (rev 121929)
@@ -288,18 +288,19 @@
void ResourceLoader::didFinishLoading(double finishTime)
{
- // If load has been cancelled after finishing (which could happen with a
- // _javascript_ that changes the window location), do nothing.
+ didFinishLoadingOnePart(finishTime);
+
+ // If the load has been cancelled by a delegate in response to didFinishLoad(), do not release
+ // the resources a second time, they have been released by cancel.
if (m_cancelled)
return;
- ASSERT(!m_reachedTerminalState);
-
- didFinishLoadingOnePart(finishTime);
releaseResources();
}
void ResourceLoader::didFinishLoadingOnePart(double finishTime)
{
+ // If load has been cancelled after finishing (which could happen with a
+ // _javascript_ that changes the window location), do nothing.
if (m_cancelled)
return;
ASSERT(!m_reachedTerminalState);
Modified: trunk/Tools/ChangeLog (121928 => 121929)
--- trunk/Tools/ChangeLog 2012-07-06 00:04:05 UTC (rev 121928)
+++ trunk/Tools/ChangeLog 2012-07-06 00:20:44 UTC (rev 121929)
@@ -1,3 +1,20 @@
+2012-07-05 Benjamin Poulain <bpoul...@apple.com>
+
+ Double release of resources if the load is canceled in a callback of ResourceLoader::didFinishLoading
+ https://bugs.webkit.org/show_bug.cgi?id=90431
+
+ Reviewed by Anders Carlsson.
+
+ Add a Mac API test.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.html: Added.
+ * TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.mm: Added.
+ (-[CancelLoadFromResourceLoadDelegate webView:resource:didFinishLoadingFromDataSource:]):
+ (-[CancelLoadFromResourceLoadDelegateFrameLoadDelegate webView:didFinishLoadForFrame:]):
+ (TestWebKitAPI):
+ (TestWebKitAPI::TEST):
+
2012-07-05 Dave Tharp <dth...@codeaurora.org>
Adding myself as committer to committers.py
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (121928 => 121929)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2012-07-06 00:04:05 UTC (rev 121928)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2012-07-06 00:20:44 UTC (rev 121929)
@@ -19,6 +19,8 @@
1ADBEFAE130C689C00D61D19 /* ForceRepaint.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1ADBEFAD130C689C00D61D19 /* ForceRepaint.cpp */; };
1ADBEFE3130C6AA100D61D19 /* simple-accelerated-compositing.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 1ADBEFBC130C6A0100D61D19 /* simple-accelerated-compositing.html */; };
1AEDE22613E5E7E700E62FE8 /* InjectedBundleControllerMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1AEDE22413E5E7A000E62FE8 /* InjectedBundleControllerMac.mm */; };
+ 26DF5A5E15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */; };
+ 26DF5A6315A2A27E003689C2 /* CancelLoadFromResourceLoadDelegate.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 26DF5A6115A2A22B003689C2 /* CancelLoadFromResourceLoadDelegate.html */; };
333B9CE21277F23100FEFCE3 /* PreventEmptyUserAgent.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 333B9CE11277F23100FEFCE3 /* PreventEmptyUserAgent.cpp */; };
33BE5AF5137B5A6C00705813 /* MouseMoveAfterCrash.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 33BE5AF4137B5A6C00705813 /* MouseMoveAfterCrash.cpp */; };
33BE5AF9137B5AAE00705813 /* MouseMoveAfterCrash_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 33BE5AF8137B5AAE00705813 /* MouseMoveAfterCrash_Bundle.cpp */; };
@@ -189,6 +191,7 @@
B55F11BE15191A0600915916 /* Ahem.ttf in Copy Resources */,
B55F11B71517D03300915916 /* attributedStringCustomFont.html in Copy Resources */,
76E182DF154767E600F1FADD /* auto-submitting-form.html in Copy Resources */,
+ 26DF5A6315A2A27E003689C2 /* CancelLoadFromResourceLoadDelegate.html in Copy Resources */,
5142B2731517C8C800C32B19 /* ContextMenuCanCopyURL.html in Copy Resources */,
9B4F8FA7159D52DD002D9F94 /* HTMLCollectionNamedItem.html in Copy Resources */,
9B26FCCA159D16DE00CC3765 /* HTMLFormCollectionNamedItem.html in Copy Resources */,
@@ -231,6 +234,8 @@
1ADBEFAD130C689C00D61D19 /* ForceRepaint.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ForceRepaint.cpp; sourceTree = "<group>"; };
1ADBEFBC130C6A0100D61D19 /* simple-accelerated-compositing.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "simple-accelerated-compositing.html"; sourceTree = "<group>"; };
1AEDE22413E5E7A000E62FE8 /* InjectedBundleControllerMac.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = InjectedBundleControllerMac.mm; sourceTree = "<group>"; };
+ 26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CancelLoadFromResourceLoadDelegate.mm; sourceTree = "<group>"; };
+ 26DF5A6115A2A22B003689C2 /* CancelLoadFromResourceLoadDelegate.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = CancelLoadFromResourceLoadDelegate.html; sourceTree = "<group>"; };
333B9CE11277F23100FEFCE3 /* PreventEmptyUserAgent.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PreventEmptyUserAgent.cpp; sourceTree = "<group>"; };
33BE5AF4137B5A6C00705813 /* MouseMoveAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MouseMoveAfterCrash.cpp; sourceTree = "<group>"; };
33BE5AF8137B5AAE00705813 /* MouseMoveAfterCrash_Bundle.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MouseMoveAfterCrash_Bundle.cpp; sourceTree = "<group>"; };
@@ -663,6 +668,7 @@
C07E6CB013FD737C0038B22B /* Resources */,
379028B514FABD92007E6B43 /* AcceptsFirstMouse.mm */,
B55F119F1516834F00915916 /* AttributedString.mm */,
+ 26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */,
5142B2701517C88B00C32B19 /* ContextMenuCanCopyURL.mm */,
37DC678B140D7C5000ABCCDB /* DOMRangeOfString.mm */,
C07E6CAE13FD67650038B22B /* DynamicDeviceScaleFactor.mm */,
@@ -691,6 +697,7 @@
B55F11B9151916E600915916 /* Ahem.ttf */,
B55F11B01517A2C400915916 /* attributedStringCustomFont.html */,
379028B814FABE49007E6B43 /* acceptsFirstMouse.html */,
+ 26DF5A6115A2A22B003689C2 /* CancelLoadFromResourceLoadDelegate.html */,
5142B2721517C89100C32B19 /* ContextMenuCanCopyURL.html */,
37DC678F140D7D3A00ABCCDB /* DOMRangeOfString.html */,
9B4F8FA6159D52CA002D9F94 /* HTMLCollectionNamedItem.html */,
@@ -907,6 +914,7 @@
52B8CF9615868CF000281053 /* SetDocumentURI.mm in Sources */,
9B26FC6C159D061000CC3765 /* HTMLFormCollectionNamedItem.mm in Sources */,
9B4F8FA4159D52B1002D9F94 /* HTMLCollectionNamedItem.mm in Sources */,
+ 26DF5A5E15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Added: trunk/Tools/TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.html (0 => 121929)
--- trunk/Tools/TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.html (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.html 2012-07-06 00:20:44 UTC (rev 121929)
@@ -0,0 +1,8 @@
+<script>
+ function cancelLoadAndJumpToBlank() {
+ window.stop();
+ window.location = "about:blank";
+ }
+</script>
+<img src=""
+<script src=""
\ No newline at end of file
Added: trunk/Tools/TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.mm (0 => 121929)
--- trunk/Tools/TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.mm (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.mm 2012-07-06 00:20:44 UTC (rev 121929)
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2012 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+#import "PlatformUtilities.h"
+#import <wtf/RetainPtr.h>
+
+@interface CancelLoadFromResourceLoadDelegate : NSObject {
+ size_t resourceCount;
+}
+
+@end
+
+@implementation CancelLoadFromResourceLoadDelegate
+
+- (void)webView:(WebView *)sender resource:(id)identifier didFinishLoadingFromDataSource:(WebDataSource *)dataSource
+{
+ // Break the load once we have loaded the <script> and <img>.
+ ++resourceCount;
+ if (resourceCount > 2)
+ [sender stringByEvaluatingJavaScriptFromString:@"cancelLoadAndJumpToBlank()"];
+}
+@end
+
+
+static bool didFinishLoad = false;
+
+@interface CancelLoadFromResourceLoadDelegateFrameLoadDelegate : NSObject
+@end
+
+@implementation CancelLoadFromResourceLoadDelegateFrameLoadDelegate
+- (void)webView:(WebView *)sender didFinishLoadForFrame:(WebFrame *)frame
+{
+ if ([[sender mainFrameURL] isEqualToString:@"about:blank"])
+ didFinishLoad = true;
+}
+@end
+
+namespace TestWebKitAPI {
+
+TEST(WebKit1, CancelLoadFromResourceLoadDelegate)
+{
+ NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
+
+ RetainPtr<WebView> webView(AdoptNS, [[WebView alloc] initWithFrame:NSMakeRect(0, 0, 120, 200) frameName:nil groupName:nil]);
+
+ RetainPtr<CancelLoadFromResourceLoadDelegate> resourceLoadDelegate(AdoptNS, [[CancelLoadFromResourceLoadDelegate alloc] init]);
+ webView.get().resourceLoadDelegate = resourceLoadDelegate.get();
+ RetainPtr<CancelLoadFromResourceLoadDelegateFrameLoadDelegate> frameLoadDelegate(AdoptNS, [[CancelLoadFromResourceLoadDelegateFrameLoadDelegate alloc] init]);
+ webView.get().frameLoadDelegate = frameLoadDelegate.get();
+
+ [[webView.get() mainFrame] loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"CancelLoadFromResourceLoadDelegate" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]];
+
+ Util::run(&didFinishLoad);
+
+ [pool drain];
+ // If we finished without crashing, the test passed.
+}
+
+} // namespace TestWebKitAPI
