Bug 43198 – Handle contents size change in WebKit2

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Balazs Kelemen]

The web process and the UI client need to communicate about zooming and content size.

[Kenneth Rohde Christiansen]

It will be nice to have this as two separate patches.

[Balazs Kelemen]

Created attachment 63188 [details]
proposed patch

[WebKit Review Bot]

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.

[Kenneth Rohde Christiansen]

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.

[Sam Weinig]

What is the use case for communicating content size changes?

[Kenneth Rohde Christiansen]

(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)

[Balazs Kelemen]

Created attachment 63727 [details]
rebased patch

[WebKit Review Bot]

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.

[Balazs Kelemen]

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.

[WebKit Review Bot]

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.

[WebKit Review Bot]

Attachment 63729 [details] did not build on win:
Build output: http://queues.webkit.org/results/3632493

[Sam Weinig]

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.

[Balazs Kelemen]

Created attachment 63828 [details]
Patch

[WebKit Review Bot]

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.

[WebKit Review Bot]

Attachment 63828 [details] did not build on win:
Build output: http://queues.webkit.org/results/3674050

[Balazs Kelemen]

Created attachment 63841 [details]
reuploaded for new win ews result

[WebKit Review Bot]

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.

[WebKit Review Bot]

Attachment 63841 [details] did not build on win:
Build output: http://queues.webkit.org/results/3595987

[Andras Becsi]

(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.

[Balazs Kelemen]

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 :(

[WebKit Review Bot]

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.

[WebKit Commit Bot]

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

[Andras Becsi]

Committed r65419: <http://trac.webkit.org/changeset/65419>

[Andras Becsi]

Comment on attachment 63893 [details]
build fixed patch

Clearing flags.

[Andras Becsi]

Had to roll out because it broke the Windows and Snow Leopard builds. Forward declaration of class String seems to be the culprit.

[Andras Becsi]

Committed r65422: <http://trac.webkit.org/changeset/65422>