Bug 27376 – [WINCE] Add WinCE specific files for platform/network

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Adam Treat]

The following patch is the WinCE implementation of platform/network.  The files have been purified with cpplint.

[Adam Treat]

Created attachment 32951 [details]
Add platform/network WinCE specific files

[Kwang Yul Seo]

There are many WinCE devices which lack Connection Manager. So I think we need a guard around Connection Manager code.

[Yong Li]

Created attachment 33104 [details]
fix some coding style conflicts

fix some coding style conflicts

[Yong Li]

Created attachment 33105 [details]
oops. forgot to add ChangeLog to it

add ChangeLog to it

[Eric Seidel (no email)]

Comment on attachment 33105 [details]
oops. forgot to add ChangeLog to it

Too large to review.  Please explain the 3rd party code you're adding and why you know it to be a compatible license to WebKit.

[Eric Seidel (no email)]

Comment on attachment 32951 [details]
Add platform/network WinCE specific files

Seems this is made obsolete by a later patch.

[George Staikos]

From the RFC: ftp://ftp.rfc-editor.org/in-notes/rfc3492.txt

B. Disclaimer and license

   Regarding this entire document or any portion of it (including the
   pseudocode and C code), the author makes no guarantees and is not
   responsible for any damage resulting from its use.  The author grants
   irrevocable permission to anyone to use, modify, and distribute it in
   any way that does not diminish the rights of anyone else to use,
   modify, and distribute it, provided that redistributed derivative
   works do not contain misleading author or version information.
   Derivative works need not be licensed under similar terms.

[Adam Treat]

Created attachment 33204 [details]
Broken out ResourceHandleWince into own patch

Checked with latest cpplint and passes.

[Adam Treat]

Created attachment 33205 [details]
The rest of WebCore/platform/network/wince

Also checked with latest cpplint and passes.  The licensing info of third-party code is also clearly marked.

[Adam Treat]

Created attachment 33206 [details]
The rest of WebCore/platform/network/wince (v2)

[Adam Barth]

Comment on attachment 33204 [details]
Broken out ResourceHandleWince into own patch

One class per file, please.

+  static ResourceJobManager jm;

You should use the DEFINE_STATIC_LOCAL macro.

+ ConnectionManager() //, LPCWSTR userAgentTex)

Please don't included commented out code.

+ delete m_connMgr;
delete[] m_formDataString;

Why not OwnPtr and OwnArray?

+ ResourceHandle::setResponse

The fake URL parsing in this function is not cool.  Also, this primitive mime sniffing is very insecure.

I stopped reviewing this patch at this point.

[Adam Barth]

Comment on attachment 33206 [details]
The rest of WebCore/platform/network/wince (v2)

+ #ifndef AuthenticationChallenge_h

Missing space before this line.

+ #if PLATFORM(TORCHMOBILE)

Is TORCHMOBILE really a platform?  I would have expected WINCE here.

+ static ThreadData g_threadData = {0};

No real point in declaring this static inside the anonymous namespace...

+ } // namespace WebCore

Missing space before this line.

+ punycode.c

It's unclear to me whether this license is compatible with the BSD license because the BSD license does diminish someone's ability to distribute: they must include the license block.  I can't R+ this patch without input from a lawyer.

Also, this file isn't even close to WebKit style.

[Yong Li]

(In reply to comment #12)
> (From update of attachment 33206 [details])
> + #ifndef AuthenticationChallenge_h
> 
> Missing space before this line.
> 
> + #if PLATFORM(TORCHMOBILE)
> 
> Is TORCHMOBILE really a platform?  I would have expected WINCE here.
> 
The file is for WINCE only. I will change it to #if ENABLE(SINGLE_THREADED)

> + static ThreadData g_threadData = {0};
> 
> No real point in declaring this static inside the anonymous namespace...

In case ThreadData conflicts with WTF::ThreadData.

> 
> + } // namespace WebCore
> 
> Missing space before this line.
> 
> + punycode.c
> 
> It's unclear to me whether this license is compatible with the BSD license
> because the BSD license does diminish someone's ability to distribute: they
> must include the license block.  I can't R+ this patch without input from a
> lawyer.
> 
> Also, this file isn't even close to WebKit style.

k. will move the file out of there

[Patrick R. Gansterer]

Created attachment 62436 [details]
Complete reimplementation of ResourceHandleWin

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/network/win/ResourceHandleWin.cpp:381:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/network/win/ResourceHandleWin.cpp:387:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/network/win/ResourceHandleWin.cpp:394:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/platform/network/win/ResourceHandleWin.cpp:416:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/network/win/ResourceHandleWin.cpp:442:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 5 in 4 files


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

[Patrick R. Gansterer]

Created attachment 62440 [details]
Complete reimplementation of ResourceHandleWin

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/network/win/ResourceHandleWin.cpp:394:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 1 in 4 files


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

[Patrick R. Gansterer]

Comment on attachment 62440 [details]
Complete reimplementation of ResourceHandleWin

I opended master bug 43712 for this.

[Patrick R. Gansterer]

Created attachment 71273 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/network/win/ResourceError.h:33:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 1 in 5 files


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

[Patrick R. Gansterer]

Created attachment 71275 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 71275 [details]
Patch

Clearing flags on attachment: 71275

Committed r70319: <http://trac.webkit.org/changeset/70319>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.