Bug 146249

Summary: Update JavaScript dialog delegates to include a WKSecurityOriginRef argument
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit2Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, mitz
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=146396
Attachments:
Description Flags
Patch v1 achristensen: review+

Description Brady Eidson 2015-06-23 14:40:01 PDT
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
Comment 1 Brady Eidson 2015-06-23 14:44:25 PDT
Created attachment 255437 [details]
Patch v1
Comment 2 WebKit Commit Bot 2015-06-23 14:46:34 PDT
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.
Comment 3 Alex Christensen 2015-06-23 16:13:10 PDT
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.
Comment 4 Brady Eidson 2015-06-23 17:06:50 PDT
(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!
Comment 5 Brady Eidson 2015-06-24 09:59:04 PDT
http://trac.webkit.org/changeset/185915
Comment 6 mitz 2015-06-28 08:32:48 PDT
(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.