Diff
Modified: trunk/Source/WebKit2/ChangeLog (86944 => 86945)
--- trunk/Source/WebKit2/ChangeLog 2011-05-20 12:52:51 UTC (rev 86944)
+++ trunk/Source/WebKit2/ChangeLog 2011-05-20 13:24:29 UTC (rev 86945)
@@ -1,3 +1,26 @@
+2011-05-19 Adam Roben <[email protected]>
+
+ Don't try to process DownloadProxy messages twice (and robustify code that runs if we do)
+
+ Fixes <http://webkit.org/b/61142> <rdar://problem/9471680> REGRESSION (r86812): Crash
+ (preceded by assertion) in fastMalloc when downloading a file
+
+ Reviewed by Darin Adler.
+
+ * Platform/CoreIPC/ArgumentDecoder.cpp:
+ (CoreIPC::alignedBufferIsLargeEnoughToContain): Added. This helper function checks that the
+ given buffer is large enough to hold |size| bytes (and correctly handles the case where
+ we're already at the end or beyond the end of the buffer).
+
+ (CoreIPC::ArgumentDecoder::alignBufferPosition):
+ (CoreIPC::ArgumentDecoder::bufferIsLargeEnoughToContain):
+ Replaced old code that was vulnerable to underflow with the new helper function.
+
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::didReceiveSyncMessage): Added back an early return that was
+ mistakenly removed in r86812 so that we don't mistakenly pass DownloadProxy messages on to
+ a WebPageProxy after we've already handled them.
+
2011-05-19 Jer Noble <[email protected]>
Reviewed by Maciej Stachowiak.
Modified: trunk/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp (86944 => 86945)
--- trunk/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp 2011-05-20 12:52:51 UTC (rev 86944)
+++ trunk/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp 2011-05-20 13:24:29 UTC (rev 86945)
@@ -79,22 +79,27 @@
decodeUInt64(m_destinationID);
}
+static inline bool alignedBufferIsLargeEnoughToContain(const uint8_t* alignedPosition, const uint8_t* bufferEnd, size_t size)
+{
+ return bufferEnd >= alignedPosition && static_cast<size_t>(bufferEnd - alignedPosition) >= size;
+}
+
bool ArgumentDecoder::alignBufferPosition(unsigned alignment, size_t size)
{
- uint8_t* buffer = roundUpToAlignment(m_bufferPos, alignment);
- if (static_cast<size_t>(m_bufferEnd - buffer) < size) {
+ uint8_t* alignedPosition = roundUpToAlignment(m_bufferPos, alignment);
+ if (!alignedBufferIsLargeEnoughToContain(alignedPosition, m_bufferEnd, size)) {
// We've walked off the end of this buffer.
markInvalid();
return false;
}
- m_bufferPos = buffer;
+ m_bufferPos = alignedPosition;
return true;
}
bool ArgumentDecoder::bufferIsLargeEnoughToContain(unsigned alignment, size_t size) const
{
- return static_cast<size_t>(m_bufferEnd - roundUpToAlignment(m_bufferPos, alignment)) >= size;
+ return alignedBufferIsLargeEnoughToContain(roundUpToAlignment(m_bufferPos, alignment), m_bufferEnd, size);
}
bool ArgumentDecoder::decodeBytes(Vector<uint8_t>& buffer)
Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp (86944 => 86945)
--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2011-05-20 12:52:51 UTC (rev 86944)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2011-05-20 13:24:29 UTC (rev 86945)
@@ -270,6 +270,7 @@
if (messageID.is<CoreIPC::MessageClassWebContext>() || messageID.is<CoreIPC::MessageClassWebContextLegacy>()
|| messageID.is<CoreIPC::MessageClassDownloadProxy>() || messageID.is<CoreIPC::MessageClassWebIconDatabase>()) {
m_context->didReceiveSyncMessage(connection, messageID, arguments, reply);
+ return;
}
uint64_t pageID = arguments->destinationID();
Modified: trunk/Tools/ChangeLog (86944 => 86945)
--- trunk/Tools/ChangeLog 2011-05-20 12:52:51 UTC (rev 86944)
+++ trunk/Tools/ChangeLog 2011-05-20 13:24:29 UTC (rev 86945)
@@ -1,3 +1,32 @@
+2011-05-19 Adam Roben <[email protected]>
+
+ Test that the WebKit2 UI process doesn't crash when starting a download
+
+ Test for <http://webkit.org/b/61142> <rdar://problem/9471680> REGRESSION (r86812): Crash
+ (preceded by assertion) in fastMalloc when downloading a file
+
+ Reviewed by Darin Adler.
+
+ * TestWebKitAPI/Tests/WebKit2/18-characters.html: Added.
+
+ * TestWebKitAPI/Tests/WebKit2/DownloadDecideDestinationCrash.cpp: Added.
+ (TestWebKitAPI::decidePolicyForNavigationAction): Start a download.
+ (TestWebKitAPI::decideDestinationWithSuggestedFilename): Record that the download was
+ started, cancel the download, and return a bogus string.
+
+ (TestWebKitAPI::setContextDownloadClient):
+ (TestWebKitAPI::setPagePolicyClient):
+ Simple helper functions.
+
+ (TestWebKitAPI::TEST): Load 18-characters.html, which should trigger a download thanks to
+ our policy client, and run until we know that the download was started. If we haven't
+ crashed, we win!
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/win/TestWebKitAPI.vcproj:
+ * TestWebKitAPI/win/copy-resources.cmd:
+ Added new files.
+
2011-05-20 Kent Tamura <[email protected]>
Reviewed by Ryosuke Niwa.
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (86944 => 86945)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2011-05-20 12:52:51 UTC (rev 86944)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2011-05-20 13:24:29 UTC (rev 86945)
@@ -55,6 +55,7 @@
C01A23F21266156700C9ED55 /* spacebar-scrolling.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = C02B7882126615410026BF0F /* spacebar-scrolling.html */; };
C02B77F2126612140026BF0F /* SpacebarScrolling.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C02B77F1126612140026BF0F /* SpacebarScrolling.cpp */; };
C02B7854126613AE0026BF0F /* Carbon.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = C02B7853126613AE0026BF0F /* Carbon.framework */; };
+ C045F9451385C2EA00C0F3CD /* DownloadDecideDestinationCrash.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C045F9441385C2E900C0F3CD /* DownloadDecideDestinationCrash.cpp */; };
C0ADBE7C12FCA4D000D2C129 /* _javascript_Test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C0ADBE7A12FCA4D000D2C129 /* _javascript_Test.cpp */; };
C0ADBE8312FCA6AA00D2C129 /* RestoreSessionStateContainingFormData.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C0ADBE8212FCA6AA00D2C129 /* RestoreSessionStateContainingFormData.cpp */; };
C0ADBE9612FCA79B00D2C129 /* simple-form.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = C0ADBE8412FCA6B600D2C129 /* simple-form.html */; };
@@ -163,6 +164,8 @@
C02B77F1126612140026BF0F /* SpacebarScrolling.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SpacebarScrolling.cpp; sourceTree = "<group>"; };
C02B7853126613AE0026BF0F /* Carbon.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; name = Carbon.framework; sourceTree = SDKROOT; };
C02B7882126615410026BF0F /* spacebar-scrolling.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "spacebar-scrolling.html"; sourceTree = "<group>"; };
+ C045F9441385C2E900C0F3CD /* DownloadDecideDestinationCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DownloadDecideDestinationCrash.cpp; sourceTree = "<group>"; };
+ C045F9461385C2F800C0F3CD /* 18-characters.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "18-characters.html"; sourceTree = "<group>"; };
C0ADBE7A12FCA4D000D2C129 /* _javascript_Test.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = _javascript_Test.cpp; sourceTree = "<group>"; };
C0ADBE7B12FCA4D000D2C129 /* _javascript_Test.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = _javascript_Test.h; sourceTree = "<group>"; };
C0ADBE8212FCA6AA00D2C129 /* RestoreSessionStateContainingFormData.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RestoreSessionStateContainingFormData.cpp; sourceTree = "<group>"; };
@@ -284,6 +287,7 @@
F6F3F29013342FEB00A6BF19 /* CookieManager.cpp */,
BCB6803F126FBFE100642A61 /* DocumentStartUserScriptAlertCrash.cpp */,
BCB68041126FBFF100642A61 /* DocumentStartUserScriptAlertCrash_Bundle.cpp */,
+ C045F9441385C2E900C0F3CD /* DownloadDecideDestinationCrash.cpp */,
1A5FEFDC1270E2A3000E2921 /* EvaluateJavaScript.cpp */,
BCC8B95A12611F4700DE46A4 /* FailedLoad.cpp */,
1A02C84E125D4A8400E3F4BD /* Find.cpp */,
@@ -321,6 +325,7 @@
BC90977B125571AE00083756 /* Resources */ = {
isa = PBXGroup;
children = (
+ C045F9461385C2F800C0F3CD /* 18-characters.html */,
BC2D004A12A9FEB300E732A3 /* file-with-anchor.html */,
1A02C84B125D4A5E00E3F4BD /* find.html */,
BCBD372E125ABBE600D2C29F /* icon.png */,
@@ -461,6 +466,7 @@
BC246D9A132F1FE100B56D7C /* CanHandleRequest.cpp in Sources */,
F6F3F29113342FEB00A6BF19 /* CookieManager.cpp in Sources */,
33BE5AF5137B5A6C00705813 /* MouseMoveAfterCrash.cpp in Sources */,
+ C045F9451385C2EA00C0F3CD /* DownloadDecideDestinationCrash.cpp in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2/18-characters.html ( => )
Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2/DownloadDecideDestinationCrash.cpp
===================================================================
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2/DownloadDecideDestinationCrash.cpp (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2/DownloadDecideDestinationCrash.cpp 2011-05-20 13:24:29 UTC (rev 86945)
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2011 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.
+ */
+
+#include "Test.h"
+
+#include "PlatformUtilities.h"
+#include "PlatformWebView.h"
+#include <WebKit2/WKDownload.h>
+
+namespace TestWebKitAPI {
+
+static bool didDecideDestination;
+
+static void decidePolicyForNavigationAction(WKPageRef, WKFrameRef, WKFrameNavigationType, WKEventModifiers, WKEventMouseButton, WKURLRequestRef, WKFramePolicyListenerRef listener, WKTypeRef, const void*)
+{
+ WKFramePolicyListenerDownload(listener);
+}
+
+static WKStringRef decideDestinationWithSuggestedFilename(WKContextRef, WKDownloadRef download, WKStringRef, bool*, const void*)
+{
+ didDecideDestination = true;
+ WKDownloadCancel(download);
+ return Util::toWK("does not matter").leakRef();
+}
+
+static void setContextDownloadClient(WKContextRef context)
+{
+ WKContextDownloadClient client;
+ memset(&client, 0, sizeof(client));
+ client.decideDestinationWithSuggestedFilename = decideDestinationWithSuggestedFilename;
+
+ WKContextSetDownloadClient(context, &client);
+}
+
+static void setPagePolicyClient(WKPageRef page)
+{
+ WKPagePolicyClient policyClient;
+ memset(&policyClient, 0, sizeof(policyClient));
+ policyClient.decidePolicyForNavigationAction = decidePolicyForNavigationAction;
+
+ WKPageSetPagePolicyClient(page, &policyClient);
+}
+
+TEST(WebKit2, DownloadDecideDestinationCrash)
+{
+ WKRetainPtr<WKContextRef> context = adoptWK(WKContextCreate());
+ setContextDownloadClient(context.get());
+
+ PlatformWebView webView(context.get());
+ setPagePolicyClient(webView.page());
+
+ // The length of this filename was specially chosen to trigger the crash conditions in
+ // <http://webkit.org/b/61142>. Specifically, it causes ArgumentDecoder::m_bufferPos and m_bufferEnd
+ // to be equal after the DecideDestinationWithSuggestedFilename message has been handled.
+ WKPageLoadURL(webView.page(), adoptWK(Util::createURLForResource("18-characters", "html")).get());
+
+ Util::run(&didDecideDestination);
+}
+
+} // namespace TestWebKitAPI
Property changes: trunk/Tools/TestWebKitAPI/Tests/WebKit2/DownloadDecideDestinationCrash.cpp
Added: svn:eol-style
Modified: trunk/Tools/TestWebKitAPI/win/TestWebKitAPI.vcproj (86944 => 86945)
--- trunk/Tools/TestWebKitAPI/win/TestWebKitAPI.vcproj 2011-05-20 12:52:51 UTC (rev 86944)
+++ trunk/Tools/TestWebKitAPI/win/TestWebKitAPI.vcproj 2011-05-20 13:24:29 UTC (rev 86945)
@@ -412,6 +412,10 @@
Name="WebKit2"
>
<File
+ RelativePath="..\Tests\WebKit2\18-characters.html"
+ >
+ </File>
+ <File
RelativePath="..\Tests\WebKit2\AboutBlankLoad.cpp"
>
</File>
@@ -428,6 +432,10 @@
>
</File>
<File
+ RelativePath="..\Tests\WebKit2\DownloadDecideDestinationCrash.cpp"
+ >
+ </File>
+ <File
RelativePath="..\Tests\WebKit2\EvaluateJavaScript.cpp"
>
</File>
Modified: trunk/Tools/TestWebKitAPI/win/copy-resources.cmd (86944 => 86945)
--- trunk/Tools/TestWebKitAPI/win/copy-resources.cmd 2011-05-20 12:52:51 UTC (rev 86944)
+++ trunk/Tools/TestWebKitAPI/win/copy-resources.cmd 2011-05-20 13:24:29 UTC (rev 86945)
@@ -8,6 +8,7 @@
echo Copying resources...
mkdir 2>NUL "%ResourcesDirectory%"
for %%f in (
+ ..\Tests\WebKit2\18-characters.html
..\Tests\WebKit2\file-with-anchor.html
..\Tests\WebKit2\find.html
..\Tests\WebKit2\icon.png
