98507 2012-10-05 04:03:42 -0700 [GTK] Test /webkit2/WebKitWebView/populate-menu asserts in debug bots 2012-10-08 02:28:34 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC Linux RESOLVED FIXED Gtk P2 Normal --- 1 cgarcia webkit-unassigned gustavo mario mrobinson webkit.review.bot oldest_to_newest 735557 0 cgarcia 2012-10-05 04:03:42 -0700 /webkit2/WebKitWebView/populate-menu: ASSERTION FAILED: iconRecord || m_retainedPageURLs.contains(pageURLOriginal) ../../Source/WebCore/loader/icon/IconDatabase.cpp(266) : virtual WebCore::Image* WebCore::IconDatabase::synchronousIconForPageURL(const WTF::String&, const WebCore::IntSize&) 735561 1 167305 cgarcia 2012-10-05 04:09:49 -0700 Created attachment 167305 Patch 735562 2 webkit.review.bot 2012-10-05 04:11:49 -0700 Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API 735645 3 167305 mrobinson 2012-10-05 08:18:22 -0700 Comment on attachment 167305 Patch Okay. But before you land, do you mind having Mario look over this too? 736577 4 167305 mario 2012-10-08 01:55:23 -0700 Comment on attachment 167305 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167305&action=review The patch looks good to me too. A pity I didn't realize of this issue when writing the original patch, quite happy Carlos figured it out and fixed it, though. > Source/WebKit2/UIProcess/API/gtk/WebKitFaviconDatabase.cpp:151 > PendingIconRequestVector* icons = database->priv->pendingIconRequests.get(pageURL); Completely off-topic thing: you might consider renaming this 'icons' variable before landing to something more adequate, such as 'pendingIconRequests', or something like that. > Source/WebKit2/UIProcess/API/gtk/WebKitFaviconDatabase.cpp:156 > + RefPtr<cairo_surface_t> icon = getIconSurfaceSynchronously(database, pageURL, &error.outPtr()); I like this refactoring: taking this call out of the loop. Good catch. 736592 5 cgarcia 2012-10-08 02:28:34 -0700 Committed r130625: <http://trac.webkit.org/changeset/130625> 167305 2012-10-05 04:09:49 -0700 2012-10-08 01:55:23 -0700 Patch wk2-favicons-assertion.diff text/plain 4566 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBjYmZhNjRhLi4xZmY5NDQ0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjIg QEAKKzIwMTItMTAtMDUgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgorCisgICAgICAgIFtHVEtdIFRlc3QgL3dlYmtpdDIvV2ViS2l0V2ViVmlldy9wb3B1bGF0ZS1t ZW51IGFzc2VydHMgaW4gZGVidWcgYm90cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9OTg1MDcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBNYWtlIHN1cmUgd2UgcmV0YWluIHRoZSBwYWdlIFVSTCBiZWZvcmUg YXNraW5nIGZvciB0aGUgaWNvbi4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvZ3RrL1dlYktp dEZhdmljb25EYXRhYmFzZS5jcHA6CisgICAgICAgIChnZXRJY29uU3VyZmFjZVN5bmNocm9ub3Vz bHkpOiBSZXRhaW4gdGhlIHBhZ2UgVVJMIGJlZm9yZSBhc2tpbmcKKyAgICAgICAgZm9yIHRoZSBp Y29uLCBhbmQgcmVsZWFzZSBpdCBpZiB3ZSBmYWlsIHRvIGdldCBhbiBpY29uLgorICAgICAgICAo cHJvY2Vzc1BlbmRpbmdJY29uc0ZvclBhZ2VVUkwpOiBSZW5hbWVkIHRvIG1ha2UgaXQgY2xlYXIg aXQKKyAgICAgICAgcmVjZWl2ZXMgYSBTcmluZyBhbmQgbm90IGEgY2hhciogYW5kIG1vdmVkCisg ICAgICAgIGdldEljb25TdXJmYWNlU3luY2hyb25vdXNseSgpIGNhbGwgb3V0IG9mIHRoZSBsb29w IHRvIGNhbGwgaXQgb25seQorICAgICAgICBvbmNlIGV2ZW4gaWYgdGhlcmUgYXJlIG11bHRpcGxl IHJlcXVlc3RzIHBlbmRpbmcuCisgICAgICAgIChpY29uRGF0YVJlYWR5Rm9yUGFnZVVSTENhbGxi YWNrKTogRG8gbm90IHJldGFpbiB0aGUgcGFnZSBVUkwKKyAgICAgICAgaGVyZS4KKwogMjAxMi0x MC0wNSAgSm9uZ3Nlb2sgWWFuZyAgPGpzNDUueWFuZ0BzYW1zdW5nLmNvbT4KIAogICAgICAgICBb RUZMXVtXSzJdIFJlbW92ZSAid2ViIiB3b3JkIGZyb20gZXdrX3Jlc291cmNlIEFQSXMKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZ3RrL1dlYktpdEZhdmljb25EYXRh YmFzZS5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRGYXZpY29u RGF0YWJhc2UuY3BwCmluZGV4IGE2ZGRlNDMuLjc0YTZmYzEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZ3RrL1dlYktpdEZhdmljb25EYXRhYmFzZS5jcHAKKysrIGIv U291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9ndGsvV2ViS2l0RmF2aWNvbkRhdGFiYXNlLmNw cApAQCAtMTE5LDE3ICsxMTksMjEgQEAgc3RhdGljIGNhaXJvX3N1cmZhY2VfdCogZ2V0SWNvblN1 cmZhY2VTeW5jaHJvbm91c2x5KFdlYktpdEZhdmljb25EYXRhYmFzZSogZGF0YWIKIHsKICAgICBB U1NFUlQoaXNNYWluVGhyZWFkKCkpOwogCisgICAgZGF0YWJhc2UtPnByaXYtPmljb25EYXRhYmFz ZS0+cmV0YWluSWNvbkZvclBhZ2VVUkwocGFnZVVSTCk7CisKICAgICAvLyBUaGUgZXhhY3Qgc2l6 ZSB3ZSBwYXNzIGlzIGlycmVsZXZhbnQgdG8gdGhlIGljb25EYXRhYmFzZSBjb2RlLgogICAgIC8v IFdlIG11c3QgcGFzcyBzb21ldGhpbmcgZ3JlYXRlciB0aGFuIDB4MCB0byBnZXQgYW4gaWNvbi4K ICAgICBXZWJDb3JlOjpJbWFnZSogaWNvbkltYWdlID0gZGF0YWJhc2UtPnByaXYtPmljb25EYXRh YmFzZS0+aW1hZ2VGb3JQYWdlVVJMKHBhZ2VVUkwsIFdlYkNvcmU6OkludFNpemUoMSwgMSkpOwog ICAgIGlmICghaWNvbkltYWdlKSB7CiAgICAgICAgIGdfc2V0X2Vycm9yKGVycm9yLCBXRUJLSVRf RkFWSUNPTl9EQVRBQkFTRV9FUlJPUiwgV0VCS0lUX0ZBVklDT05fREFUQUJBU0VfRVJST1JfRkFW SUNPTl9VTktOT1dOLCBfKCJVbmtub3duIGZhdmljb24gZm9yIHBhZ2UgJXMiKSwgcGFnZVVSTC51 dGY4KCkuZGF0YSgpKTsKKyAgICAgICAgZGF0YWJhc2UtPnByaXYtPmljb25EYXRhYmFzZS0+cmVs ZWFzZUljb25Gb3JQYWdlVVJMKHBhZ2VVUkwpOwogICAgICAgICByZXR1cm4gMDsKICAgICB9CiAK ICAgICBXZWJDb3JlOjpOYXRpdmVJbWFnZVB0ciBpY29uID0gaWNvbkltYWdlLT5uYXRpdmVJbWFn ZUZvckN1cnJlbnRGcmFtZSgpOwogICAgIGlmICghaWNvbikgewogICAgICAgICBnX3NldF9lcnJv cihlcnJvciwgV0VCS0lUX0ZBVklDT05fREFUQUJBU0VfRVJST1IsIFdFQktJVF9GQVZJQ09OX0RB VEFCQVNFX0VSUk9SX0ZBVklDT05fTk9UX0ZPVU5ELCBfKCJQYWdlICVzIGRvZXMgbm90IGhhdmUg YSBmYXZpY29uIiksIHBhZ2VVUkwudXRmOCgpLmRhdGEoKSk7CisgICAgICAgIGRhdGFiYXNlLT5w cml2LT5pY29uRGF0YWJhc2UtPnJlbGVhc2VJY29uRm9yUGFnZVVSTChwYWdlVVJMKTsKICAgICAg ICAgcmV0dXJuIDA7CiAgICAgfQogCkBAIC0xNDIsMTcgKzE0NiwyNCBAQCBzdGF0aWMgdm9pZCBk ZWxldGVQZW5kaW5nSWNvblJlcXVlc3RzKFdlYktpdEZhdmljb25EYXRhYmFzZSogZGF0YWJhc2Us IFBlbmRpbmdJYwogICAgIGRlbGV0ZSByZXF1ZXN0czsKIH0KIAotc3RhdGljIHZvaWQgcHJvY2Vz c1BlbmRpbmdJY29uc0ZvclVSSShXZWJLaXRGYXZpY29uRGF0YWJhc2UqIGRhdGFiYXNlLCBjb25z dCBTdHJpbmcmIHBhZ2VVUkwpCitzdGF0aWMgdm9pZCBwcm9jZXNzUGVuZGluZ0ljb25zRm9yUGFn ZVVSTChXZWJLaXRGYXZpY29uRGF0YWJhc2UqIGRhdGFiYXNlLCBjb25zdCBTdHJpbmcmIHBhZ2VV UkwpCiB7CiAgICAgUGVuZGluZ0ljb25SZXF1ZXN0VmVjdG9yKiBpY29ucyA9IGRhdGFiYXNlLT5w cml2LT5wZW5kaW5nSWNvblJlcXVlc3RzLmdldChwYWdlVVJMKTsKICAgICBpZiAoIWljb25zKQog ICAgICAgICByZXR1cm47CiAKKyAgICBHT3duUHRyPEdFcnJvcj4gZXJyb3I7CisgICAgUmVmUHRy PGNhaXJvX3N1cmZhY2VfdD4gaWNvbiA9IGdldEljb25TdXJmYWNlU3luY2hyb25vdXNseShkYXRh YmFzZSwgcGFnZVVSTCwgJmVycm9yLm91dFB0cigpKTsKKwogICAgIGZvciAoc2l6ZV90IGkgPSAw OyBpIDwgaWNvbnMtPnNpemUoKTsgKytpKSB7CiAgICAgICAgIEdTaW1wbGVBc3luY1Jlc3VsdCog cmVzdWx0ID0gaWNvbnMtPmF0KGkpLmdldCgpOwogICAgICAgICBHZXRGYXZpY29uU3VyZmFjZUFz eW5jRGF0YSogZGF0YSA9IHN0YXRpY19jYXN0PEdldEZhdmljb25TdXJmYWNlQXN5bmNEYXRhKj4o Z19zaW1wbGVfYXN5bmNfcmVzdWx0X2dldF9vcF9yZXNfZ3BvaW50ZXIocmVzdWx0KSk7Ci0gICAg ICAgIGlmICghZ19jYW5jZWxsYWJsZV9pc19jYW5jZWxsZWQoZGF0YS0+Y2FuY2VsbGFibGUuZ2V0 KCkpKQotICAgICAgICAgICAgZGF0YS0+aWNvbiA9IGdldEljb25TdXJmYWNlU3luY2hyb25vdXNs eShkYXRhYmFzZSwgcGFnZVVSTCwgJmRhdGEtPmVycm9yLm91dFB0cigpKTsKKyAgICAgICAgaWYg KCFnX2NhbmNlbGxhYmxlX2lzX2NhbmNlbGxlZChkYXRhLT5jYW5jZWxsYWJsZS5nZXQoKSkpIHsK KyAgICAgICAgICAgIGlmIChlcnJvcikKKyAgICAgICAgICAgICAgICBnX3Byb3BhZ2F0ZV9lcnJv cigmZGF0YS0+ZXJyb3Iub3V0UHRyKCksIGVycm9yLnJlbGVhc2UoKSk7CisgICAgICAgICAgICBl bHNlCisgICAgICAgICAgICAgICAgZGF0YS0+aWNvbiA9IGljb247CisgICAgICAgIH0KIAogICAg ICAgICBnX3NpbXBsZV9hc3luY19yZXN1bHRfY29tcGxldGUocmVzdWx0KTsKICAgICB9CkBAIC0x NjQsMTEgKzE3NSwxMCBAQCBzdGF0aWMgdm9pZCBpY29uRGF0YVJlYWR5Rm9yUGFnZVVSTENhbGxi YWNrKFdLSWNvbkRhdGFiYXNlUmVmIHdrSWNvbkRhdGFiYXNlLCBXSwogICAgIEFTU0VSVChpc01h aW5UaHJlYWQoKSk7CiAKICAgICBXZWJLaXRGYXZpY29uRGF0YWJhc2UqIGRhdGFiYXNlID0gV0VC S0lUX0ZBVklDT05fREFUQUJBU0UoY2xpZW50SW5mbyk7Ci0gICAgU3RyaW5nIHBhZ2VVUkxTdHJp bmcgPSB0b0ltcGwod2tQYWdlVVJMKS0+c3RyaW5nKCk7CisgICAgU3RyaW5nIHBhZ2VVUkwgPSB0 b0ltcGwod2tQYWdlVVJMKS0+c3RyaW5nKCk7CiAKLSAgICBkYXRhYmFzZS0+cHJpdi0+aWNvbkRh dGFiYXNlLT5yZXRhaW5JY29uRm9yUGFnZVVSTChwYWdlVVJMU3RyaW5nKTsKLSAgICBwcm9jZXNz UGVuZGluZ0ljb25zRm9yVVJJKGRhdGFiYXNlLCBwYWdlVVJMU3RyaW5nKTsKLSAgICBnX3NpZ25h bF9lbWl0KGRhdGFiYXNlLCBzaWduYWxzW0lDT05fUkVBRFldLCAwLCBwYWdlVVJMU3RyaW5nLnV0 ZjgoKS5kYXRhKCkpOworICAgIHByb2Nlc3NQZW5kaW5nSWNvbnNGb3JQYWdlVVJMKGRhdGFiYXNl LCBwYWdlVVJMKTsKKyAgICBnX3NpZ25hbF9lbWl0KGRhdGFiYXNlLCBzaWduYWxzW0lDT05fUkVB RFldLCAwLCBwYWdlVVJMLnV0ZjgoKS5kYXRhKCkpOwogfQogCiBXZWJLaXRGYXZpY29uRGF0YWJh c2UqIHdlYmtpdEZhdmljb25EYXRhYmFzZUNyZWF0ZShXZWJJY29uRGF0YWJhc2UqIGljb25EYXRh YmFzZSkK