Bug 27376 - [WINCE] Add WinCE specific files for platform/network
Summary: [WINCE] Add WinCE specific files for platform/network
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Patrick R. Gansterer
URL:
Keywords:
Depends on:
Blocks: 23154
  Show dependency treegraph
 
Reported: 2009-07-17 10:02 PDT by Adam Treat
Modified: 2010-10-22 11:22 PDT (History)
7 users (show)

See Also:


Attachments
Add platform/network WinCE specific files (71.14 KB, patch)
2009-07-17 10:03 PDT, Adam Treat
no flags Details | Formatted Diff | Diff
fix some coding style conflicts (66.25 KB, patch)
2009-07-20 14:18 PDT, Yong Li
no flags Details | Formatted Diff | Diff
oops. forgot to add ChangeLog to it (67.61 KB, patch)
2009-07-20 14:19 PDT, Yong Li
eric: review-
Details | Formatted Diff | Diff
Broken out ResourceHandleWince into own patch (36.99 KB, patch)
2009-07-21 13:28 PDT, Adam Treat
abarth: review-
Details | Formatted Diff | Diff
The rest of WebCore/platform/network/wince (36.99 KB, patch)
2009-07-21 13:29 PDT, Adam Treat
no flags Details | Formatted Diff | Diff
The rest of WebCore/platform/network/wince (v2) (33.36 KB, patch)
2009-07-21 13:31 PDT, Adam Treat
abarth: review-
Details | Formatted Diff | Diff
Complete reimplementation of ResourceHandleWin (38.23 KB, patch)
2010-07-23 09:52 PDT, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff
Complete reimplementation of ResourceHandleWin (38.25 KB, patch)
2010-07-23 10:09 PDT, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff
Patch (10.85 KB, patch)
2010-10-20 05:06 PDT, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff
Patch (10.85 KB, patch)
2010-10-20 05:15 PDT, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Treat 2009-07-17 10:02:43 PDT
The following patch is the WinCE implementation of platform/network.  The files have been purified with cpplint.
Comment 1 Adam Treat 2009-07-17 10:03:51 PDT
Created attachment 32951 [details]
Add platform/network WinCE specific files
Comment 2 Kwang Yul Seo 2009-07-17 10:27:41 PDT
There are many WinCE devices which lack Connection Manager. So I think we need a guard around Connection Manager code.
Comment 3 Yong Li 2009-07-20 14:18:12 PDT
Created attachment 33104 [details]
fix some coding style conflicts

fix some coding style conflicts
Comment 4 Yong Li 2009-07-20 14:19:59 PDT
Created attachment 33105 [details]
oops. forgot to add ChangeLog to it

add ChangeLog to it
Comment 5 Eric Seidel (no email) 2009-07-20 15:19:21 PDT
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.
Comment 6 Eric Seidel (no email) 2009-07-20 15:20:03 PDT
Comment on attachment 32951 [details]
Add platform/network WinCE specific files

Seems this is made obsolete by a later patch.
Comment 7 George Staikos 2009-07-20 15:51:57 PDT
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.
Comment 8 Adam Treat 2009-07-21 13:28:22 PDT
Created attachment 33204 [details]
Broken out ResourceHandleWince into own patch

Checked with latest cpplint and passes.
Comment 9 Adam Treat 2009-07-21 13:29:19 PDT
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.
Comment 10 Adam Treat 2009-07-21 13:31:23 PDT
Created attachment 33206 [details]
The rest of WebCore/platform/network/wince (v2)
Comment 11 Adam Barth 2009-09-01 17:42:36 PDT
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.
Comment 12 Adam Barth 2009-09-01 17:53:05 PDT
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.
Comment 13 Yong Li 2009-09-01 18:02:43 PDT
(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
Comment 14 Patrick R. Gansterer 2010-07-23 09:52:01 PDT
Created attachment 62436 [details]
Complete reimplementation of ResourceHandleWin
Comment 15 WebKit Review Bot 2010-07-23 09:56:58 PDT
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.
Comment 16 Patrick R. Gansterer 2010-07-23 10:09:31 PDT
Created attachment 62440 [details]
Complete reimplementation of ResourceHandleWin
Comment 17 WebKit Review Bot 2010-07-23 10:12:29 PDT
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.
Comment 18 Patrick R. Gansterer 2010-08-09 03:27:07 PDT
Comment on attachment 62440 [details]
Complete reimplementation of ResourceHandleWin

I opended master bug 43712 for this.
Comment 19 Patrick R. Gansterer 2010-10-20 05:06:35 PDT
Created attachment 71273 [details]
Patch
Comment 20 WebKit Review Bot 2010-10-20 05:10:32 PDT
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.
Comment 21 Patrick R. Gansterer 2010-10-20 05:15:28 PDT
Created attachment 71275 [details]
Patch
Comment 22 WebKit Commit Bot 2010-10-22 11:22:48 PDT
Comment on attachment 71275 [details]
Patch

Clearing flags on attachment: 71275

Committed r70319: <http://trac.webkit.org/changeset/70319>
Comment 23 WebKit Commit Bot 2010-10-22 11:22:55 PDT
All reviewed patches have been landed.  Closing bug.