Bug 146249 – Update JavaScript dialog delegates to include a WKSecurityOriginRef argument

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

Update JavaScript dialog delegates to include a WKSecurityOriginRef argument.

This is the C-SPI version of https://bugs.webkit.org/show_bug.cgi?id=146162

In radar as rdar://problem/21269187

[Brady Eidson]

Created attachment 255437 [details]
Patch v1

[WebKit Commit Bot]

Attachment 255437 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/C/WKPage.cpp:1411:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/API/C/WKPage.cpp:1427:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/API/C/WKPage.cpp:1444:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Comment on attachment 255437 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=255437&action=review

> Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:121
> +    WKPageRunJavaScriptAlertCallback_deprecatedForUseWithV5             runJavaScriptAlert_deprecatedForUseWithV5;
> +    WKPageRunJavaScriptConfirmCallback_deprecatedForUseWithV5           runJavaScriptConfirm_deprecatedForUseWithV5;
> +    WKPageRunJavaScriptPromptCallback_deprecatedForUseWithV5            runJavaScriptPrompt_deprecatedForUseWithV5;

You shouldn't need to change the name of V0-V4 and the unnamed one, right?

> Tools/TestWebKitAPI/Tests/WebKit2/DocumentStartUserScriptAlertCrash.cpp:60
> +    uiClient.runJavaScriptAlert_deprecatedForUseWithV5 = runJavaScriptAlert;

otter (ditto to a later comment)

> Tools/TestWebKitAPI/Tests/mac/FullscreenZoomInitialFrame.mm:115
> +    uiClient.runJavaScriptAlert_deprecatedForUseWithV5 = runJavaScriptAlert;

Please change the name of the function, too.  Line 60.  That will avoid lots of confusion.

> Tools/TestWebKitAPI/Tests/mac/PageVisibilityStateWithWindowChanges.mm:102
> +    uiClient.runJavaScriptAlert_deprecatedForUseWithV5 = runJavaScriptAlert;

ditto.

[Brady Eidson]

(In reply to comment #3)
> Comment on attachment 255437 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=255437&action=review
> 
> > Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:121
> > +    WKPageRunJavaScriptAlertCallback_deprecatedForUseWithV5             runJavaScriptAlert_deprecatedForUseWithV5;
> > +    WKPageRunJavaScriptConfirmCallback_deprecatedForUseWithV5           runJavaScriptConfirm_deprecatedForUseWithV5;
> > +    WKPageRunJavaScriptPromptCallback_deprecatedForUseWithV5            runJavaScriptPrompt_deprecatedForUseWithV5;
> 
> You shouldn't need to change the name of V0-V4 and the unnamed one, right?

That's actually not right - We change the name in earlier versions.

But this did make me realize ForUseWithV5 is wrong - It's ForUseWithV0, as we tag with the first version the method originally appeared in.


> 
> > Tools/TestWebKitAPI/Tests/WebKit2/DocumentStartUserScriptAlertCrash.cpp:60
> > +    uiClient.runJavaScriptAlert_deprecatedForUseWithV5 = runJavaScriptAlert;
> 
> otter (ditto to a later comment)
> 
> > Tools/TestWebKitAPI/Tests/mac/FullscreenZoomInitialFrame.mm:115
> > +    uiClient.runJavaScriptAlert_deprecatedForUseWithV5 = runJavaScriptAlert;
> 
> Please change the name of the function, too.  Line 60.  That will avoid lots
> of confusion.
> 
> > Tools/TestWebKitAPI/Tests/mac/PageVisibilityStateWithWindowChanges.mm:102
> > +    uiClient.runJavaScriptAlert_deprecatedForUseWithV5 = runJavaScriptAlert;
> 
> ditto.

Will do, thanks!

[Brady Eidson]

http://trac.webkit.org/changeset/185915

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 255437 [details]
> > Patch v1
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=255437&action=review
> > 
> > > Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:121
> > > +    WKPageRunJavaScriptAlertCallback_deprecatedForUseWithV5             runJavaScriptAlert_deprecatedForUseWithV5;
> > > +    WKPageRunJavaScriptConfirmCallback_deprecatedForUseWithV5           runJavaScriptConfirm_deprecatedForUseWithV5;
> > > +    WKPageRunJavaScriptPromptCallback_deprecatedForUseWithV5            runJavaScriptPrompt_deprecatedForUseWithV5;
> > 
> > You shouldn't need to change the name of V0-V4 and the unnamed one, right?
> 
> That's actually not right - We change the name in earlier versions.

The goes back to when we intended for clients to be using the “current” version of the clients. Now the clients are using specific versions, there is no reason to introduce a source incompatibility like this.

Filed bug 146396.