107036 2013-01-16 11:07:45 -0800 [GTK] Remove null check for WebPopupMenuProxy::Client in WebPopupMenuProxyGtk::showPopupMenu 2014-02-09 02:24:29 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID Gtk, LayoutTestFailure P2 Normal --- 128495 104114 1 zan zan andersca commit-queue mrobinson sam oldest_to_newest 808540 0 zan 2013-01-16 11:07:45 -0800 Based on a Coverity warning listed in bug #104114. 808572 1 183014 zan 2013-01-16 11:40:25 -0800 Created attachment 183014 Patch 808576 2 183014 mrobinson 2013-01-16 11:45:31 -0800 Comment on attachment 183014 Patch Hrm. Is this correct? Could it be possible for m_client to become null during the execution of the event loop? 808589 3 183014 zan 2013-01-16 12:03:07 -0800 Comment on attachment 183014 Patch No, this approach is incorrect. 811610 4 183666 zan 2013-01-20 05:10:56 -0800 Created attachment 183666 Patch 811611 5 zan 2013-01-20 05:12:10 -0800 This now deserves a better title. 811612 6 183667 zan 2013-01-20 05:13:32 -0800 Created attachment 183667 Patch 812173 7 183667 mrobinson 2013-01-21 08:14:45 -0800 Comment on attachment 183667 Patch So you're positive that the client cannot be invalidated by running the main loop? 812339 8 zan 2013-01-21 12:36:27 -0800 (In reply to comment #7) > (From update of attachment 183667 [details]) > So you're positive that the client cannot be invalidated by running the main loop? There are three places where the popup can get invalidated. 1. http://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp#L3065 This invalidation happens after the hidePopupMenu is called on the WebPopupMenuProxy. This call pops down the actual menu, which causes the unmap signal to be handled - there, the loop of the popup menu proxy is stopped and the selected item (if any) is signaled to the client. Only after that is the popup menu proxy invalidated. 2. http://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp#L3083 This invalidation happens after the call to the showPopupMenu method on the WebPopupMenuProxy returns. This method runs a loop until a selection is made in the popup menu or the popup menu is unmapped. After the loop is stopped (and still in the showPopupMenu) the selected item (if any) is signaled to the client. Again, only after that is the popup menu proxy invalidated. 3. http://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp#L3094 Same as #1. So I believe the current code cannot cause the invalidation while the loop is running. I guess a WebKit2 owner is required to look at this? 814246 9 mrobinson 2013-01-23 08:56:28 -0800 Okay. I guess we need a nod from a WebKit2 OWNER. 852378 10 zan 2013-03-11 09:00:15 -0700 Adding WK2 owners to the CC list. 971977 11 zan 2014-01-25 10:43:36 -0800 Committed r162768: <http://trac.webkit.org/changeset/162768> 978563 12 commit-queue 2014-02-09 02:19:56 -0800 Re-opened since this is blocked by bug 128495 978568 13 zan 2014-02-09 02:24:29 -0800 The popup menu proxy can be invalidated through the HidePopupMenu message from the WebKitWebProces before it exits the run loop. Because of that the early return is actually needed. 183014 2013-01-16 11:40:25 -0800 2013-01-16 12:03:07 -0800 Patch bug-107036-20130116203727.patch text/plain 1861 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTM5ODc3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZGJjYTQyZGM2MGQyOGU5 MzZkODUyZjhkZTdlNzU5M2NlNjA0YmQwNy4uN2I5MzdlYjViZGE1M2MzZmU0MzlkOWZmZDFkZGQ3 ODQwNzM1ZDI4NyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDEzLTAxLTE2ICBaYW4g RG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1RLXSBDaGVjayBm b3IgbV9jbGllbnQgYXQgdGhlIGJlZ2lubmluZyBvZiBXZWJQb3B1cE1lbnVQcm94eUd0azo6c2hv d1BvcHVwTWVudQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTA3MDM2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgKiBVSVByb2Nlc3MvZ3RrL1dlYlBvcHVwTWVudVByb3h5R3RrLmNwcDoKKyAgICAgICAgKFdl YktpdDo6V2ViUG9wdXBNZW51UHJveHlHdGs6OnNob3dQb3B1cE1lbnUpOiBSZXR1cm4gYXQgdGhl IHN0YXJ0IG9mIHRoZSBtZXRob2QKKyAgICAgICAgaWYgdGhlIGNsaWVudCBpcyBudWxsIChmb3Ig aW5zdGFuY2UgYWZ0ZXIgdGhlIHByb3h5IGlzIGludmFsaWRhdGVkKS4KKwogMjAxMy0wMS0xNSAg QW5kZXJzIENhcmxzc29uICA8YW5kZXJzY2FAYXBwbGUuY29tPgogCiAgICAgICAgIE1lcmdlIExh eWVyVHJlZUhvc3RDQSBhbmQgTGF5ZXJUcmVlSG9zdE1hYwpkaWZmIC0tZ2l0IGEvU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL2d0ay9XZWJQb3B1cE1lbnVQcm94eUd0ay5jcHAgYi9Tb3VyY2UvV2Vi S2l0Mi9VSVByb2Nlc3MvZ3RrL1dlYlBvcHVwTWVudVByb3h5R3RrLmNwcAppbmRleCBhNDE4Mzlh M2ViOTc2NGVkNDczNGFmMmU5NzY4NDc3ZTkwZWYzNDI3Li5kNTAzYjIzZTUyOWEwOWFjYmUwNzdj ZDI1ZTA5Zjg5NmNmZTIxODQ3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv Z3RrL1dlYlBvcHVwTWVudVByb3h5R3RrLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nl c3MvZ3RrL1dlYlBvcHVwTWVudVByb3h5R3RrLmNwcApAQCAtNjUsNiArNjUsOSBAQCBHdGtBY3Rp b24qIFdlYlBvcHVwTWVudVByb3h5R3RrOjpjcmVhdGVHdGtBY3Rpb25Gb3JNZW51SXRlbShjb25z dCBXZWJQb3B1cEl0ZW0mCiAKIHZvaWQgV2ViUG9wdXBNZW51UHJveHlHdGs6OnNob3dQb3B1cE1l bnUoY29uc3QgSW50UmVjdCYgcmVjdCwgVGV4dERpcmVjdGlvbiB0ZXh0RGlyZWN0aW9uLCBkb3Vi bGUgcGFnZVNjYWxlRmFjdG9yLCBjb25zdCBWZWN0b3I8V2ViUG9wdXBJdGVtPiYgaXRlbXMsIGNv bnN0IFBsYXRmb3JtUG9wdXBNZW51RGF0YSYgZGF0YSwgaW50MzJfdCBzZWxlY3RlZEluZGV4KQog eworICAgIGlmICghbV9jbGllbnQpCisgICAgICAgIHJldHVybjsKKwogICAgIGlmIChtX3BvcHVw KQogICAgICAgICBtX3BvcHVwLT5jbGVhcigpOwogICAgIGVsc2UKQEAgLTEwNSw5ICsxMDgsNiBA QCB2b2lkIFdlYlBvcHVwTWVudVByb3h5R3RrOjpzaG93UG9wdXBNZW51KGNvbnN0IEludFJlY3Qm IHJlY3QsIFRleHREaXJlY3Rpb24gdGV4dAogCiAgICAgZ19zaWduYWxfaGFuZGxlcl9kaXNjb25u ZWN0KG1fcG9wdXAtPnBsYXRmb3JtTWVudSgpLCB1bm1hcEhhbmRsZXIpOwogCi0gICAgaWYgKCFt X2NsaWVudCkKLSAgICAgICAgcmV0dXJuOwotCiAgICAgbV9jbGllbnQtPnZhbHVlQ2hhbmdlZEZv clBvcHVwTWVudSh0aGlzLCBtX2FjdGl2ZUl0ZW0pOwogfQogCg== 183666 2013-01-20 05:10:56 -0800 2013-01-20 05:13:28 -0800 Patch bug-107036-20130120140753.patch text/plain 1759 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTQwMjcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMGE4Zjk5MjczNDM0ZDgx OGEyNzBiMjZiZGVkYjdhMjg4MTNlZTU5ZC4uMDAwMWRiZjFjZWU5YzE4ZDcxMjgzMjg4Zjk1OGI2 ZjU5YzExMmFjYiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEzLTAxLTIwICBaYW4g RG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1RLXSBDaGVjayBm b3IgbV9jbGllbnQgYXQgdGhlIGJlZ2lubmluZyBvZiBXZWJQb3B1cE1lbnVQcm94eUd0azo6c2hv d1BvcHVwTWVudQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTA3MDM2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgKiBVSVByb2Nlc3MvZ3RrL1dlYlBvcHVwTWVudVByb3h5R3RrLmNwcDoKKyAgICAgICAgKFdl YktpdDo6V2ViUG9wdXBNZW51UHJveHlHdGs6OnNob3dQb3B1cE1lbnUpOiBEb24ndCBwZXJmb3Jt IGFuIGVhcmx5IHJldHVybgorICAgICAgICBpZiB0aGUgY2xpZW50IGlzIG5vdCBhdmFpbGFibGUg LSBpdCBhbHdheXMgc2hvdWxkIGJlLiBUaGUgY2xpZW50IGlzIHNldCB0byBudWxsCisgICAgICAg IG9ubHkgd2hlbiB0aGUgcHJveHkgaXMgaW52YWxpZGF0ZWQuIFRoaXMgaGFwcGVucyBpbW1lZGlh dGVseSBhZnRlciB0aGlzIGZ1bmN0aW9uCisgICAgICAgIHJldHVybnMsIGVpdGhlciB3aXRoIHRo ZSBwb3B1cCBiZWluZyBmb3JjZWQgdG8gY2xvc2Ugb3IgYmVpbmcgY2xvc2VkIGJ5IHRoZSB1c2Vy CisgICAgICAgIHdpdGggcG9zc2libHkgYW4gaXRlbSBiZWluZyBjaG9zZW4uIFJhdGhlciB0aGFu IHRoZSBlYXJseSByZXR1cm4sIHVzZSBhbiBBU1NFUlQuCisKIDIwMTMtMDEtMTcgIFNhbSBXZWlu aWcgIDxzYW1Ad2Via2l0Lm9yZz4KIAogICAgICAgICBEb24ndCBpbml0aWFsaXplIEFwcEtpdCBm b3IgcHJvY2Vzc2VzIHRoYXQgZG9uJ3QgdXNlIGl0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0 Mi9VSVByb2Nlc3MvZ3RrL1dlYlBvcHVwTWVudVByb3h5R3RrLmNwcCBiL1NvdXJjZS9XZWJLaXQy L1VJUHJvY2Vzcy9ndGsvV2ViUG9wdXBNZW51UHJveHlHdGsuY3BwCmluZGV4IGE0MTgzOWEzZWI5 NzY0ZWQ0NzM0YWYyZTk3Njg0NzdlOTBlZjM0MjcuLmNlZWFjNjlhNDJhMjUxOWMzZTI0MTQ4NTRh ZjZlODdhNDI4MTJlNjcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9ndGsv V2ViUG9wdXBNZW51UHJveHlHdGsuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9n dGsvV2ViUG9wdXBNZW51UHJveHlHdGsuY3BwCkBAIC0xMDUsOSArMTA1LDcgQEAgdm9pZCBXZWJQ b3B1cE1lbnVQcm94eUd0azo6c2hvd1BvcHVwTWVudShjb25zdCBJbnRSZWN0JiByZWN0LCBUZXh0 RGlyZWN0aW9uIHRleHQKIAogICAgIGdfc2lnbmFsX2hhbmRsZXJfZGlzY29ubmVjdChtX3BvcHVw LT5wbGF0Zm9ybU1lbnUoKSwgdW5tYXBIYW5kbGVyKTsKIAotICAgIGlmICghbV9jbGllbnQpCi0g ICAgICAgIHJldHVybjsKLQorICAgIEFTU0VSVChtX2NsaWVudCk7CiAgICAgbV9jbGllbnQtPnZh bHVlQ2hhbmdlZEZvclBvcHVwTWVudSh0aGlzLCBtX2FjdGl2ZUl0ZW0pOwogfQogCg== 183667 2013-01-20 05:13:32 -0800 2014-01-24 16:51:25 -0800 Patch bug-107036-20130120141029.patch text/plain 1771 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTQwMjcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMGE4Zjk5MjczNDM0ZDgx OGEyNzBiMjZiZGVkYjdhMjg4MTNlZTU5ZC4uZWYyY2RkMjM3ZWU2ZTYwOTA4MGNlODNiOWI5OWZl Y2VlMGY5MTNlYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEzLTAxLTIwICBaYW4g RG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1RLXSBSZW1vdmUg bnVsbCBjaGVjayBmb3IgV2ViUG9wdXBNZW51UHJveHk6OkNsaWVudCBpbiBXZWJQb3B1cE1lbnVQ cm94eUd0azo6c2hvd1BvcHVwTWVudQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MTA3MDM2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvZ3RrL1dlYlBvcHVwTWVudVByb3h5R3RrLmNwcDoK KyAgICAgICAgKFdlYktpdDo6V2ViUG9wdXBNZW51UHJveHlHdGs6OnNob3dQb3B1cE1lbnUpOiBE b24ndCBwZXJmb3JtIGFuIGVhcmx5IHJldHVybgorICAgICAgICBpZiB0aGUgY2xpZW50IGlzIG5v dCBhdmFpbGFibGUgLSBpdCBhbHdheXMgc2hvdWxkIGJlLiBUaGUgY2xpZW50IGlzIHNldCB0byBu dWxsCisgICAgICAgIG9ubHkgd2hlbiB0aGUgcHJveHkgaXMgaW52YWxpZGF0ZWQuIFRoaXMgaGFw cGVucyBpbW1lZGlhdGVseSBhZnRlciB0aGlzIGZ1bmN0aW9uCisgICAgICAgIHJldHVybnMsIGVp dGhlciB3aXRoIHRoZSBwb3B1cCBiZWluZyBmb3JjZWQgdG8gY2xvc2Ugb3IgYmVpbmcgY2xvc2Vk IGJ5IHRoZSB1c2VyCisgICAgICAgIHdpdGggcG9zc2libHkgYW4gaXRlbSBiZWluZyBjaG9zZW4u IFJhdGhlciB0aGFuIHRoZSBlYXJseSByZXR1cm4sIHVzZSBhbiBBU1NFUlQuCisKIDIwMTMtMDEt MTcgIFNhbSBXZWluaWcgIDxzYW1Ad2Via2l0Lm9yZz4KIAogICAgICAgICBEb24ndCBpbml0aWFs aXplIEFwcEtpdCBmb3IgcHJvY2Vzc2VzIHRoYXQgZG9uJ3QgdXNlIGl0CmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZ3RrL1dlYlBvcHVwTWVudVByb3h5R3RrLmNwcCBiL1Nv dXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9ndGsvV2ViUG9wdXBNZW51UHJveHlHdGsuY3BwCmluZGV4 IGE0MTgzOWEzZWI5NzY0ZWQ0NzM0YWYyZTk3Njg0NzdlOTBlZjM0MjcuLmNlZWFjNjlhNDJhMjUx OWMzZTI0MTQ4NTRhZjZlODdhNDI4MTJlNjcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJ UHJvY2Vzcy9ndGsvV2ViUG9wdXBNZW51UHJveHlHdGsuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQy L1VJUHJvY2Vzcy9ndGsvV2ViUG9wdXBNZW51UHJveHlHdGsuY3BwCkBAIC0xMDUsOSArMTA1LDcg QEAgdm9pZCBXZWJQb3B1cE1lbnVQcm94eUd0azo6c2hvd1BvcHVwTWVudShjb25zdCBJbnRSZWN0 JiByZWN0LCBUZXh0RGlyZWN0aW9uIHRleHQKIAogICAgIGdfc2lnbmFsX2hhbmRsZXJfZGlzY29u bmVjdChtX3BvcHVwLT5wbGF0Zm9ybU1lbnUoKSwgdW5tYXBIYW5kbGVyKTsKIAotICAgIGlmICgh bV9jbGllbnQpCi0gICAgICAgIHJldHVybjsKLQorICAgIEFTU0VSVChtX2NsaWVudCk7CiAgICAg bV9jbGllbnQtPnZhbHVlQ2hhbmdlZEZvclBvcHVwTWVudSh0aGlzLCBtX2FjdGl2ZUl0ZW0pOwog fQogCg==