Bug 50217 – [WINCE] add main() function

• Component: Platform
• Status: RESOLVED FIXED

[Patrick R. Gansterer]

see patch

[Patrick R. Gansterer]

Created attachment 75118 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/wince/main.cpp']" exit_code: 1
WebKit/wince/main.cpp:26:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit/wince/main.cpp:117:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 2 in 1 files


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

[Adam Roben (:aroben)]

Comment on attachment 75118 [details]
Patch

It doesn't seem appropriate to have a full browser in WebKit itself. Perhaps you should move this code to a new WebKitTools/WinCELauncher directory.

[Patrick R. Gansterer]

Created attachment 75152 [details]
Patch

(In reply to comment #3)
> (From update of attachment 75118 [details])
> It doesn't seem appropriate to have a full browser in WebKit itself. Perhaps you should move this code to a new WebKitTools/WinCELauncher directory.
Uuups, now in the corret directory and with ChangeLog.

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKitTools/ChangeLog', u'WebKitTools/WinCELauncher/main.cpp']" exit_code: 1
WebKitTools/WinCELauncher/main.cpp:26:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/WinCELauncher/main.cpp:117:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 2 in 2 files


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

[Adam Roben (:aroben)]

Comment on attachment 75152 [details]
Patch

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

> WebKitTools/WinCELauncher/main.cpp:103
> +    if (fullscreen)
> +        SetWindowPos(hMainWindow, HWND_TOPMOST, 0, 0, GetSystemMetrics(SM_CXSCREEN),
> +            GetSystemMetrics(SM_CYSCREEN), SWP_SHOWWINDOW);

This should be enclosed in braces, since it is more than one line (even though it's only a single statement).

> WebKitTools/WinCELauncher/main.cpp:123
> +    delete webView;

Could you use an OwnPtr here?

[Adam Roben (:aroben)]

(In reply to comment #5)
> Attachment 75152 [details] did not pass style-queue:
> 
> Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKitTools/ChangeLog', u'WebKitTools/WinCELauncher/main.cpp']" exit_code: 1
> WebKitTools/WinCELauncher/main.cpp:26:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
> WebKitTools/WinCELauncher/main.cpp:117:  One line control clauses should not use braces.  [whitespace/braces] [4]
> Total errors found: 2 in 2 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

You should change WebKitTools/Scripts/webkitpy/style/checkers.py to exclude the build/include_order rule when checking the WinCELauncher directory.

[Patrick R. Gansterer]

(In reply to comment #6)
> (From update of attachment 75152 [details])
> > WebKitTools/WinCELauncher/main.cpp:123
> > +    delete webView;
> 
> Could you use an OwnPtr here?
I'm not sure if it's cool to use "private" WebKit classes outside of it.
WinLauncher.cpp does a "gWebView->Release()" too, but I can change it.


(In reply to comment #7)
> You should change WebKitTools/Scripts/webkitpy/style/checkers.py to exclude the build/include_order rule when checking the WinCELauncher directory.
Are there any examples? I didn't find the place :-/.

[Adam Roben (:aroben)]

(In reply to comment #8)
> (In reply to comment #6)
> > (From update of attachment 75152 [details] [details])
> > > WebKitTools/WinCELauncher/main.cpp:123
> > > +    delete webView;
> > 
> > Could you use an OwnPtr here?
> I'm not sure if it's cool to use "private" WebKit classes outside of it.
> WinLauncher.cpp does a "gWebView->Release()" too, but I can change it.

We have other projects that use WTF headers (e.g., DumpRenderTree). As long as you're only using the header and not actually linking against WTF it seems fine.

> (In reply to comment #7)
> > You should change WebKitTools/Scripts/webkitpy/style/checkers.py to exclude the build/include_order rule when checking the WinCELauncher directory.
> Are there any examples? I didn't find the place :-/.

You'd add to the _PATH_RULES_SPECIFIER list <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/style/checker.py#L114>. Probably something like this:

([# WinCELauncher has no config.h.
  "WebKitTools/WinCELauncher"],
 ["-build/include_order"]),

[Patrick R. Gansterer]

Comment on attachment 75152 [details]
Patch

Clearing flags on attachment: 75152

Manually committed r73026: <http://trac.webkit.org/changeset/73026>

[Patrick R. Gansterer]

All reviewed patches have been landed.  Closing bug.