Diff
Modified: trunk/Source/WebKit/ChangeLog (219663 => 219664)
--- trunk/Source/WebKit/ChangeLog 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Source/WebKit/ChangeLog 2017-07-19 20:42:21 UTC (rev 219664)
@@ -1,3 +1,33 @@
+2017-07-19 Brady Eidson <beid...@apple.com>
+
+ iBooks sometimes crashes when closing a book.
+ <rdar://problem/31180331> and https://bugs.webkit.org/show_bug.cgi?id=174658
+
+ Reviewed by Oliver Hunt.
+
+ - LegacyCustomProtocolManagerProxy should not reference a WebProcessPool directly.
+ - LegacyCustomProtocolManagerProxy should invalidate in its destructor.
+
+ * UIProcess/Network/CustomProtocols/LegacyCustomProtocolManagerProxy.cpp:
+ (WebKit::LegacyCustomProtocolManagerProxy::LegacyCustomProtocolManagerProxy):
+ (WebKit::LegacyCustomProtocolManagerProxy::~LegacyCustomProtocolManagerProxy):
+ (WebKit::LegacyCustomProtocolManagerProxy::startLoading):
+ (WebKit::LegacyCustomProtocolManagerProxy::stopLoading):
+ (WebKit::LegacyCustomProtocolManagerProxy::invalidate):
+ (WebKit::LegacyCustomProtocolManagerProxy::wasRedirectedToRequest):
+ (WebKit::LegacyCustomProtocolManagerProxy::didReceiveResponse):
+ (WebKit::LegacyCustomProtocolManagerProxy::didLoadData):
+ (WebKit::LegacyCustomProtocolManagerProxy::didFailWithError):
+ (WebKit::LegacyCustomProtocolManagerProxy::didFinishLoading):
+ (WebKit::LegacyCustomProtocolManagerProxy::processDidClose): Deleted.
+ * UIProcess/Network/CustomProtocols/LegacyCustomProtocolManagerProxy.h:
+
+ * UIProcess/Network/NetworkProcessProxy.cpp:
+ (WebKit::NetworkProcessProxy::NetworkProcessProxy):
+ (WebKit::NetworkProcessProxy::didClose):
+ * UIProcess/Network/NetworkProcessProxy.h:
+ (WebKit::NetworkProcessProxy::processPool):
+
2017-07-19 Yusuke Suzuki <utatane....@gmail.com>
[WTF] Implement WTF::ThreadGroup
Modified: trunk/Source/WebKit/UIProcess/Cocoa/LegacyCustomProtocolManagerClient.mm (219663 => 219664)
--- trunk/Source/WebKit/UIProcess/Cocoa/LegacyCustomProtocolManagerClient.mm 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Source/WebKit/UIProcess/Cocoa/LegacyCustomProtocolManagerClient.mm 2017-07-19 20:42:21 UTC (rev 219664)
@@ -148,8 +148,11 @@
void LegacyCustomProtocolManagerClient::invalidate(LegacyCustomProtocolManagerProxy&)
{
- for (auto& loader : m_loaderMap)
- [loader.value customProtocolManagerProxyDestroyed];
+ while (!m_loaderMap.isEmpty()) {
+ auto loader = m_loaderMap.take(m_loaderMap.begin()->key);
+ ASSERT(loader);
+ [loader customProtocolManagerProxyDestroyed];
+ }
}
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/Network/CustomProtocols/LegacyCustomProtocolManagerProxy.cpp (219663 => 219664)
--- trunk/Source/WebKit/UIProcess/Network/CustomProtocols/LegacyCustomProtocolManagerProxy.cpp 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Source/WebKit/UIProcess/Network/CustomProtocols/LegacyCustomProtocolManagerProxy.cpp 2017-07-19 20:42:21 UTC (rev 219664)
@@ -22,65 +22,64 @@
#include "LegacyCustomProtocolManagerProxy.h"
#include "APICustomProtocolManagerClient.h"
-#include "ChildProcessProxy.h"
#include "LegacyCustomProtocolManagerMessages.h"
#include "LegacyCustomProtocolManagerProxyMessages.h"
+#include "NetworkProcessProxy.h"
#include "WebProcessPool.h"
#include <WebCore/ResourceRequest.h>
namespace WebKit {
-LegacyCustomProtocolManagerProxy::LegacyCustomProtocolManagerProxy(ChildProcessProxy* childProcessProxy, WebProcessPool& processPool)
- : m_childProcessProxy(childProcessProxy)
- , m_processPool(processPool)
+LegacyCustomProtocolManagerProxy::LegacyCustomProtocolManagerProxy(NetworkProcessProxy& networkProcessProxy)
+ : m_networkProcessProxy(networkProcessProxy)
{
- ASSERT(m_childProcessProxy);
- m_childProcessProxy->addMessageReceiver(Messages::LegacyCustomProtocolManagerProxy::messageReceiverName(), *this);
+ m_networkProcessProxy.addMessageReceiver(Messages::LegacyCustomProtocolManagerProxy::messageReceiverName(), *this);
}
LegacyCustomProtocolManagerProxy::~LegacyCustomProtocolManagerProxy()
{
- m_childProcessProxy->removeMessageReceiver(Messages::LegacyCustomProtocolManagerProxy::messageReceiverName());
+ m_networkProcessProxy.removeMessageReceiver(Messages::LegacyCustomProtocolManagerProxy::messageReceiverName());
+ invalidate();
}
void LegacyCustomProtocolManagerProxy::startLoading(uint64_t customProtocolID, const WebCore::ResourceRequest& request)
{
- m_processPool.customProtocolManagerClient().startLoading(*this, customProtocolID, request);
+ m_networkProcessProxy.processPool().customProtocolManagerClient().startLoading(*this, customProtocolID, request);
}
void LegacyCustomProtocolManagerProxy::stopLoading(uint64_t customProtocolID)
{
- m_processPool.customProtocolManagerClient().stopLoading(*this, customProtocolID);
+ m_networkProcessProxy.processPool().customProtocolManagerClient().stopLoading(*this, customProtocolID);
}
-void LegacyCustomProtocolManagerProxy::processDidClose()
+void LegacyCustomProtocolManagerProxy::invalidate()
{
- m_processPool.customProtocolManagerClient().invalidate(*this);
+ m_networkProcessProxy.processPool().customProtocolManagerClient().invalidate(*this);
}
void LegacyCustomProtocolManagerProxy::wasRedirectedToRequest(uint64_t customProtocolID, const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& redirectResponse)
{
- m_childProcessProxy->send(Messages::LegacyCustomProtocolManager::WasRedirectedToRequest(customProtocolID, request, redirectResponse), 0);
+ m_networkProcessProxy.send(Messages::LegacyCustomProtocolManager::WasRedirectedToRequest(customProtocolID, request, redirectResponse), 0);
}
void LegacyCustomProtocolManagerProxy::didReceiveResponse(uint64_t customProtocolID, const WebCore::ResourceResponse& response, uint32_t cacheStoragePolicy)
{
- m_childProcessProxy->send(Messages::LegacyCustomProtocolManager::DidReceiveResponse(customProtocolID, response, cacheStoragePolicy), 0);
+ m_networkProcessProxy.send(Messages::LegacyCustomProtocolManager::DidReceiveResponse(customProtocolID, response, cacheStoragePolicy), 0);
}
void LegacyCustomProtocolManagerProxy::didLoadData(uint64_t customProtocolID, const IPC::DataReference& data)
{
- m_childProcessProxy->send(Messages::LegacyCustomProtocolManager::DidLoadData(customProtocolID, data), 0);
+ m_networkProcessProxy.send(Messages::LegacyCustomProtocolManager::DidLoadData(customProtocolID, data), 0);
}
void LegacyCustomProtocolManagerProxy::didFailWithError(uint64_t customProtocolID, const WebCore::ResourceError& error)
{
- m_childProcessProxy->send(Messages::LegacyCustomProtocolManager::DidFailWithError(customProtocolID, error), 0);
+ m_networkProcessProxy.send(Messages::LegacyCustomProtocolManager::DidFailWithError(customProtocolID, error), 0);
}
void LegacyCustomProtocolManagerProxy::didFinishLoading(uint64_t customProtocolID)
{
- m_childProcessProxy->send(Messages::LegacyCustomProtocolManager::DidFinishLoading(customProtocolID), 0);
+ m_networkProcessProxy.send(Messages::LegacyCustomProtocolManager::DidFinishLoading(customProtocolID), 0);
}
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/Network/CustomProtocols/LegacyCustomProtocolManagerProxy.h (219663 => 219664)
--- trunk/Source/WebKit/UIProcess/Network/CustomProtocols/LegacyCustomProtocolManagerProxy.h 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Source/WebKit/UIProcess/Network/CustomProtocols/LegacyCustomProtocolManagerProxy.h 2017-07-19 20:42:21 UTC (rev 219664)
@@ -45,18 +45,17 @@
namespace WebKit {
-class ChildProcessProxy;
-class WebProcessPool;
+class NetworkProcessProxy;
class LegacyCustomProtocolManagerProxy : public IPC::MessageReceiver {
public:
- LegacyCustomProtocolManagerProxy(ChildProcessProxy*, WebProcessPool&);
+ LegacyCustomProtocolManagerProxy(NetworkProcessProxy&);
~LegacyCustomProtocolManagerProxy();
void startLoading(uint64_t customProtocolID, const WebCore::ResourceRequest&);
void stopLoading(uint64_t customProtocolID);
- void processDidClose();
+ void invalidate();
void wasRedirectedToRequest(uint64_t customProtocolID, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&);
void didReceiveResponse(uint64_t customProtocolID, const WebCore::ResourceResponse&, uint32_t cacheStoragePolicy);
@@ -68,8 +67,7 @@
// IPC::MessageReceiver
void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
- ChildProcessProxy* m_childProcessProxy;
- WebProcessPool& m_processPool;
+ NetworkProcessProxy& m_networkProcessProxy;
#if PLATFORM(COCOA)
typedef HashMap<uint64_t, RetainPtr<WKCustomProtocolLoader>> LoaderMap;
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (219663 => 219664)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2017-07-19 20:42:21 UTC (rev 219664)
@@ -69,7 +69,7 @@
: ChildProcessProxy(processPool.alwaysRunsAtBackgroundPriority())
, m_processPool(processPool)
, m_numPendingConnectionRequests(0)
- , m_customProtocolManagerProxy(this, processPool)
+ , m_customProtocolManagerProxy(*this)
, m_throttler(*this, processPool.shouldTakeUIBackgroundAssertion())
{
connect();
@@ -234,7 +234,7 @@
{
if (m_downloadProxyMap)
m_downloadProxyMap->processDidClose();
- m_customProtocolManagerProxy.processDidClose();
+ m_customProtocolManagerProxy.invalidate();
m_tokenForHoldingLockedFiles = nullptr;
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (219663 => 219664)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2017-07-19 20:42:21 UTC (rev 219664)
@@ -77,6 +77,7 @@
void setIsHoldingLockedFiles(bool);
ProcessThrottler& throttler() { return m_throttler; }
+ WebProcessPool& processPool() { return m_processPool; }
private:
NetworkProcessProxy(WebProcessPool&);
Modified: trunk/Tools/ChangeLog (219663 => 219664)
--- trunk/Tools/ChangeLog 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Tools/ChangeLog 2017-07-19 20:42:21 UTC (rev 219664)
@@ -1,3 +1,21 @@
+2017-07-19 Brady Eidson <beid...@apple.com>
+
+ iBooks sometimes crashes when closing a book.
+ <rdar://problem/31180331> and https://bugs.webkit.org/show_bug.cgi?id=174658
+
+ Reviewed by Oliver Hunt.
+
+ * TestWebKitAPI/Tests/WebKit2ObjC/CustomProtocolsTest.mm:
+ (-[ProcessPoolDestroyedDuringLoadingProtocol startLoading]):
+ (-[ProcessPoolDestroyedDuringLoadingProtocol finishTheLoad]):
+ (-[ProcessPoolDestroyedDuringLoadingProtocol stopLoading]):
+ (TestWebKitAPI::TEST):
+
+ Add a "spin the runloop X number of times" utility:
+ * TestWebKitAPI/Utilities.h:
+ * TestWebKitAPI/cocoa/UtilitiesCocoa.mm:
+ (TestWebKitAPI::Util::spinRunLoop):
+
2017-07-19 Jonathan Bedard <jbed...@apple.com>
lint-test-expectations should be run during style checking
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2ObjC/CustomProtocolsTest.mm (219663 => 219664)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2ObjC/CustomProtocolsTest.mm 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2ObjC/CustomProtocolsTest.mm 2017-07-19 20:42:21 UTC (rev 219664)
@@ -105,6 +105,40 @@
@end
+@interface ProcessPoolDestroyedDuringLoadingProtocol : TestProtocol
+-(void)finishTheLoad;
+@end
+
+static bool isDone;
+static RetainPtr<ProcessPoolDestroyedDuringLoadingProtocol> processPoolProtocolInstance;
+
+@implementation ProcessPoolDestroyedDuringLoadingProtocol
+
+- (void)startLoading
+{
+ NSURL *requestURL = self.request.URL;
+ NSData *data = "" dataUsingEncoding:NSASCIIStringEncoding];
+ RetainPtr<NSURLResponse> response = adoptNS([[NSURLResponse alloc] initWithURL:requestURL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]);
+
+ [self.client URLProtocol:self didReceiveResponse:response.get() cacheStoragePolicy:NSURLCacheStorageNotAllowed];
+ [self.client URLProtocol:self didLoadData:data];
+
+ processPoolProtocolInstance = self;
+ isDone = true;
+}
+
+- (void)finishTheLoad
+{
+ [self.client URLProtocolDidFinishLoading:self];
+}
+
+- (void)stopLoading
+{
+ isDone = true;
+}
+
+@end
+
namespace TestWebKitAPI {
static void runTest()
@@ -132,6 +166,38 @@
[CloseWhileStartingProtocol unregister];
}
+TEST(WebKit2CustomProtocolsTest, ProcessPoolDestroyedDuringLoading)
+{
+ [ProcessPoolDestroyedDuringLoadingProtocol registerWithScheme:@"custom"];
+
+ auto autoreleasePool = adoptNS([[NSAutoreleasePool alloc] init]);
+ auto browsingContextGroup = adoptNS([[WKBrowsingContextGroup alloc] initWithIdentifier:@"TestIdentifier"]);
+ auto processGroup = adoptNS([[WKProcessGroup alloc] init]);
+ auto wkView = adoptNS([[WKView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) processGroup:processGroup.get() browsingContextGroup:browsingContextGroup.get()]);
+
+ [[wkView browsingContextController] loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"custom:///test"]]];
+
+ Util::run(&isDone);
+ isDone = false;
+
+ processGroup = nil;
+ wkView = nil;
+ browsingContextGroup = nil;
+ autoreleasePool = nil;
+
+ ASSERT(processPoolProtocolInstance);
+ [processPoolProtocolInstance finishTheLoad];
+
+ // isDone might already be true if the protocol has already been told to stopLoading.
+ if (!isDone)
+ Util::run(&isDone);
+
+ // To crash reliably we need to spin the runloop a few times after the custom protocol has completed.
+ Util::spinRunLoop(10);
+
+ [ProcessPoolDestroyedDuringLoadingProtocol unregister];
+}
+
} // namespace TestWebKitAPI
#endif // WK_API_ENABLED
Modified: trunk/Tools/TestWebKitAPI/Utilities.h (219663 => 219664)
--- trunk/Tools/TestWebKitAPI/Utilities.h 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Tools/TestWebKitAPI/Utilities.h 2017-07-19 20:42:21 UTC (rev 219664)
@@ -28,8 +28,13 @@
namespace TestWebKitAPI {
namespace Util {
-// Runs a platform runloop until the 'done' is true.
+// Runs a platform runloop until the 'done' flag is true.
void run(bool* done);
+
+// Runs a platform runloop `count` number of spins.
+void spinRunLoop(uint64_t count);
+
+// Runs a platform runloop until the amount of seconds has passed.
void sleep(double seconds);
} // namespace Util
Modified: trunk/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm (219663 => 219664)
--- trunk/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm 2017-07-19 20:24:15 UTC (rev 219663)
+++ trunk/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm 2017-07-19 20:42:21 UTC (rev 219664)
@@ -35,6 +35,12 @@
[[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantPast]];
}
+void spinRunLoop(uint64_t count)
+{
+ for (uint64_t i = 0; i < count; ++i)
+ [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantPast]];
+}
+
void sleep(double seconds)
{
[[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:seconds]];
