Bug 100201

Summary: Compiling WTF outside of WebKit failed on Windows
Product: WebKit Reporter: mizi.bug <mizi.bug>
Component: PlatformAssignee: Patrick R. Gansterer <paroga>
Status: RESOLVED FIXED    
Severity: Enhancement CC: benjamin, bfulgham, ojan.autocc, paroga, roger_fong, webkit.review.bot
Priority: P5    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
patch for this bug
mizi.bug: review-
Patch
none
Patch
paroga: review-, paroga: commit-queue-
Patch none

Description mizi.bug 2012-10-23 22:45:36 PDT
1>  Source\WTF\wtf\win\MainThreadWin.cpp
1>..\wtf\win\MainThreadWin.cpp(69): error C2440: “=”: unable to convert from “const LPCWSTR”to“LPCSTR”
1>..\wtf\win\MainThreadWin.cpp(78): error C2664: “CreateWindowExA”: unable to convert from “const LPCWSTR” to “LPCSTR”
1>..\wtf\win\MainThreadWin.cpp(79): error C2664: “RegisterWindowMessageA”: unable to convert from “const wchar_t [33]”to“LPCSTR”
Comment 1 mizi.bug 2012-10-23 22:53:45 PDT
Created attachment 170319 [details]
patch for this bug
Comment 2 Alexey Proskuryakov 2012-10-24 10:42:04 PDT
Would you be willing to submit a patch for inclusion in WebKit? Please follow the steps in <http://www.webkit.org/coding/contributing.html> if so - in particular, every patch needs a ChangeLog, and it should be marked for review to be in review queue.
Comment 3 mizi.bug 2012-10-25 09:18:35 PDT
(In reply to comment #2)
> Would you be willing to submit a patch for inclusion in WebKit? Please follow the steps in <http://www.webkit.org/coding/contributing.html> if so - in particular, every patch needs a ChangeLog, and it should be marked for review to be in review queue.

thx for reply. I should follow the contributing guide, but my network here is awful, so I can't check out the code with SVN or GIT, and the webkit-patch doesn't work, so I thought this may be an easy way.
I will try again, anymore advises?
Comment 4 mizi.bug 2012-10-26 08:46:55 PDT
Created attachment 170936 [details]
Patch
Comment 5 WebKit Review Bot 2012-11-03 22:57:20 PDT
Comment on attachment 170936 [details]
Patch

Rejecting attachment 170936 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
code: 1 cwd: /mnt/git/webkit-commit-queue

Parsed 2 diffs from patch file(s).
patching file source/wtf/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file source/wtf/wtf/win/MainThreadWin.cpp
Hunk #1 FAILED at 40.
Hunk #2 FAILED at 76.
2 out of 2 hunks FAILED -- saving rejects to file source/wtf/wtf/win/MainThreadWin.cpp.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Ryosuke Ni..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/14719409
Comment 6 Darin Adler 2012-11-04 08:54:44 PST
Comment on attachment 170936 [details]
Patch

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

> source/wtf/wtf/win/MainThreadWin.cpp:44
> +const LPTSTR kThreadingWindowMessge = TEXT("com.apple.WebKit.MainThreadFired");

Messge here is a typo, with a missing "a".
Comment 7 mizi.bug 2012-11-10 05:01:01 PST
Created attachment 173444 [details]
Patch
Comment 8 mizi.bug 2012-11-10 05:12:07 PST
(In reply to comment #6)
> (From update of attachment 170936 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=170936&action=review
> 
> > source/wtf/wtf/win/MainThreadWin.cpp:44
> > +const LPTSTR kThreadingWindowMessge = TEXT("com.apple.WebKit.MainThreadFired");
> 
> Messge here is a typo, with a missing "a".

You mean "RegisterWindowMessage" should change to "RegisterWindowMessageA"?
It seems ok with VC 2010 in my machine.
Comment 9 Brent Fulgham 2012-11-13 10:54:45 PST
Comment on attachment 173444 [details]
Patch

Why do you need to provide TEXT macros here.  Are you trying to build in non-UNICODE?
Comment 10 mizi.bug 2012-11-14 10:32:36 PST
(In reply to comment #9)
> (From update of attachment 173444 [details])
> Why do you need to provide TEXT macros here.  Are you trying to build in non-UNICODE?

Since I compiled the WTF seperately and it failed. And why not use "TEXT" here? I think it is more reasonable and error-proofing.
Comment 11 Brent Fulgham 2012-11-15 18:05:19 PST
(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 173444 [details] [details])
> > Why do you need to provide TEXT macros here.  Are you trying to build in non-UNICODE?
> 
> Since I compiled the WTF seperately and it failed. And why not use "TEXT" here? I think it is more reasonable and error-proofing.

It seems like you built WTF in ANSI mode, which I do not think is supported.  If you built with -DUNICODE, none of these changes should have been necessary.
Comment 12 mizi.bug 2012-11-15 22:56:58 PST
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (From update of attachment 173444 [details] [details] [details])
> > > Why do you need to provide TEXT macros here.  Are you trying to build in non-UNICODE?
> > 
> > Since I compiled the WTF seperately and it failed. And why not use "TEXT" here? I think it is more reasonable and error-proofing.
> 
> It seems like you built WTF in ANSI mode, which I do not think is supported.  If you built with -DUNICODE, none of these changes should have been necessary.

I knew everything will be fine if I used the UNICODE mode. But why not use more compatible code if we can? If we are compelled to use the UNICODE mode, we can still put some warning or macro somewhere else.
And with my experience I think WTF can work well in ANSI mode, even it cannot, the reason is definitely not because of the code here. So I think that should be another topic.
Comment 13 Patrick R. Gansterer 2012-12-08 16:12:50 PST
Comment on attachment 173444 [details]
Patch

You need to define UNICODE in your buildsystem. WebKit does not support compiling without UNICODE.
Comment 14 Patrick R. Gansterer 2012-12-08 16:17:35 PST
(In reply to comment #13)
> (From update of attachment 173444 [details])
> You need to define UNICODE in your buildsystem. WebKit does not support compiling without UNICODE.

If you want to support non-UNICODE builds outside of WebKit you should change the code to use the Unicode function of the Windows API directly (e.g. RegisterClassW instead of RegisterClass).
Comment 15 Patrick R. Gansterer 2012-12-08 16:53:24 PST
Created attachment 178387 [details]
Patch
Comment 16 WebKit Review Bot 2012-12-08 18:31:37 PST
Comment on attachment 178387 [details]
Patch

Clearing flags on attachment: 178387

Committed r137054: <http://trac.webkit.org/changeset/137054>
Comment 17 WebKit Review Bot 2012-12-08 18:31:41 PST
All reviewed patches have been landed.  Closing bug.