Bug 47386 - Need implementation of ChromeClient windowRect related functions.
Summary: Need implementation of ChromeClient windowRect related functions.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-07 17:43 PDT by Sam Weinig
Modified: 2010-10-10 18:00 PDT (History)
2 users (show)

See Also:


Attachments
Patch (32.54 KB, patch)
2010-10-08 21:05 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Updated. (35.16 KB, patch)
2010-10-08 21:29 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (35.19 KB, patch)
2010-10-08 21:46 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (35.73 KB, patch)
2010-10-09 13:18 PDT, Sam Weinig
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2010-10-07 17:43:12 PDT
Need implementation of ChromeClient windowRect related functions.
Comment 1 Sam Weinig 2010-10-08 21:05:55 PDT
Created attachment 70336 [details]
Patch
Comment 2 WebKit Review Bot 2010-10-08 21:13:34 PDT
Attachment 70336 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/WebUIClient.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
WebKit2/Shared/API/c/WKGeometry.h:49:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit2/Shared/API/c/WKGeometry.h:49:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebKit2/UIProcess/API/C/WKPage.h:158:  Extra space between WKPageGetWindowRectCallback and getWindowRect  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:159:  Extra space between WKPageSetWindowRectCallback and setWindowRect  [whitespace/declaration] [3]
WebKit2/UIProcess/win/WebView.cpp:37:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 6 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Early Warning System Bot 2010-10-08 21:15:00 PDT
Attachment 70336 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4350004
Comment 4 Sam Weinig 2010-10-08 21:29:34 PDT
Created attachment 70338 [details]
Updated.
Comment 5 WebKit Review Bot 2010-10-08 21:31:22 PDT
Attachment 70338 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/WebUIClient.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
WebKit2/Shared/API/c/WKGeometry.h:49:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit2/Shared/API/c/WKGeometry.h:49:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebKit2/UIProcess/API/C/WKPage.h:158:  Extra space between WKPageGetWindowRectCallback and getWindowRect  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:159:  Extra space between WKPageSetWindowRectCallback and setWindowRect  [whitespace/declaration] [3]
WebKitTools/WebKitTestRunner/win/PlatformWebViewWin.cpp:1:  Missing spaces around /  [whitespace/operators] [3]
WebKit2/UIProcess/win/WebView.cpp:37:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 7 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Early Warning System Bot 2010-10-08 21:38:13 PDT
Attachment 70338 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4285004
Comment 7 Sam Weinig 2010-10-08 21:46:58 PDT
Created attachment 70340 [details]
Patch
Comment 8 WebKit Review Bot 2010-10-08 21:49:19 PDT
Attachment 70340 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/Shared/API/c/WKGeometry.h:49:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit2/Shared/API/c/WKGeometry.h:49:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebKit2/UIProcess/API/C/WKPage.h:158:  Extra space between WKPageGetWindowRectCallback and getWindowRect  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:159:  Extra space between WKPageSetWindowRectCallback and setWindowRect  [whitespace/declaration] [3]
WebKitTools/WebKitTestRunner/win/PlatformWebViewWin.cpp:1:  Missing spaces around /  [whitespace/operators] [3]
Total errors found: 5 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 mitz 2010-10-09 12:54:50 PDT
Comment on attachment 70336 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=70336&action=review

I’d prefer “window frame” to “window rect”. The latter doesn’t mean much.

> WebKit2/Shared/API/c/WKGeometry.h:49
> +  	WKSize size;

Not enough indentation

> WebKit2/UIProcess/PageClient.h:57
> +    virtual WebCore::FloatRect transformRectToDeviceSpace(const WebCore::FloatRect&) = 0;
> +    virtual WebCore::FloatRect transformRectToUserSpace(const WebCore::FloatRect&) = 0;

Is “rect” necessary in these methods’ names?

> WebKit2/UIProcess/WebPageProxy.cpp:54
> +#include <WebCore/FloatRect.h>
>  
>  #include "WKContextPrivate.h"

Not sure why the #includes are separated like this.

> WebKit2/UIProcess/WebPageProxy.h:283
> +    void windowRect(WebCore::FloatRect&);

getWindowRect would be a better name.

> WebKit2/UIProcess/WebUIClient.cpp:32
>  #include <WebCore/IntSize.h>
> +#include <WebCore/FloatRect.h>

F < I

> WebKit2/UIProcess/win/WebView.cpp:37
>  #include <WebCore/IntRect.h>
> +#include <WebCore/FloatRect.h>

F < I
Comment 10 Sam Weinig 2010-10-09 13:18:45 PDT
Created attachment 70367 [details]
Patch
Comment 11 WebKit Review Bot 2010-10-09 13:20:21 PDT
Attachment 70367 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:158:  Extra space between WKPageGetWindowFrameCallback and getWindowFrame  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:159:  Extra space between WKPageSetWindowFrameCallback and setWindowFrame  [whitespace/declaration] [3]
Total errors found: 2 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Early Warning System Bot 2010-10-09 13:30:06 PDT
Attachment 70367 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4276008
Comment 13 Sam Weinig 2010-10-09 13:32:55 PDT
Landed in 69457.
Comment 14 Simon Fraser (smfr) 2010-10-10 18:00:12 PDT
Comment on attachment 70367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=70367&action=review

> WebKit2/UIProcess/PageClient.h:57
> +    virtual WebCore::FloatRect transformToDeviceSpace(const WebCore::FloatRect&) = 0;
> +    virtual WebCore::FloatRect transformToUserSpace(const WebCore::FloatRect&) = 0;

I'd prefer these use the term 'convert', rather than 'transform', since we use that elsewhere for similar things.