We are hitting the following assertion during layout tests: GLib-CRITICAL **: g_variant_builder_add_value: assertion '!GVSB(builder)->expected_type || g_variant_is_of_type (value, GVSB(builder)->expected_type)' failed with the stack g_variant_builder_add () at /home/slave/webkitgtk/gtk-linux-64-debug-tests/build/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gvariant.c:5531 (anonymous namespace)::KeyedEncoderGlib::encodeBool (this=0x7fdcd4001aa0, key="wasDataRead", value=false) at ../../Source/WebCore/platform/glib/KeyedEncoderGlib.cpp:61 (anonymous namespace)::<lambda(WebCore::KeyedEncoder&, const WebCore::CanvasActivityRecord&)>::operator()((anonymous namespace)::KeyedEncoder &, const (anonymous namespace)::CanvasActivityRecord &) const (__closure=0x7fdcf7ffe10f, encoderInner=..., canvasActivityRecord=...) at ../../Source/WebCore/loader/ResourceLoadStatistics.cpp:86 (anonymous namespace)::KeyedEncoder::encodeObject<WebCore::CanvasActivityRecord, WebCore::encodeCanvasActivityRecord(WebCore::KeyedEncoder&, const WTF::String&, const WebCore::CanvasActivityRecord&)::<lambda(WebCore::KeyedEncoder&, const WebCore::CanvasActivityRecord&)> >(const WTF::String &, const (anonymous namespace)::CanvasActivityRecord &, <unknown type in /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x28e67f>) (this=0x7fdcd4001aa0, key="canvasActivityRecord", object=..., function=<unknown type in /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x28e67f>) at ../../Source/WebCore/platform/KeyedCoding.h:175 (anonymous namespace)::encodeCanvasActivityRecord (encoder=..., label="canvasActivityRecord", canvasActivityRecord=...) at ../../Source/WebCore/loader/ResourceLoadStatistics.cpp:90
Created attachment 349664 [details] Patch
Comment on attachment 349664 [details] Patch I had to stare at this for a while to convince myself it should be different from KeyedEncoderGlib::beginArray, but I think the change is correct. But surely this changes the database serialization format. How do we fix existing databases that are already broken? Will the critical continue to occur forever for existing broken databases? That requires further investigation. Hopefully we have some mechanism somewhere to handle incompatible changes like this? I'd like Carlos to review this, please.
This method is not used in the IDB case, only for ResourceLoadStatistics, as I understand it. I don't think that serializes to disk? (This is why it's only come up since r235897.)
Comment on attachment 349664 [details] Patch OK then It *does* serialize to disk, but we have not yet enabled resource load statistics for GTK or WPE (we need to) so doesn't matter.
Of course, there's another problem here: this code should not be running at all, because resource load statistics should never be enabled.
BTW this was added for database process, years before resource load statistics existed, so I'm a bit surprised it's only used for resource load statistics nowadays.
Comment on attachment 349664 [details] Patch Clearing flags on attachment: 349664 Committed r235982: <https://trac.webkit.org/changeset/235982>
All reviewed patches have been landed. Closing bug.