Diff
Modified: trunk/Source/WebKit/ChangeLog (282782 => 282783)
--- trunk/Source/WebKit/ChangeLog 2021-09-20 21:16:42 UTC (rev 282782)
+++ trunk/Source/WebKit/ChangeLog 2021-09-20 21:36:44 UTC (rev 282783)
@@ -1,3 +1,25 @@
+2021-09-20 Alex Christensen <[email protected]>
+
+ WebKit might load custom URI scheme request content multiple times
+ https://bugs.webkit.org/show_bug.cgi?id=229116
+
+ Reviewed by Brady Eidson.
+
+ Use a ResourceLoaderIdentifier and a WebPageProxyIdentifier as keys in WebURLSchemeHandler.
+ This also makes it so that we don't need a new one per page on Cocoa platforms.
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _initializeWithConfiguration:]):
+ * UIProcess/WebURLSchemeHandler.cpp:
+ (WebKit::WebURLSchemeHandler::startTask):
+ (WebKit::WebURLSchemeHandler::processForTaskIdentifier const):
+ (WebKit::WebURLSchemeHandler::stopAllTasksForPage):
+ (WebKit::WebURLSchemeHandler::stopTask):
+ (WebKit::WebURLSchemeHandler::taskCompleted):
+ * UIProcess/WebURLSchemeHandler.h:
+ * UIProcess/WebURLSchemeTask.cpp:
+ (WebKit::WebURLSchemeTask::didComplete):
+
2021-09-20 Michael Catanzaro <[email protected]>
Use ObjectIdentifier for WebURLSchemeHandler
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (282782 => 282783)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2021-09-20 21:16:42 UTC (rev 282782)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2021-09-20 21:36:44 UTC (rev 282783)
@@ -434,7 +434,7 @@
_resourceLoadDelegate = makeUnique<WebKit::ResourceLoadDelegate>(self);
for (auto& pair : pageConfiguration->urlSchemeHandlers())
- _page->setURLSchemeHandlerForScheme(WebKit::WebURLSchemeHandlerCocoa::create(static_cast<WebKit::WebURLSchemeHandlerCocoa&>(pair.value.get()).apiHandler()), pair.key);
+ _page->setURLSchemeHandlerForScheme(pair.value.get(), pair.key);
_page->setCocoaView(self);
Modified: trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.cpp (282782 => 282783)
--- trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.cpp 2021-09-20 21:16:42 UTC (rev 282782)
+++ trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.cpp 2021-09-20 21:36:44 UTC (rev 282783)
@@ -46,7 +46,7 @@
void WebURLSchemeHandler::startTask(WebPageProxy& page, WebProcessProxy& process, PageIdentifier webPageID, URLSchemeTaskParameters&& parameters, SyncLoadCompletionHandler&& completionHandler)
{
auto taskIdentifier = parameters.taskIdentifier;
- auto result = m_tasks.add(taskIdentifier, WebURLSchemeTask::create(*this, page, process, webPageID, WTFMove(parameters), WTFMove(completionHandler)));
+ auto result = m_tasks.add({ taskIdentifier, page.identifier() }, WebURLSchemeTask::create(*this, page, process, webPageID, WTFMove(parameters), WTFMove(completionHandler)));
ASSERT(result.isNewEntry);
auto pageEntry = m_tasksByPageIdentifier.add(page.identifier(), HashSet<WebCore::ResourceLoaderIdentifier>());
@@ -56,11 +56,12 @@
platformStartTask(page, result.iterator->value);
}
-WebProcessProxy* WebURLSchemeHandler::processForTaskIdentifier(WebCore::ResourceLoaderIdentifier taskIdentifier) const
+WebProcessProxy* WebURLSchemeHandler::processForTaskIdentifier(WebPageProxy& page, WebCore::ResourceLoaderIdentifier taskIdentifier) const
{
- if (!decltype(m_tasks)::isValidKey(taskIdentifier))
+ auto key = std::make_pair(taskIdentifier, page.identifier());
+ if (!decltype(m_tasks)::isValidKey(key))
return nullptr;
- auto iterator = m_tasks.find(taskIdentifier);
+ auto iterator = m_tasks.find(key);
if (iterator == m_tasks.end())
return nullptr;
return iterator->value->process();
@@ -76,7 +77,7 @@
Vector<WebCore::ResourceLoaderIdentifier> taskIdentifiersToStop;
taskIdentifiersToStop.reserveInitialCapacity(tasksByPage.size());
for (auto taskIdentifier : tasksByPage) {
- if (!process || processForTaskIdentifier(taskIdentifier) == process)
+ if (!process || processForTaskIdentifier(page, taskIdentifier) == process)
taskIdentifiersToStop.uncheckedAppend(taskIdentifier);
}
@@ -87,9 +88,10 @@
void WebURLSchemeHandler::stopTask(WebPageProxy& page, WebCore::ResourceLoaderIdentifier taskIdentifier)
{
- if (!decltype(m_tasks)::isValidKey(taskIdentifier))
+ auto key = std::make_pair(taskIdentifier, page.identifier());
+ if (!decltype(m_tasks)::isValidKey(key))
return;
- auto iterator = m_tasks.find(taskIdentifier);
+ auto iterator = m_tasks.find(key);
if (iterator == m_tasks.end())
return;
@@ -100,9 +102,9 @@
m_tasks.remove(iterator);
}
-void WebURLSchemeHandler::taskCompleted(WebURLSchemeTask& task)
+void WebURLSchemeHandler::taskCompleted(WebPageProxyIdentifier pageID, WebURLSchemeTask& task)
{
- auto takenTask = m_tasks.take(task.identifier());
+ auto takenTask = m_tasks.take({ task.identifier(), pageID });
ASSERT_UNUSED(takenTask, takenTask == &task);
removeTaskFromPageMap(task.pageProxyID(), task.identifier());
Modified: trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.h (282782 => 282783)
--- trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.h 2021-09-20 21:16:42 UTC (rev 282782)
+++ trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.h 2021-09-20 21:36:44 UTC (rev 282783)
@@ -55,7 +55,7 @@
void startTask(WebPageProxy&, WebProcessProxy&, WebCore::PageIdentifier, URLSchemeTaskParameters&&, SyncLoadCompletionHandler&&);
void stopTask(WebPageProxy&, WebCore::ResourceLoaderIdentifier taskIdentifier);
void stopAllTasksForPage(WebPageProxy&, WebProcessProxy*);
- void taskCompleted(WebURLSchemeTask&);
+ void taskCompleted(WebPageProxyIdentifier, WebURLSchemeTask&);
protected:
WebURLSchemeHandler();
@@ -66,11 +66,11 @@
virtual void platformTaskCompleted(WebURLSchemeTask&) { };
void removeTaskFromPageMap(WebPageProxyIdentifier, WebCore::ResourceLoaderIdentifier);
- WebProcessProxy* processForTaskIdentifier(WebCore::ResourceLoaderIdentifier) const;
+ WebProcessProxy* processForTaskIdentifier(WebPageProxy&, WebCore::ResourceLoaderIdentifier) const;
WebURLSchemeHandlerIdentifier m_identifier;
- HashMap<WebCore::ResourceLoaderIdentifier, Ref<WebURLSchemeTask>> m_tasks;
+ HashMap<std::pair<WebCore::ResourceLoaderIdentifier, WebPageProxyIdentifier>, Ref<WebURLSchemeTask>> m_tasks;
HashMap<WebPageProxyIdentifier, HashSet<WebCore::ResourceLoaderIdentifier>> m_tasksByPageIdentifier;
SyncLoadCompletionHandler m_syncLoadCompletionHandler;
Modified: trunk/Source/WebKit/UIProcess/WebURLSchemeTask.cpp (282782 => 282783)
--- trunk/Source/WebKit/UIProcess/WebURLSchemeTask.cpp 2021-09-20 21:16:42 UTC (rev 282782)
+++ trunk/Source/WebKit/UIProcess/WebURLSchemeTask.cpp 2021-09-20 21:36:44 UTC (rev 282783)
@@ -226,7 +226,7 @@
}
m_process->send(Messages::WebPage::URLSchemeTaskDidComplete(m_urlSchemeHandler->identifier(), m_identifier, error), m_webPageID);
- m_urlSchemeHandler->taskCompleted(*this);
+ m_urlSchemeHandler->taskCompleted(pageProxyID(), *this);
return ExceptionType::None;
}
Modified: trunk/Tools/ChangeLog (282782 => 282783)
--- trunk/Tools/ChangeLog 2021-09-20 21:16:42 UTC (rev 282782)
+++ trunk/Tools/ChangeLog 2021-09-20 21:36:44 UTC (rev 282783)
@@ -1,3 +1,23 @@
+2021-09-20 Michael Catanzaro <[email protected]>
+
+ WebKit might load custom URI scheme request content multiple times
+ https://bugs.webkit.org/show_bug.cgi?id=229116
+
+ Reviewed by Brady Eidson.
+
+ Let's load the same URL 50 times and make sure all loads complete with the same content.
+ Without the fix, the test hangs forever spinning its main loop, because only a few of the
+ requests actually complete, so the test gets stuck waiting.
+
+ Note I picked 50 because when running the test 100 times, wpebackend-fdo dies to an
+ unrelated problem: its internal Wayland registry stops registering new global objects, and
+ each new web process starts crashing. That's weird, but not the problem we're trying to
+ solve today.
+
+ * TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp:
+ (createTestWebViewWithWebContext):
+ (testWebContextURIScheme):
+
2021-09-20 Jonathan Bedard <[email protected]>
[webkitcorepy] Move FileLock from webkitpy
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp (282782 => 282783)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp 2021-09-20 21:16:42 UTC (rev 282782)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp 2021-09-20 21:36:44 UTC (rev 282783)
@@ -113,7 +113,8 @@
test->m_uriSchemeRequest = request;
test->assertObjectIsDeletedWhenTestFinishes(G_OBJECT(request));
- g_assert_true(webkit_uri_scheme_request_get_web_view(request) == test->m_webView);
+ if (test->m_uriSchemeRequestCallbackUsesTestWebView)
+ g_assert_true(webkit_uri_scheme_request_get_web_view(request) == test->m_webView);
const char* scheme = webkit_uri_scheme_request_get_scheme(request);
g_assert_nonnull(scheme);
@@ -164,6 +165,8 @@
GRefPtr<WebKitURISchemeRequest> m_uriSchemeRequest;
HashMap<String, URISchemeHandler> m_handlersMap;
+ bool m_uriSchemeRequestCallbackUsesTestWebView { true };
+ int m_loadCounter { 0 };
};
String generateHTMLContent(unsigned contentLength)
@@ -193,6 +196,21 @@
return builder.toString();
}
+static GRefPtr<WebKitWebView> createTestWebViewWithWebContext(WebKitWebContext* context)
+{
+ WebKitWebView* view = WEBKIT_WEB_VIEW(g_object_new(WEBKIT_TYPE_WEB_VIEW,
+#if PLATFORM(WPE)
+ "backend", Test::createWebViewBackend(),
+#endif
+ "web-context", context,
+ nullptr));
+
+#if PLATFORM(GTK)
+ g_object_ref_sink(view);
+#endif
+ return adoptGRef(view);
+}
+
static void testWebContextURIScheme(URISchemeTest* test, gconstpointer)
{
test->registerURISchemeHandler("foo", kBarHTML, strlen(kBarHTML), "text/html");
@@ -278,6 +296,41 @@
g_assert_true(test->m_loadEvents.contains(LoadTrackingTest::ProvisionalLoadFailed));
g_assert_true(test->m_loadFailed);
g_assert_error(test->m_error.get(), G_IO_ERROR, G_IO_ERROR_CLOSED);
+
+ // Torture test time: make sure it still works if we issue a bunch of different requests all at
+ // once. Each request should finish and return exactly the same data.
+ int numIterations = 50;
+ GRefPtr<WebKitWebView> views[numIterations];
+ test->m_uriSchemeRequestCallbackUsesTestWebView = false;
+ for (int i = 0; i < numIterations; i++) {
+ views[i] = createTestWebViewWithWebContext(test->m_webContext.get());
+ test->assertObjectIsDeletedWhenTestFinishes(G_OBJECT(views[i].get()));
+ webkit_web_view_load_uri(views[i].get(), "foo:blank");
+ g_signal_connect(views[i].get(), "load-changed", G_CALLBACK(+[] (WebKitWebView* webView, WebKitLoadEvent loadEvent, gpointer userData) {
+ auto* test = static_cast<URISchemeTest*>(userData);
+ if (loadEvent != WEBKIT_LOAD_FINISHED)
+ return;
+ test->m_loadCounter--;
+ if (!test->m_loadCounter)
+ g_main_loop_quit(test->m_mainLoop);
+ }), test);
+ }
+ test->m_loadCounter = numIterations;
+ g_main_loop_run(test->m_mainLoop);
+
+ for (int i = 0; i < numIterations; i++) {
+ WebKitWebResource* resource = webkit_web_view_get_main_resource(views[i].get());
+ g_assert_nonnull(resource);
+ webkit_web_resource_get_data(resource, nullptr, +[] (GObject* object, GAsyncResult* result, gpointer userData) {
+ auto* test = static_cast<URISchemeTest*>(userData);
+ gsize dataSize;
+ unsigned char* data = "" result, &dataSize, nullptr);
+ g_assert_cmpint(strncmp(reinterpret_cast<char*>(data), kBarHTML, dataSize), ==, 0);
+ g_main_loop_quit(test->m_mainLoop);
+ }, test);
+ g_main_loop_run(test->m_mainLoop);
+ }
+ test->m_uriSchemeRequestCallbackUsesTestWebView = true;
}
#if PLATFORM(GTK)
