Bug 66823 – Add the request info to the WebPageProxy::CreateNewPage message

• Component: WebKit2
• Status: RESOLVED FIXED

[Alice Liu]

Add the request info to the WebPageProxy::CreateNewPage message.  WebKit clients will then have more information, such as the request url, to help them decide on how to handle a CreateNewPage message.

[Alice Liu]

Created attachment 104937 [details]
patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:53:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 12 files


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

[Alice Liu]

Created attachment 104942 [details]
patch w/ fixed style

[Alice Liu]

Created attachment 104946 [details]
patch with fixes for MiniBrowser on gtk and windows

[Alexey Proskuryakov]

Sending initial request to the client seems quite surprising. What is the client going to do with this information? The request can result in a redirect, so a completely different resource will be used in the end.

Also, it seems like this patch would break nightly builds due to changing API.

[Alice Liu]

(In reply to comment #5)
> Sending initial request to the client seems quite surprising. What is the client going to do with this information? The request can result in a redirect, so a completely different resource will be used in the end.

I'm not sure I see why you consider the redirect issue to be a problem.  A client can definitely make use of initial request info. your comment seems to assume that the client will be using the resource as specified and that no updates to it will be made.  that's not necessarily the case.  The request info is merely being provided so that the client can act on the createNewPage message as it sees fit.  

> Also, it seems like this patch would break nightly builds due to changing API.

i did try to address the API change where I was able to find it, namely the mini browser implementations for mac, win and gtk.  Did i miss something, and could you be more specific about where to find what i missed? 

thanks

[Alexey Proskuryakov]

> I'm not sure I see why you consider the redirect issue to be a problem.  A client can definitely make use of initial request info.

Due to my weak imagination, I cannot fathom any valid use for this info in the client, which makes me wonder about it.

> Did i miss something, and could you be more specific about where to find what i missed? 

Quoting myself, "it seems like this patch would break nightly builds". Those need to work with Safari 5.1.

[Alice Liu]

Created attachment 105272 [details]
patch - keeps nightlies working

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/WebUIClient.cpp:48:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebKit2/UIProcess/WebUIClient.cpp:72:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebKit2/Shared/APIClientTraits.h:46:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 3 in 16 files


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

[Alice Liu]

I won't be uploading a new patch just to to fix the style.  I'll just say that that I have fixed the null-comparison style, but i'm not going to fix the brace style.

[Alice Liu]

Created attachment 105312 [details]
patch - with fix for minibrowser win

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/Shared/APIClientTraits.h:46:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 1 in 16 files


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

[Alice Liu]

http://trac.webkit.org/projects/webkit/changeset/93902