189585 2018-09-13 07:25:44 -0700 [GLib] Fix format string in KeyedEncoderGlib::beginObject(). 2018-09-13 12:08:56 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 Ms2ger Ms2ger bugs-noreply commit-queue mcatanzaro oldest_to_newest 1459347 0 Ms2ger 2018-09-13 07:25:44 -0700 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 1459348 1 349664 Ms2ger 2018-09-13 07:27:32 -0700 Created attachment 349664 Patch 1459358 2 349664 mcatanzaro 2018-09-13 08:15:46 -0700 Comment on attachment 349664 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. 1459373 3 Ms2ger 2018-09-13 08:56:34 -0700 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.) 1459435 4 349664 mcatanzaro 2018-09-13 11:03:54 -0700 Comment on attachment 349664 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. 1459437 5 mcatanzaro 2018-09-13 11:05:07 -0700 Of course, there's another problem here: this code should not be running at all, because resource load statistics should never be enabled. 1459438 6 mcatanzaro 2018-09-13 11:06:05 -0700 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. 1459466 7 349664 commit-queue 2018-09-13 12:08:54 -0700 Comment on attachment 349664 Patch Clearing flags on attachment: 349664 Committed r235982: <https://trac.webkit.org/changeset/235982> 1459467 8 commit-queue 2018-09-13 12:08:56 -0700 All reviewed patches have been landed. Closing bug. 349664 2018-09-13 07:27:32 -0700 2018-09-13 12:08:54 -0700 Patch bug-189585-20180913162730.patch text/plain 1783 Ms2ger U3VidmVyc2lvbiBSZXZpc2lvbjogMjM1OTcwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZjAyNGY2YjczZjM3MWFi MzNkYjc4NmM3MTExMWM2YTQ3YTc3NGY0MC4uMWEyMzhjZTY3NjMyMzEzOTlkZTc5MDhmN2M5MTYz MmM4OGY3NGE4ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDE4LTA5LTEzICBNczJn ZXIgIDxNczJnZXJAaWdhbGlhLmNvbT4KKworICAgICAgICBbR0xpYl0gRml4IGZvcm1hdCBzdHJp bmcgaW4gS2V5ZWRFbmNvZGVyR2xpYjo6YmVnaW5PYmplY3QoKS4KKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4OTU4NQorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoaXMgYXBwZWFycyB0byBmaXggdGhlIGZv bGxvd2luZyBhc3NlcnRpb24gbG9jYWxseToKKworICAgICAgICBHTGliLUNSSVRJQ0FMICoqOiBn X3ZhcmlhbnRfYnVpbGRlcl9hZGRfdmFsdWU6IGFzc2VydGlvbiAnIUdWU0IoYnVpbGRlciktPmV4 cGVjdGVkX3R5cGUgfHwgZ192YXJpYW50X2lzX29mX3R5cGUgKHZhbHVlLCBHVlNCKGJ1aWxkZXIp LT5leHBlY3RlZF90eXBlKScgZmFpbGVkCisKKyAgICAgICAgQ292ZXJlZCBieSBleGlzdGluZyB0 ZXN0cy4KKworICAgICAgICAqIHBsYXRmb3JtL2dsaWIvS2V5ZWRFbmNvZGVyR2xpYi5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpLZXllZEVuY29kZXJHbGliOjpiZWdpbk9iamVjdCk6CisKIDIwMTgt MDktMTEgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5vcmc+CiAKICAgICAgICAgaW1wb3J0 ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9zaGFkb3ctZG9tL2Zvcm0tY29udHJvbC1mb3JtLWF0 dHJpYnV0ZS5odG1sIGhpdHMgYXNzZXJ0aW9uCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9nbGliL0tleWVkRW5jb2RlckdsaWIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ2xpYi9LZXllZEVuY29kZXJHbGliLmNwcAppbmRleCA5ZDE4MmZhZTkxN2RjZjI0N2RkZmQ3 NjRlN2Q2Y2VmZGRkNWUzODVhLi43NDA0YmNiZDQ0OWFhNDkyMWI4Y2UyYjdlMDE4NDQ5NTAxMDZm NjgwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9nbGliL0tleWVkRW5jb2Rl ckdsaWIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dsaWIvS2V5ZWRFbmNvZGVy R2xpYi5jcHAKQEAgLTk4LDcgKzk4LDcgQEAgdm9pZCBLZXllZEVuY29kZXJHbGliOjplbmNvZGVT dHJpbmcoY29uc3QgU3RyaW5nJiBrZXksIGNvbnN0IFN0cmluZyYgdmFsdWUpCiAKIHZvaWQgS2V5 ZWRFbmNvZGVyR2xpYjo6YmVnaW5PYmplY3QoY29uc3QgU3RyaW5nJiBrZXkpCiB7Ci0gICAgR1Jl ZlB0cjxHVmFyaWFudEJ1aWxkZXI+IGJ1aWxkZXIgPSBhZG9wdEdSZWYoZ192YXJpYW50X2J1aWxk ZXJfbmV3KEdfVkFSSUFOVF9UWVBFKCJhYXtzdn0iKSkpOworICAgIEdSZWZQdHI8R1ZhcmlhbnRC dWlsZGVyPiBidWlsZGVyID0gYWRvcHRHUmVmKGdfdmFyaWFudF9idWlsZGVyX25ldyhHX1ZBUklB TlRfVFlQRSgiYXtzdn0iKSkpOwogICAgIG1fb2JqZWN0U3RhY2suYXBwZW5kKHN0ZDo6bWFrZV9w YWlyKGtleSwgYnVpbGRlcikpOwogICAgIG1fdmFyaWFudEJ1aWxkZXJTdGFjay5hcHBlbmQoYnVp bGRlci5nZXQoKSk7CiB9Cg==