Diff
Modified: trunk/Source/WebKit/ChangeLog (220010 => 220011)
--- trunk/Source/WebKit/ChangeLog 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/ChangeLog 2017-07-28 20:03:42 UTC (rev 220011)
@@ -1,3 +1,60 @@
+2017-07-28 Brady Eidson <beid...@apple.com>
+
+ WKURLSchemeTaskImpl instances are being abandoned (except if the task is stopped).
+ <rdar://problem/33568276> and https://bugs.webkit.org/show_bug.cgi?id=174895
+
+ Reviewed by Alex Christensen.
+
+ There was a lot going on here:
+ - There was an unbroken cycle between URLSchemeHandlers and URLSchemeTasks
+ - WKURLSchemeTasks were not destroying their API::URLSchemeTask implementation object
+ - When a WKWebView was dealloc'ed, it was not cleaning up the related UIProcess tasks
+ - If the NetworkingProcess crashed, WebProcesses were not cleaning up their tasks
+ (and therefore the UIProcess tasks were also not getting cleaned up)
+ - If a WebProcess crashed, the UIProcess was not cleaning up the related tasks
+
+ * UIProcess/API/Cocoa/WKURLSchemeTask.mm:
+ (-[WKURLSchemeTaskImpl dealloc]): Call API::~URLSchemeTask
+
+ * UIProcess/Cocoa/WebURLSchemeHandlerCocoa.h:
+ * UIProcess/Cocoa/WebURLSchemeHandlerCocoa.mm:
+ (WebKit::WebURLSchemeHandlerCocoa::platformTaskCompleted): Clear out the API wrapper map
+ so the wrapper and implementation object can both be destroyed.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::close): Call stopAllURLSchemeTasks.
+ (WebKit::WebPageProxy::processDidTerminate): Call stopAllURLSchemeTasks.
+ (WebKit::WebPageProxy::stopAllURLSchemeTasks):
+ * UIProcess/WebPageProxy.h:
+
+ * UIProcess/WebURLSchemeHandler.cpp:
+ (WebKit::WebURLSchemeHandler::startTask):
+ (WebKit::WebURLSchemeHandler::stopAllTasksForPage):
+ (WebKit::WebURLSchemeHandler::stopTask):
+ (WebKit::WebURLSchemeHandler::taskCompleted):
+ * UIProcess/WebURLSchemeHandler.h:
+
+ * UIProcess/WebURLSchemeTask.cpp:
+ (WebKit::WebURLSchemeTask::WebURLSchemeTask):
+ * UIProcess/WebURLSchemeTask.h:
+ (WebKit::WebURLSchemeTask::pageID): Store separate from the WebPage so messaging
+ is possible even after the WebPage* has been cleared.
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::stopAllURLSchemeTasks):
+ * WebProcess/WebPage/WebPage.h:
+
+ * WebProcess/WebPage/WebURLSchemeHandlerProxy.cpp:
+ (WebKit::WebURLSchemeHandlerProxy::stopAllTasks):
+ (WebKit::WebURLSchemeHandlerProxy::taskDidComplete):
+ (WebKit::WebURLSchemeHandlerProxy::taskDidStopLoading):
+ (WebKit::WebURLSchemeHandlerProxy::removeTask):
+ * WebProcess/WebPage/WebURLSchemeHandlerProxy.h:
+
+ * WebProcess/WebProcess.cpp:
+ (WebKit::WebProcess::networkProcessConnectionClosed): Call stopAllURLSchemeTasks
+ for each WebPage.
+
2017-07-28 Frederic Wang <fw...@igalia.cpm>
Fix typo in scrollPositionChangedViaDelegatedScrolling
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKURLSchemeTask.mm (220010 => 220011)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKURLSchemeTask.mm 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKURLSchemeTask.mm 2017-07-28 20:03:42 UTC (rev 220011)
@@ -28,6 +28,7 @@
#if WK_API_ENABLED
+#import "WebURLSchemeHandler.h"
#import "WebURLSchemeTask.h"
#import <WebCore/ResourceError.h>
#import <WebCore/ResourceResponse.h>
@@ -61,6 +62,13 @@
@implementation WKURLSchemeTaskImpl
+- (void)dealloc
+{
+ _urlSchemeTask->API::URLSchemeTask::~URLSchemeTask();
+
+ [super dealloc];
+}
+
- (NSURLRequest *)request
{
return _urlSchemeTask->task().request().nsURLRequest(DoNotUpdateHTTPBody);
Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.h (220010 => 220011)
--- trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.h 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.h 2017-07-28 20:03:42 UTC (rev 220011)
@@ -47,6 +47,7 @@
void platformStartTask(WebPageProxy&, WebURLSchemeTask&) final;
void platformStopTask(WebPageProxy&, WebURLSchemeTask&) final;
+ void platformTaskCompleted(WebURLSchemeTask&) final;
RetainPtr<id <WKURLSchemeHandler>> m_apiHandler;
HashMap<uint64_t, Ref<API::URLSchemeTask>> m_apiTasks;
Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.mm (220010 => 220011)
--- trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.mm 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.mm 2017-07-28 20:03:42 UTC (rev 220011)
@@ -76,4 +76,13 @@
#endif
}
+void WebURLSchemeHandlerCocoa::platformTaskCompleted(WebURLSchemeTask& task)
+{
+#if WK_API_ENABLED
+ m_apiTasks.remove(task.identifier());
+#else
+ UNUSED_PARAM(task);
+#endif
+}
+
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (220010 => 220011)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2017-07-28 20:03:42 UTC (rev 220011)
@@ -781,6 +781,8 @@
// Make sure we don't hold a process assertion after getting closed.
m_activityToken = nullptr;
#endif
+
+ stopAllURLSchemeTasks();
}
bool WebPageProxy::tryClose()
@@ -5355,8 +5357,20 @@
if (auto* automationSession = process().processPool().automationSession())
automationSession->terminate();
}
+
+ stopAllURLSchemeTasks();
}
+void WebPageProxy::stopAllURLSchemeTasks()
+{
+ HashSet<WebURLSchemeHandler*> handlers;
+ for (auto& handler : m_urlSchemeHandlersByScheme.values())
+ handlers.add(handler.ptr());
+
+ for (auto* handler : handlers)
+ handler->stopAllTasksForPage(*this);
+}
+
#if PLATFORM(IOS)
void WebPageProxy::processWillBecomeSuspended()
{
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (220010 => 220011)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2017-07-28 20:03:42 UTC (rev 220011)
@@ -1623,6 +1623,8 @@
void viewIsBecomingVisible();
+ void stopAllURLSchemeTasks();
+
PageClient& m_pageClient;
Ref<API::PageConfiguration> m_configuration;
Modified: trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.cpp (220010 => 220011)
--- trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.cpp 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.cpp 2017-07-28 20:03:42 UTC (rev 220011)
@@ -26,6 +26,7 @@
#include "config.h"
#include "WebURLSchemeHandler.h"
+#include "WebPageProxy.h"
#include "WebURLSchemeTask.h"
using namespace WebCore;
@@ -53,9 +54,26 @@
auto result = m_tasks.add(taskIdentifier, WebURLSchemeTask::create(*this, page, taskIdentifier, request));
ASSERT(result.isNewEntry);
+ auto pageEntry = m_tasksByPageIdentifier.add(page.pageID(), HashSet<uint64_t>());
+ ASSERT(!pageEntry.iterator->value.contains(taskIdentifier));
+ pageEntry.iterator->value.add(taskIdentifier);
+
platformStartTask(page, result.iterator->value);
}
+void WebURLSchemeHandler::stopAllTasksForPage(WebPageProxy& page)
+{
+ auto iterator = m_tasksByPageIdentifier.find(page.pageID());
+ if (iterator == m_tasksByPageIdentifier.end())
+ return;
+
+ auto& tasksByPage = iterator->value;
+ while (!tasksByPage.isEmpty())
+ stopTask(page, *tasksByPage.begin());
+
+ ASSERT(m_tasksByPageIdentifier.find(page.pageID()) == m_tasksByPageIdentifier.end());
+}
+
void WebURLSchemeHandler::stopTask(WebPageProxy& page, uint64_t taskIdentifier)
{
auto iterator = m_tasks.find(taskIdentifier);
@@ -62,11 +80,28 @@
if (iterator == m_tasks.end())
return;
+ auto pageIterator = m_tasksByPageIdentifier.find(page.pageID());
+ ASSERT(pageIterator != m_tasksByPageIdentifier.end());
+ ASSERT(pageIterator->value.contains(taskIdentifier));
+ pageIterator->value.remove(taskIdentifier);
+
iterator->value->stop();
-
platformStopTask(page, iterator->value);
m_tasks.remove(iterator);
+ if (pageIterator->value.isEmpty())
+ m_tasksByPageIdentifier.remove(pageIterator);
}
+void WebURLSchemeHandler::taskCompleted(WebURLSchemeTask& task)
+{
+ auto takenTask = m_tasks.take(task.identifier());
+ ASSERT_UNUSED(takenTask, takenTask->ptr() == &task);
+
+ ASSERT(m_tasksByPageIdentifier.get(task.pageID()).contains(task.identifier()));
+ m_tasksByPageIdentifier.get(task.pageID()).remove(task.identifier());
+
+ platformTaskCompleted(task);
+}
+
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.h (220010 => 220011)
--- trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.h 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.h 2017-07-28 20:03:42 UTC (rev 220011)
@@ -27,6 +27,7 @@
#include "WebURLSchemeTask.h"
#include <wtf/HashMap.h>
+#include <wtf/HashSet.h>
#include <wtf/RefCounted.h>
#include <wtf/RefPtr.h>
@@ -47,6 +48,8 @@
void startTask(WebPageProxy&, uint64_t taskIdentifier, const WebCore::ResourceRequest&);
void stopTask(WebPageProxy&, uint64_t taskIdentifier);
+ void stopAllTasksForPage(WebPageProxy&);
+ void taskCompleted(WebURLSchemeTask&);
protected:
WebURLSchemeHandler();
@@ -54,10 +57,12 @@
private:
virtual void platformStartTask(WebPageProxy&, WebURLSchemeTask&) = 0;
virtual void platformStopTask(WebPageProxy&, WebURLSchemeTask&) = 0;
+ virtual void platformTaskCompleted(WebURLSchemeTask&) = 0;
uint64_t m_identifier;
HashMap<uint64_t, Ref<WebURLSchemeTask>> m_tasks;
+ HashMap<uint64_t, HashSet<uint64_t>> m_tasksByPageIdentifier;
}; // class WebURLSchemeHandler
Modified: trunk/Source/WebKit/UIProcess/WebURLSchemeTask.cpp (220010 => 220011)
--- trunk/Source/WebKit/UIProcess/WebURLSchemeTask.cpp 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/UIProcess/WebURLSchemeTask.cpp 2017-07-28 20:03:42 UTC (rev 220011)
@@ -44,6 +44,7 @@
: m_urlSchemeHandler(handler)
, m_page(&page)
, m_identifier(resourceIdentifier)
+ , m_pageIdentifier(page.pageID())
, m_request(request)
{
}
@@ -115,6 +116,8 @@
m_completed = true;
m_page->send(Messages::WebPage::URLSchemeTaskDidComplete(m_urlSchemeHandler->identifier(), m_identifier, error));
+ m_urlSchemeHandler->taskCompleted(*this);
+
return ExceptionType::None;
}
Modified: trunk/Source/WebKit/UIProcess/WebURLSchemeTask.h (220010 => 220011)
--- trunk/Source/WebKit/UIProcess/WebURLSchemeTask.h 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/UIProcess/WebURLSchemeTask.h 2017-07-28 20:03:42 UTC (rev 220011)
@@ -46,6 +46,7 @@
static Ref<WebURLSchemeTask> create(WebURLSchemeHandler&, WebPageProxy&, uint64_t identifier, const WebCore::ResourceRequest&);
uint64_t identifier() const { return m_identifier; }
+ uint64_t pageID() const { return m_pageIdentifier; }
const WebCore::ResourceRequest& request() const { return m_request; }
@@ -71,6 +72,7 @@
Ref<WebURLSchemeHandler> m_urlSchemeHandler;
WebPageProxy* m_page;
uint64_t m_identifier;
+ uint64_t m_pageIdentifier;
WebCore::ResourceRequest m_request;
bool m_stopped { false };
bool m_responseSent { false };
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (220010 => 220011)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2017-07-28 20:03:42 UTC (rev 220011)
@@ -5908,6 +5908,16 @@
return m_schemeToURLSchemeHandlerProxyMap.get(scheme);
}
+void WebPage::stopAllURLSchemeTasks()
+{
+ HashSet<WebURLSchemeHandlerProxy*> handlers;
+ for (auto& handler : m_schemeToURLSchemeHandlerProxyMap.values())
+ handlers.add(handler.get());
+
+ for (auto* handler : handlers)
+ handler->stopAllTasks();
+}
+
void WebPage::registerURLSchemeHandler(uint64_t handlerIdentifier, const String& scheme)
{
auto schemeResult = m_schemeToURLSchemeHandlerProxyMap.add(scheme, WebURLSchemeHandlerProxy::create(*this, handlerIdentifier));
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (220010 => 220011)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2017-07-28 20:03:42 UTC (rev 220011)
@@ -979,6 +979,8 @@
#endif
WebURLSchemeHandlerProxy* urlSchemeHandlerForScheme(const String&);
+ void stopAllURLSchemeTasks();
+
std::optional<double> cpuLimit() const { return m_cpuLimit; }
static PluginView* pluginViewForFrame(WebCore::Frame*);
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeHandlerProxy.cpp (220010 => 220011)
--- trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeHandlerProxy.cpp 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeHandlerProxy.cpp 2017-07-28 20:03:42 UTC (rev 220011)
@@ -55,6 +55,12 @@
result.iterator->value->startLoading();
}
+void WebURLSchemeHandlerProxy::stopAllTasks()
+{
+ while (!m_tasks.isEmpty())
+ m_tasks.begin()->value->stopLoading();
+}
+
void WebURLSchemeHandlerProxy::taskDidPerformRedirection(uint64_t taskIdentifier, WebCore::ResourceResponse&& redirectResponse, WebCore::ResourceRequest&& newRequest)
{
auto* task = m_tasks.get(taskIdentifier);
@@ -84,18 +90,25 @@
void WebURLSchemeHandlerProxy::taskDidComplete(uint64_t taskIdentifier, const ResourceError& error)
{
- auto task = m_tasks.take(taskIdentifier);
- if (!task)
- return;
-
- WebProcess::singleton().webLoaderStrategy().removeURLSchemeTaskProxy(*task);
- task->didComplete(error);
+ if (auto task = removeTask(taskIdentifier))
+ task->didComplete(error);
}
void WebURLSchemeHandlerProxy::taskDidStopLoading(WebURLSchemeTaskProxy& task)
{
ASSERT(m_tasks.get(task.identifier()) == &task);
- m_tasks.remove(task.identifier());
+ removeTask(task.identifier());
}
+RefPtr<WebURLSchemeTaskProxy> WebURLSchemeHandlerProxy::removeTask(uint64_t identifier)
+{
+ auto task = m_tasks.take(identifier);
+ if (!task)
+ return nullptr;
+
+ WebProcess::singleton().webLoaderStrategy().removeURLSchemeTaskProxy(*task);
+
+ return task;
+}
+
} // namespace WebKit
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeHandlerProxy.h (220010 => 220011)
--- trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeHandlerProxy.h 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeHandlerProxy.h 2017-07-28 20:03:42 UTC (rev 220011)
@@ -49,6 +49,7 @@
~WebURLSchemeHandlerProxy();
void startNewTask(WebCore::ResourceLoader&);
+ void stopAllTasks();
uint64_t identifier() const { return m_identifier; }
WebPage& page() { return m_webPage; }
@@ -61,6 +62,9 @@
private:
WebURLSchemeHandlerProxy(WebPage&, uint64_t identifier);
+
+ RefPtr<WebURLSchemeTaskProxy> removeTask(uint64_t identifier);
+
WebPage& m_webPage;
uint64_t m_identifier { 0 };
Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (220010 => 220011)
--- trunk/Source/WebKit/WebProcess/WebProcess.cpp 2017-07-28 18:54:33 UTC (rev 220010)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp 2017-07-28 20:03:42 UTC (rev 220011)
@@ -1129,6 +1129,9 @@
m_webLoaderStrategy.networkProcessCrashed();
WebSocketStream::networkProcessCrashed();
+
+ for (auto& page : m_pageMap.values())
+ page->stopAllURLSchemeTasks();
}
WebLoaderStrategy& WebProcess::webLoaderStrategy()
