Bug 43198

Summary: Handle contents size change in WebKit2
Product: WebKit Reporter: Balazs Kelemen <kbalazs>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, andersca, commit-queue, kenneth, koivisto, luiz, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 44053    
Bug Blocks:    
Attachments:
Description Flags
proposed patch
none
rebased patch
none
style fixed rebased patch
none
Patch
none
reuploaded for new win ews result
none
build fixed patch none

Description Balazs Kelemen 2010-07-29 10:01:58 PDT
The web process and the UI client need to communicate about zooming and content size.
Comment 1 Kenneth Rohde Christiansen 2010-07-29 10:34:56 PDT
It will be nice to have this as two separate patches.
Comment 2 Balazs Kelemen 2010-08-02 04:27:21 PDT
Created attachment 63188 [details]
proposed patch
Comment 3 WebKit Review Bot 2010-08-02 04:29:10 PDT
Attachment 63188 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:128:  Extra space between WKPageContentsSizeChangedCallback and contentsSizeChanged  [whitespace/declaration] [3]
WebKit2/UIProcess/WebUIClient.h:32:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Kenneth Rohde Christiansen 2010-08-03 06:39:07 PDT
This looks OK to me, but it would be nice with Anders' and Sam's feedback. Like whether this really belongs to the ui client.
Comment 5 Sam Weinig 2010-08-03 08:52:01 PDT
What is the use case for communicating content size changes?
Comment 6 Kenneth Rohde Christiansen 2010-08-03 11:10:33 PDT
(In reply to comment #5)
> What is the use case for communicating content size changes?

We use that with tiling for instance and for creating our own custom scroll indicators (I guess similar to what the iphone does)
Comment 7 Balazs Kelemen 2010-08-06 09:06:55 PDT
Created attachment 63727 [details]
rebased patch
Comment 8 WebKit Review Bot 2010-08-06 09:08:13 PDT
Attachment 63727 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:128:  Extra space between WKPageContentsSizeChangedCallback and contentsSizeChanged  [whitespace/declaration] [3]
WebKit2/UIProcess/WebUIClient.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Balazs Kelemen 2010-08-06 09:15:27 PDT
Created attachment 63729 [details]
style fixed rebased patch

Fixed the "code inside a namespace" style violation. The other would be not consistent with it's environment with correct style so I do not changed it.
Comment 10 WebKit Review Bot 2010-08-06 09:17:35 PDT
Attachment 63729 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:128:  Extra space between WKPageContentsSizeChangedCallback and contentsSizeChanged  [whitespace/declaration] [3]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 WebKit Review Bot 2010-08-06 14:37:57 PDT
Attachment 63729 [details] did not build on win:
Build output: http://queues.webkit.org/results/3632493
Comment 12 Sam Weinig 2010-08-06 21:18:48 PDT
Comment on attachment 63729 [details]
style fixed rebased patch

> @@ -124,6 +125,7 @@ typedef WKStringRef (*WKPageRunJavaScriptPromptCallback)(WKPageRef page, WKStrin
>  struct WKPageUIClient {
>      int                                                                 version;
>      const void *                                                        clientInfo;
> +    WKPageContentsSizeChangedCallback                                   contentsSizeChanged;
>      WKPageCreateNewPageCallback                                         createNewPage;
>      WKPageShowPageCallback                                              showPage;
>      WKPageCloseCallback                                                 close;

Please put the new callback last.

Otherwise, this looks fine.
Comment 13 Balazs Kelemen 2010-08-07 14:35:02 PDT
Created attachment 63828 [details]
Patch
Comment 14 WebKit Review Bot 2010-08-07 14:36:34 PDT
Attachment 63828 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:134:  Extra space between WKPageContentsSizeChangedCallback and contentsSizeChanged  [whitespace/declaration] [3]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 WebKit Review Bot 2010-08-07 17:00:26 PDT
Attachment 63828 [details] did not build on win:
Build output: http://queues.webkit.org/results/3674050
Comment 16 Balazs Kelemen 2010-08-08 05:13:07 PDT
Created attachment 63841 [details]
reuploaded for new win ews result
Comment 17 WebKit Review Bot 2010-08-08 05:15:16 PDT
Attachment 63841 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:134:  Extra space between WKPageContentsSizeChangedCallback and contentsSizeChanged  [whitespace/declaration] [3]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 WebKit Review Bot 2010-08-08 06:46:14 PDT
Attachment 63841 [details] did not build on win:
Build output: http://queues.webkit.org/results/3595987
Comment 19 Andras Becsi 2010-08-09 06:31:19 PDT
(In reply to comment #18)
> Attachment 63841 [details] did not build on win:
> Build output: http://queues.webkit.org/results/3595987
The culprit is http://trac.webkit.org/changeset/64877/trunk/WebKit2/UIProcess/WebPageProxy.cpp arguments was changed to pointer type, and only the Mac and the Windows ports build WK2 by default now.
Comment 20 Balazs Kelemen 2010-08-09 07:12:51 PDT
Created attachment 63893 [details]
build fixed patch

Hopefully final. I assumed that the win ews gives incorrect result but I was wrong. However the bad thing about the win ews is that it does not show build output :(
Comment 21 WebKit Review Bot 2010-08-09 07:14:30 PDT
Attachment 63893 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:134:  Extra space between WKPageContentsSizeChangedCallback and contentsSizeChanged  [whitespace/declaration] [3]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 WebKit Commit Bot 2010-08-16 04:54:51 PDT
Comment on attachment 63893 [details]
build fixed patch

Rejecting patch 63893 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christiansen', u'--force']" exit_code: 1
Last 500 characters of output:
IProcess/WebUIClient.cpp
patching file WebKit2/UIProcess/WebUIClient.h
Hunk #1 succeeded at 30 with fuzz 2 (offset -3 lines).
Hunk #2 FAILED at 51.
1 out of 2 hunks FAILED -- saving rejects to file WebKit2/UIProcess/WebUIClient.h.rej
patching file WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp
patching file WebKitTools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebKitTools/MiniBrowser/mac/BrowserWindowController.m
patching file WebKitTools/MiniBrowser/win/BrowserView.cpp

Full output: http://queues.webkit.org/results/3740248
Comment 23 Andras Becsi 2010-08-16 05:52:42 PDT
Committed r65419: <http://trac.webkit.org/changeset/65419>
Comment 24 Andras Becsi 2010-08-16 05:53:49 PDT
Comment on attachment 63893 [details]
build fixed patch

Clearing flags.
Comment 25 Andras Becsi 2010-08-16 06:25:04 PDT
Had to roll out because it broke the Windows and Snow Leopard builds. Forward declaration of class String seems to be the culprit.
Comment 26 Andras Becsi 2010-08-16 06:40:18 PDT
Committed r65422: <http://trac.webkit.org/changeset/65422>