Bug 172671 – Add unit test for WebKit2 C SPI runBeforeUnloadConfirmPanel()

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Daniel Bates]

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.

[Daniel Bates]

Created attachment 311398 [details]
Unit test

[Alex Christensen]

Comment on attachment 311398 [details]
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?

[Daniel Bates]

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.

[Daniel Bates]

Comment on attachment 311398 [details]
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) :(

[Daniel Bates]

(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.

[Daniel Bates]

Committed r217574: <http://trac.webkit.org/changeset/217574>

[Radar WebKit Bug Importer]

<rdar://problem/32479652>