Bug 66823 - Add the request info to the WebPageProxy::CreateNewPage message
Summary: Add the request info to the WebPageProxy::CreateNewPage message
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alice Liu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-23 16:58 PDT by Alice Liu
Modified: 2011-08-26 12:36 PDT (History)
3 users (show)

See Also:


Attachments
patch (12.34 KB, patch)
2011-08-23 17:12 PDT, Alice Liu
no flags Details | Formatted Diff | Diff
patch w/ fixed style (12.33 KB, patch)
2011-08-23 17:25 PDT, Alice Liu
no flags Details | Formatted Diff | Diff
patch with fixes for MiniBrowser on gtk and windows (13.97 KB, patch)
2011-08-23 17:40 PDT, Alice Liu
no flags Details | Formatted Diff | Diff
patch - keeps nightlies working (20.79 KB, patch)
2011-08-25 16:18 PDT, Alice Liu
no flags Details | Formatted Diff | Diff
patch - with fix for minibrowser win (21.57 KB, patch)
2011-08-25 22:21 PDT, Alice Liu
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alice Liu 2011-08-23 16:58:51 PDT
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.
Comment 1 Alice Liu 2011-08-23 17:12:09 PDT
Created attachment 104937 [details]
patch
Comment 2 WebKit Review Bot 2011-08-23 17:14:35 PDT
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.
Comment 3 Alice Liu 2011-08-23 17:25:05 PDT
Created attachment 104942 [details]
patch w/ fixed style
Comment 4 Alice Liu 2011-08-23 17:40:14 PDT
Created attachment 104946 [details]
patch with fixes for MiniBrowser on gtk and windows
Comment 5 Alexey Proskuryakov 2011-08-24 10:46:15 PDT
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.
Comment 6 Alice Liu 2011-08-24 11:03:38 PDT
(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
Comment 7 Alexey Proskuryakov 2011-08-24 11:24:17 PDT
> 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.
Comment 8 Alice Liu 2011-08-25 16:18:58 PDT
Created attachment 105272 [details]
patch - keeps nightlies working
Comment 9 WebKit Review Bot 2011-08-25 16:21:22 PDT
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.
Comment 10 Alice Liu 2011-08-25 16:31:34 PDT
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.
Comment 11 Alice Liu 2011-08-25 22:21:41 PDT
Created attachment 105312 [details]
patch - with fix for minibrowser win
Comment 12 WebKit Review Bot 2011-08-25 22:25:48 PDT
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.