Bug 45716 – [BREWMP] Port unicode

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Kwang Yul Seo]

Port unicode.

[Kwang Yul Seo]

Created attachment 67485 [details]
Patch

Brew MP port uses only the subset of ICU library to reduce the binary size. Follow the WinCE's implementation.

[Kwang Yul Seo]

Created attachment 67486 [details]
Patch

Oops. Uploaded the wrong patch.

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/wtf/unicode/brew/UnicodeBrew.h:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 5 files


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

[Kwang Yul Seo]

Created attachment 67490 [details]
Patch

Fix style errors.

[Kwang Yul Seo]

CC'ing Eric Seidel who reviewed bug 27305. This patch follows the WinCE approach.

[Kent Tamura]

Comment on attachment 67490 [details]
Patch

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

r- because of style errors.

> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:84
> +    int remainingCharacters = 0;

remainingCharacters should be declared just before it is used.

> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:85
> +    if (sourceLength <= resultLength)

You should enclose this block with {} because the block contains two physical lines.

> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:88
> +    else

ditto.

> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:101
> +int toUpper(UChar* result, int resultLength, const UChar* source, int sourceLength, bool* isError)

Same comments as toLower().

[Kwang Yul Seo]

Created attachment 70689 [details]
Patch

Fix style errors.

[Kwang Yul Seo]

Created attachment 70690 [details]
Patch

Oops. Remove duplicated ChangeLog entry.

[Kent Tamura]

Comment on attachment 70690 [details]
Patch

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

> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:94
> +        remainingCharacters += sourceEnd - sourceIterator;

+= looks curious here.
  int remainingCharacters = sourceIterator < sourceEnd ? sourceEnd - sourceIterator : 0;
would be better.

[Kwang Yul Seo]

Manually committed r69723: http://trac.webkit.org/changeset/69723

[Kwang Yul Seo]

All reviewed patches have been landed.  Closing bug.

[Patrick R. Gansterer]

Do you think we can share the implementation between BREWMP and WinCE?
Do you have an example of "UnicodeFromICU.h"? Is it a "copy" of ce_unicode.h|cpp?
(I'm not very happy with the current WinCE implementation, maybe we can do it better)

IMHO we can share the implementaions as a "UnicodeMinimalCopyOfICU". I don't think that there's something BREW specific in this files.

PS: Use "svn cp" if you copy UnicodeBrew from UnicodeWinCE. This helps when looking at the history.

[Kwang Yul Seo]

(In reply to comment #12)
> Do you think we can share the implementation between BREWMP and WinCE?

Yes, I just copied the implementation of WinCE only with minor style changes.

> Do you have an example of "UnicodeFromICU.h"? Is it a "copy" of ce_unicode.h|cpp?
> (I'm not very happy with the current WinCE implementation, maybe we can do it better)

Yes, it is just a copy of ce_unicode.h|cpp with slight changes.

> 
> IMHO we can share the implementaions as a "UnicodeMinimalCopyOfICU". I don't think that there's something BREW specific in this files.
> 

I agree. There is no Brew MP specific code in these files.

> PS: Use "svn cp" if you copy UnicodeBrew from UnicodeWinCE. This helps when looking at the history.

Thanks for the tip.

[Patrick R. Gansterer]

Created bug 47977 for merging the unicode implementations.