Bug 275087
Summary: | REGRESSION(274563@main): [GTK] Broke webkit_web_resource_get_data() on https://register.gitlab.gnome.org/ | ||
---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
Component: | WebKitGTK | Assignee: | Carlos Garcia Campos <cgarcia> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | bugs-noreply, cdumez, enometh, mcatanzaro, Nicole_rosario |
Priority: | P2 | ||
Version: | Other | ||
Hardware: | PC | ||
OS: | Linux | ||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=279258 |
Michael Catanzaro
Moving this from https://gitlab.gnome.org/GNOME/epiphany/-/issues/2366
274563@main "Generate Serialization for FragmentedSharedBuffer" broke the View Source function in Epiphany when used on https://register.gitlab.gnome.org/. The problem is webkit_web_resource_get_data_finish() returns only a NULL byte.
I found 275540@main "[GTK] Crash in WebPageProxy::getLoadDecisionForIcon" which was another regression also introduced by 274563@main. The problem there was fixed by accessing the unsafeBuffer of the IPC::SharedBufferReference rather than accessing its data. The problem with webkit_web_resource_get_data_finish() is the same and can be fixed in the same way (patch below).
I also found 221541@main "REGRESSION(r257667): [UNIX] Tests http/tests/incremental/split-hex-entities.pl and http/tests/misc/large-js-program.php are crashing" which looks relevant.
The following test patch fixes the regression with webkit_web_resource_get_data(), but I bet there are more similar problems elsewhere. Would be nice to find a way to fix this comprehensively instead of papering over it everywhere SharedBufferReferences are used. The difference is SharedBufferReference::span just fails if the data is non-contiguous whereas SharedBufferReference::unsafeBuffer makes it contiguous.
```
diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
index 5b84ec110b8b..5799daff1f14 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -5728,9 +5728,19 @@ static CompletionHandler<void(T data)> toAPIDataCallbackT(CompletionHandler<void
};
}
-auto* toAPIDataCallback = toAPIDataCallbackT<const std::optional<IPC::SharedBufferReference>&>;
+//auto* toAPIDataCallback = toAPIDataCallbackT<const std::optional<IPC::SharedBufferReference>&>;
auto* toAPIDataSharedBufferCallback = toAPIDataCallbackT<RefPtr<WebCore::SharedBuffer>&&>;
+static CompletionHandler<void(const std::optional<IPC::SharedBufferReference>&)> toAPIDataCallback(CompletionHandler<void(API::Data*)>&& callback)
+{
+ return [callback = WTFMove(callback)] (const std::optional<IPC::SharedBufferReference>& data) mutable {
+ if (auto buffer = data->unsafeBuffer())
+ callback(API::Data::create(buffer->span()).ptr());
+ else
+ callback(nullptr);
+ };
+}
+
#if ENABLE(MHTML)
void WebPageProxy::getContentsAsMHTMLData(CompletionHandler<void(API::Data*)>&& callback)
```
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Michael Catanzaro
Nicole, Chris, could you help with this please? My little debug patch fixes the problem with view source mode, but I don't think it's really an acceptable solution. I'd rather just revert 274563@main if we don't have any better solution.
Carlos Garcia Campos
Pull request: https://github.com/WebKit/WebKit/pull/33059
EWS
Committed 283254@main (ea6f3ad7b704): <https://commits.webkit.org/283254@main>
Reviewed commits have been landed. Closing PR #33059 and removing active labels.
Michael Catanzaro
*** Bug 279258 has been marked as a duplicate of this bug. ***