In r271879 I removed the support for data URLs in the network process for soup, assuming data URLs were always loaded from the web process, but that's not the case for synchronous loads. We don't need to go to the network process for sync loads.
Created attachment 418418 [details] Patch
It seems it's failing to find <WebCore/DataURLDecoder.h>. Aren't forwarding headers automatic in mac? I added the header to Source/WebCore/Headers.cmake
*** Bug 221023 has been marked as a duplicate of this bug. ***
Created attachment 418525 [details] Patch
Created attachment 418526 [details] Patch
Created attachment 418528 [details] Patch
Comment on attachment 418528 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418528&action=review > Source/WebCore/platform/network/DataURLDecoder.cpp:221 > + bool success = task->process(); I don't think we need a local variable here. I think the function logic would look simpler like this: if (!task->process()) return WTF::nullopt; if (task->isBase64) { if (!decodeBase64(*task, mode)) return WTF::nullopt; } else decodeEscaped(*task); return WTFMove(task->result); Also, this whole thing is duplicate code. I think this function should be called decodeSynchronously and decode should call it. > Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:547 > + result.response.setHTTPStatusCode(200); This is adding duplicate code with ResourceLoader::loadDataURL. Could we make them share code? > Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:601 > + auto syncLoadResult = loadDataURLSynchronously(request); WebLoaderStrategy::loadResourceSynchronously should return a SyncLoadResult instead of taking error, response, and data as out params.
Comment on attachment 418528 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418528&action=review >> Source/WebCore/platform/network/DataURLDecoder.cpp:221 >> + bool success = task->process(); > > I don't think we need a local variable here. I think the function logic would look simpler like this: > > if (!task->process()) > return WTF::nullopt; > if (task->isBase64) { > if (!decodeBase64(*task, mode)) > return WTF::nullopt; > } else > decodeEscaped(*task); > return WTFMove(task->result); > > Also, this whole thing is duplicate code. I think this function should be called decodeSynchronously and decode should call it. Ok, I'll moved the common parts, the whole function can't be used from the async function because the task creation is different. >> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:547 >> + result.response.setHTTPStatusCode(200); > > This is adding duplicate code with ResourceLoader::loadDataURL. Could we make them share code? Yes. >> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:601 >> + auto syncLoadResult = loadDataURLSynchronously(request); > > WebLoaderStrategy::loadResourceSynchronously should return a SyncLoadResult instead of taking error, response, and data as out params. SyncLoadResult is private. We could make it public and update all the callers, but I think that refactoring is out of scope of this bug.
Created attachment 418633 [details] Patch for landing
Committed r272009: <https://trac.webkit.org/changeset/272009>
<rdar://problem/73704517>