172671 2017-05-26 16:53:24 -0700 Add unit test for WebKit2 C SPI runBeforeUnloadConfirmPanel() 2017-05-30 20:19:36 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Local Build Mac macOS 10.12 RESOLVED FIXED InRadar P2 Normal --- 1 dbates dbates achristensen aestes beidson lforschler webkit-bug-importer oldest_to_newest 1313311 0 dbates 2017-05-26 16:53:24 -0700 We should add a unit test to ensure that we do not regress the WKPageUIClient runBeforeUnloadConfirmPanel() callback. Although we want to eventually remove the WebKit2 C SPI this is unlikely to happen immediately and there is value in adding testing coverage to ensure we do not cause regressions for existing clients. 1313313 1 311398 dbates 2017-05-26 16:55:18 -0700 Created attachment 311398 Unit test 1313330 2 311398 achristensen 2017-05-26 17:42:14 -0700 Comment on attachment 311398 Unit test View in context: https://bugs.webkit.org/attachment.cgi?id=311398&action=review > Tools/ChangeLog:8 > + We should add a unit test to ensure we do not regress the WKPageUIClient runBeforeUnloadConfirmPanel() callback. Could you add a reference to the revision this is intended to test? > Tools/TestWebKitAPI/Tests/WebKit2/ModalAlertsSPI.cpp:54 > + if (securityOrigin) { Was this always non-null before? Why did it change? 1313335 3 311398 dbates 2017-05-26 18:06:55 -0700 Comment on attachment 311398 Unit test View in context: https://bugs.webkit.org/attachment.cgi?id=311398&action=review >> Tools/TestWebKitAPI/Tests/WebKit2/ModalAlertsSPI.cpp:54 >> + if (securityOrigin) { > > Was this always non-null before? Why did it change? Please disregard this change. Will revert it before landing. securityOrigin is always non-null. 1313336 4 311398 dbates 2017-05-26 18:09:37 -0700 Comment on attachment 311398 Unit test View in context: https://bugs.webkit.org/attachment.cgi?id=311398&action=review >> Tools/ChangeLog:8 >> + We should add a unit test to ensure we do not regress the WKPageUIClient runBeforeUnloadConfirmPanel() callback. > > Could you add a reference to the revision this is intended to test? Adding this test is not part of a followup per-se. We never added a test for the runBeforeUnloadConfirmPanel callback since its inception (WKPageUIClientV0) :( 1313871 5 dbates 2017-05-30 14:10:36 -0700 (In reply to Daniel Bates from comment #3) > Comment on attachment 311398 [details] > Unit test > > View in context: > https://bugs.webkit.org/attachment.cgi?id=311398&action=review > > >> Tools/TestWebKitAPI/Tests/WebKit2/ModalAlertsSPI.cpp:54 > >> + if (securityOrigin) { > > > > Was this always non-null before? Why did it change? > > Please disregard this change. Will revert it before landing. securityOrigin > is always non-null. Wow, I was out of it. Keeping this change. Notice that we are not passed a security origin in the runBeforeUnloadConfirmPanel() callback. All other callbacks tested by this unit test are passed a security origin. 1313872 6 dbates 2017-05-30 14:13:02 -0700 Committed r217574: <http://trac.webkit.org/changeset/217574> 1313999 7 webkit-bug-importer 2017-05-30 20:19:36 -0700 <rdar://problem/32479652> 311398 2017-05-26 16:55:18 -0700 2017-05-26 17:41:09 -0700 Unit test bug-172671-20170526165518.patch text/plain 5614 dbates U3VidmVyc2lvbiBSZXZpc2lvbjogMjE3NTA3CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZWQ5ODk0OWZhNjA2YTQ5MDc5Nzk3ZWYzZWRjODMxMzIy Y2M3OGRmNC4uODU4M2I3Y2YyNTEyMGM2ZTRlYzk5YzRmNzBlODYwOTJiY2Q5YzI0MiAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5 IEBACisyMDE3LTA1LTI2ICBEYW5pZWwgQmF0ZXMgIDxkYWJhdGVzQGFwcGxlLmNvbT4KKworICAg ICAgICBBZGQgdW5pdCB0ZXN0IGZvciBXZWJLaXQyIEMgU1BJIHJ1bkJlZm9yZVVubG9hZENvbmZp cm1QYW5lbCgpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xNzI2NzEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBXZSBzaG91bGQgYWRkIGEgdW5pdCB0ZXN0IHRvIGVuc3VyZSB3ZSBkbyBub3QgcmVncmVzcyB0 aGUgV0tQYWdlVUlDbGllbnQgcnVuQmVmb3JlVW5sb2FkQ29uZmlybVBhbmVsKCkgY2FsbGJhY2su CisKKyAgICAgICAgKiBUZXN0V2ViS2l0QVBJL1Rlc3RzL1dlYktpdDIvTW9kYWxBbGVydHNTUEku Y3BwOgorICAgICAgICAoVGVzdFdlYktpdEFQSTo6YW5hbHl6ZURpYWxvZ0FyZ3VtZW50cyk6IFRl c3QgdGhlIHJ1bkJlZm9yZVVubG9hZENvbmZpcm1QYW5lbCgpIGNhbGxiYWNrIGxhc3QgYXMgd2Ug bmVlZAorICAgICAgICB0byB0cmlnZ2VyIGEgbmF2aWdhdGlvbiB0byB0ZXN0IGl0LgorICAgICAg ICAoVGVzdFdlYktpdEFQSTo6cnVuQmVmb3JlVW5sb2FkQ29uZmlybVBhbmVsKTogQWRkZWQuCisg ICAgICAgIChUZXN0V2ViS2l0QVBJOjpjcmVhdGVOZXdQYWdlKTogV2lyZSB1cCB0aGUgcnVuQmVm b3JlVW5sb2FkQ29uZmlybVBhbmVsIGNhbGxiYWNrLgorICAgICAgICAqIFRlc3RXZWJLaXRBUEkv VGVzdHMvV2ViS2l0Mi9tb2RhbC1hbGVydHMtaW4tbmV3LWFib3V0LWJsYW5rLXdpbmRvdy5odG1s OiBSZWdpc3RlciBhbiBvbmJlZm9yZXVubG9hZCBsaXN0ZW5lci4KKwogMjAxNy0wNS0yNiAgV2Vu c29uIEhzaWVoICA8d2Vuc29uX2hzaWVoQGFwcGxlLmNvbT4KIAogICAgICAgICBBZGQgdGVzdCBy ZXNvdXJjZXMgYmFjayBpbnRvIFRlc3RXZWJLaXRBUEkgQ29weSBSZXNvdXJjZXMgcGhhc2UKZGlm ZiAtLWdpdCBhL1Rvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV2ViS2l0Mi9Nb2RhbEFsZXJ0c1NQ SS5jcHAgYi9Ub29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dlYktpdDIvTW9kYWxBbGVydHNTUEku Y3BwCmluZGV4IDAyNzhmMDA1Yjk4NTA1ODgyZWI4YTRiMmFlMjhhNGUyN2UwNjU3OTcuLjEwNzA4 ZmFkMGJmMGJiOWQ3NzNmZTRkZTdhNzZlOWIzNDQ2NDhkNjMgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1Rl c3RXZWJLaXRBUEkvVGVzdHMvV2ViS2l0Mi9Nb2RhbEFsZXJ0c1NQSS5jcHAKKysrIGIvVG9vbHMv VGVzdFdlYktpdEFQSS9UZXN0cy9XZWJLaXQyL01vZGFsQWxlcnRzU1BJLmNwcApAQCAtMSw1ICsx LDUgQEAKIC8qCi0gKiBDb3B5cmlnaHQgKEMpIDIwMTUgQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJl c2VydmVkLgorICogQ29weXJpZ2h0IChDKSAyMDE1LTIwMTcgQXBwbGUgSW5jLiBBbGwgcmlnaHRz IHJlc2VydmVkLgogICoKICAqIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2UgaW4gc291cmNlIGFuZCBi aW5hcnkgZm9ybXMsIHdpdGggb3Igd2l0aG91dAogICogbW9kaWZpY2F0aW9uLCBhcmUgcGVybWl0 dGVkIHByb3ZpZGVkIHRoYXQgdGhlIGZvbGxvd2luZyBjb25kaXRpb25zCkBAIC0zOCw5ICszOCwx MiBAQCBuYW1lc3BhY2UgVGVzdFdlYktpdEFQSSB7CiAKIHN0YXRpYyBib29sIGRvbmU7CiBzdGF0 aWMgdW5zaWduZWQgZGlhbG9nc1NlZW47Ci1zdGF0aWMgY29uc3QgdW5zaWduZWQgZGlhbG9nc0V4 cGVjdGVkID0gMzsKK3N0YXRpYyBjb25zdCB1bnNpZ25lZCBkaWFsb2dzRXhwZWN0ZWQgPSA0Owor c3RhdGljIGNvbnN0IHVuc2lnbmVkIGRpYWxvZ3NCZWZvcmVVbmxvYWRDb25maXJtUGFuZWwgPSBk aWFsb2dzRXhwZWN0ZWQgLSAxOwogCi1zdGF0aWMgdm9pZCBhbmFseXplRGlhbG9nQXJndW1lbnRz KFdLUGFnZVJlZiBwYWdlLCBXS0ZyYW1lUmVmIGZyYW1lLCBXS1NlY3VyaXR5T3JpZ2luUmVmIHNl Y3VyaXR5T3JpZ2luKQorc3RhdGljIHN0ZDo6dW5pcXVlX3B0cjxQbGF0Zm9ybVdlYlZpZXc+IG9w ZW5lZFdlYlZpZXc7CisKK3N0YXRpYyB2b2lkIGFuYWx5emVEaWFsb2dBcmd1bWVudHMoV0tQYWdl UmVmIHBhZ2UsIFdLRnJhbWVSZWYgZnJhbWUsIFdLU2VjdXJpdHlPcmlnaW5SZWYgc2VjdXJpdHlP cmlnaW4gPSBudWxscHRyKQogewogICAgIEVYUEVDVF9FUShwYWdlLCBXS0ZyYW1lR2V0UGFnZShm cmFtZSkpOwogCkBAIC00OCwxNSArNTEsMjggQEAgc3RhdGljIHZvaWQgYW5hbHl6ZURpYWxvZ0Fy Z3VtZW50cyhXS1BhZ2VSZWYgcGFnZSwgV0tGcmFtZVJlZiBmcmFtZSwgV0tTZWN1cml0eU8KICAg ICBXS1JldGFpblB0cjxXS1N0cmluZ1JlZj4gdXJsU3RyaW5nID0gYWRvcHRXSyhXS1VSTENvcHlT dHJpbmcodXJsLmdldCgpKSk7CiAgICAgRVhQRUNUX1dLX1NUUkVRKCJhYm91dDpibGFuayIsIHVy bFN0cmluZy5nZXQoKSk7CiAKLSAgICBXS1JldGFpblB0cjxXS1N0cmluZ1JlZj4gcHJvdG9jb2wg PSBhZG9wdFdLKFdLU2VjdXJpdHlPcmlnaW5Db3B5UHJvdG9jb2woc2VjdXJpdHlPcmlnaW4pKTsK LSAgICBFWFBFQ1RfV0tfU1RSRVEoImZpbGUiLCBwcm90b2NvbC5nZXQoKSk7CisgICAgaWYgKHNl Y3VyaXR5T3JpZ2luKSB7CisgICAgICAgIFdLUmV0YWluUHRyPFdLU3RyaW5nUmVmPiBwcm90b2Nv bCA9IGFkb3B0V0soV0tTZWN1cml0eU9yaWdpbkNvcHlQcm90b2NvbChzZWN1cml0eU9yaWdpbikp OworICAgICAgICBFWFBFQ1RfV0tfU1RSRVEoImZpbGUiLCBwcm90b2NvbC5nZXQoKSk7CisKKyAg ICAgICAgV0tSZXRhaW5QdHI8V0tTdHJpbmdSZWY+IGhvc3QgPSBhZG9wdFdLKFdLU2VjdXJpdHlP cmlnaW5Db3B5SG9zdChzZWN1cml0eU9yaWdpbikpOworICAgICAgICBFWFBFQ1RfV0tfU1RSRVEo IiIsIGhvc3QuZ2V0KCkpOworCisgICAgICAgIEVYUEVDVF9FUShXS1NlY3VyaXR5T3JpZ2luR2V0 UG9ydChzZWN1cml0eU9yaWdpbiksIDApOworICAgIH0KIAotICAgIFdLUmV0YWluUHRyPFdLU3Ry aW5nUmVmPiBob3N0ID0gYWRvcHRXSyhXS1NlY3VyaXR5T3JpZ2luQ29weUhvc3Qoc2VjdXJpdHlP cmlnaW4pKTsKLSAgICBFWFBFQ1RfV0tfU1RSRVEoIiIsIGhvc3QuZ2V0KCkpOworICAgICsrZGlh bG9nc1NlZW47CiAKLSAgICBFWFBFQ1RfRVEoV0tTZWN1cml0eU9yaWdpbkdldFBvcnQoc2VjdXJp dHlPcmlnaW4pLCAwKTsKKyAgICAvLyBUZXN0IGJlZm9yZXVubG9hZCBsYXN0IGFzIHdlIG5lZWQg dG8gbmF2aWdhdGUgdGhlIHdpbmRvdyB0byB0cmlnZ2VyIGl0LgorICAgIGlmIChkaWFsb2dzU2Vl biA9PSBkaWFsb2dzQmVmb3JlVW5sb2FkQ29uZmlybVBhbmVsKSB7CisgICAgICAgIC8vIFVzZXIg aW50ZXJhY3Rpb24gaXMgcmVxdWlyZWQgdG8gc2hvdyB0aGUgYmVmb3JldW5sb2FkIHByb21wdC4K KyAgICAgICAgb3BlbmVkV2ViVmlldy0+c2ltdWxhdGVCdXR0b25DbGljayhrV0tFdmVudE1vdXNl QnV0dG9uTGVmdEJ1dHRvbiwgNSwgNSwgMCk7CisgICAgICAgIFdLUmV0YWluUHRyPFdLVVJMUmVm PiBibGFua0RhdGFVUkwgPSBhZG9wdFdLKFdLVVJMQ3JlYXRlV2l0aFVURjhDU3RyaW5nKCJkYXRh OnRleHQvaHRtbCIpKTsKKyAgICAgICAgV0tQYWdlTG9hZFVSTChwYWdlLCBibGFua0RhdGFVUkwu Z2V0KCkpOworICAgICAgICByZXR1cm47CisgICAgfQogCi0gICAgaWYgKCsrZGlhbG9nc1NlZW4g PT0gZGlhbG9nc0V4cGVjdGVkKQorICAgIGlmIChkaWFsb2dzU2VlbiA9PSBkaWFsb2dzRXhwZWN0 ZWQpCiAgICAgICAgIGRvbmUgPSB0cnVlOwogfQogCkBAIC03Nyw3ICs5MywxMSBAQCBzdGF0aWMg V0tTdHJpbmdSZWYgcnVuSmF2YVNjcmlwdFByb21wdChXS1BhZ2VSZWYgcGFnZSwgV0tTdHJpbmdS ZWYsIFdLU3RyaW5nUmVmLAogICAgIHJldHVybiBudWxscHRyOwogfQogCi1zdGF0aWMgc3RkOjp1 bmlxdWVfcHRyPFBsYXRmb3JtV2ViVmlldz4gb3BlbmVkV2ViVmlldzsKK3N0YXRpYyBib29sIHJ1 bkJlZm9yZVVubG9hZENvbmZpcm1QYW5lbChXS1BhZ2VSZWYgcGFnZSwgV0tTdHJpbmdSZWYsIFdL RnJhbWVSZWYgZnJhbWUsIGNvbnN0IHZvaWQqKQoreworICAgIGFuYWx5emVEaWFsb2dBcmd1bWVu dHMocGFnZSwgZnJhbWUpOworICAgIHJldHVybiBmYWxzZTsKK30KIAogc3RhdGljIFdLUGFnZVJl ZiBjcmVhdGVOZXdQYWdlKFdLUGFnZVJlZiBwYWdlLCBXS1VSTFJlcXVlc3RSZWYgdXJsUmVxdWVz dCwgV0tEaWN0aW9uYXJ5UmVmIGZlYXR1cmVzLCBXS0V2ZW50TW9kaWZpZXJzIG1vZGlmaWVycywg V0tFdmVudE1vdXNlQnV0dG9uIG1vdXNlQnV0dG9uLCBjb25zdCB2b2lkICpjbGllbnRJbmZvKQog ewpAQCAtOTIsNiArMTEyLDcgQEAgc3RhdGljIFdLUGFnZVJlZiBjcmVhdGVOZXdQYWdlKFdLUGFn ZVJlZiBwYWdlLCBXS1VSTFJlcXVlc3RSZWYgdXJsUmVxdWVzdCwgV0tEaWMKICAgICB1aUNsaWVu dC5ydW5KYXZhU2NyaXB0QWxlcnQgPSBydW5KYXZhU2NyaXB0QWxlcnQ7CiAgICAgdWlDbGllbnQu cnVuSmF2YVNjcmlwdENvbmZpcm0gPSBydW5KYXZhU2NyaXB0Q29uZmlybTsKICAgICB1aUNsaWVu dC5ydW5KYXZhU2NyaXB0UHJvbXB0ID0gcnVuSmF2YVNjcmlwdFByb21wdDsKKyAgICB1aUNsaWVu dC5ydW5CZWZvcmVVbmxvYWRDb25maXJtUGFuZWwgPSBydW5CZWZvcmVVbmxvYWRDb25maXJtUGFu ZWw7CiAKICAgICBXS1BhZ2VTZXRQYWdlVUlDbGllbnQob3BlbmVkV2ViVmlldy0+cGFnZSgpLCAm dWlDbGllbnQuYmFzZSk7CiAKZGlmZiAtLWdpdCBhL1Rvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMv V2ViS2l0Mi9tb2RhbC1hbGVydHMtaW4tbmV3LWFib3V0LWJsYW5rLXdpbmRvdy5odG1sIGIvVG9v bHMvVGVzdFdlYktpdEFQSS9UZXN0cy9XZWJLaXQyL21vZGFsLWFsZXJ0cy1pbi1uZXctYWJvdXQt Ymxhbmstd2luZG93Lmh0bWwKaW5kZXggZjc2Y2I2NTBiN2FiOGNiNTFkYTc5OTVjMWU5MWJkYzQw YTRkODAxMS4uMTliZjM5ZDA3ZWY5MDE2MTE1ZGZlNGU0ZDlkNTJjYjNkMDQ5ZDEwMCAxMDA2NDQK LS0tIGEvVG9vbHMvVGVzdFdlYktpdEFQSS9UZXN0cy9XZWJLaXQyL21vZGFsLWFsZXJ0cy1pbi1u ZXctYWJvdXQtYmxhbmstd2luZG93Lmh0bWwKKysrIGIvVG9vbHMvVGVzdFdlYktpdEFQSS9UZXN0 cy9XZWJLaXQyL21vZGFsLWFsZXJ0cy1pbi1uZXctYWJvdXQtYmxhbmstd2luZG93Lmh0bWwKQEAg LTcsNiArNyw3IEBAIHZhciBuZXdXaW5kb3cgPSB3aW5kb3cub3BlbigiYWJvdXQ6YmxhbmsiKTsK IG5ld1dpbmRvdy5hbGVydCgiVGVzdGluZyBhbGVydCIpOwogbmV3V2luZG93LmNvbmZpcm0oIlRl c3RpbmcgY29uZmlybSIpOwogbmV3V2luZG93LnByb21wdCgiVGVzdGluZyBwcm9tcHQiLCAiRGVm YXVsdCB0ZXh0Iik7CituZXdXaW5kb3cub25iZWZvcmV1bmxvYWQgPSAoKSA9PiAiRGVmYXVsdCB0 ZXh0IjsKIAogPC9zY3JpcHQ+CiA8L2hlYWQ+Cg==