Created attachment 52898 [details] Patch Start the dump of WebKit2.
Is there a design doc for the structure of WebKit2? It's unclear to me how this relates to other ports.
Attachment 52898 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 Last 3072 characters of output: the same line [whitespace/newline] [4] WebKit2/Platform/CoreIPC/Attachment.h:58: More than one command on the same line [whitespace/newline] [4] WebKit2/Platform/CoreIPC/Attachment.h:61: More than one command on the same line [whitespace/newline] [4] WebKit2/Platform/CoreIPC/Attachment.h:62: More than one command on the same line [whitespace/newline] [4] WebKit2/Platform/CoreIPC/Attachment.h:63: More than one command on the same line [whitespace/newline] [4] WebKit2/Platform/CoreIPC/Attachment.h:64: More than one command on the same line [whitespace/newline] [4] WebKit2/Platform/win/RunLoopWin.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/Platform/win/RunLoopWin.cpp:54: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] WebKit2/Platform/win/WorkQueueWin.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/Platform/CoreIPC/Attachment.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/Platform/CoreIPC/Attachment.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WebKit2/Platform/CoreIPC/Connection.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/Platform/CoreIPC/Connection.cpp:38: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit2/Platform/CoreIPC/Connection.cpp:144: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebKit2/Platform/WorkQueue.h:37: "wtf/HashMap.h" already included at WebKit2/Platform/WorkQueue.h:31 [build/include] [4] WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/Platform/CoreIPC/ArgumentDecoder.h:32: Alphabetical sorting problem. [build/include_order] [4] WebKit2/Platform/CoreIPC/ArgumentDecoder.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp:96: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp:149: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 42 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #1) > Is there a design doc for the structure of WebKit2? It's unclear to me how > this relates to other ports. It's a new take on WebKit port design, so WebKit2 kinda seems reasonable.
I know. I was just giving Sam a hard time. :) Glad we have a meeting coming up. Should be fun to talk about.
(In reply to comment #4) > I know. I was just giving Sam a hard time. :) > > Glad we have a meeting coming up. Should be fun to talk about. I also think we should land in buildable chunks! :P
More seriously, we have some experience with this stuff. Are you interested in a real review, or is the plan to land the code in the tree, presuming it was reviewed internally by WebKit Reviewers (tm)?
(In reply to comment #5) > (In reply to comment #4) > > I know. I was just giving Sam a hard time. :) > > > > Glad we have a meeting coming up. Should be fun to talk about. > > I also think we should land in buildable chunks! :P This chunk is buildable! :)
(In reply to comment #6) > More seriously, we have some experience with this stuff. Are you interested in > a real review, or is the plan to land the code in the tree, presuming it was > reviewed internally by WebKit Reviewers (tm)? Any feedback would be appreciated, but since we have the ok to land this (and have done internal review) we would like to land it and work iteratively in the Open Source from now on. Side note: this shouldn't break anyone since no one builds it.
Okiedokes. I can file bugs about anything I notice as the patches go by. You'd make me a happy man if you fixed the stylebot nits as you go, especially "Code inside a namespace should not be indented" because that one is a PITA to clean up incrementally.
Comment on attachment 52898 [details] Patch > + std::queue<WorkItem*> workItemQueue; There's already using namespace std, so just queue. > + std::auto_ptr<WorkItem> item(workItemQueue.front()); And just auto_ptr. > +void RunLoop::scheduleWork(std::auto_ptr<WorkItem> item) And here. I'll review more later.
Comment on attachment 52898 [details] Patch r=me. (There are some minor style issues which we plan to fix once we land).
(In reply to comment #11) > (From update of attachment 52898 [details]) > r=me. > > (There are some minor style issues which we plan to fix once we land). This is a big no no for every other port introduction. It does not look good that you are landing this with style problems and without any community discussion before you've announced it today.
Landed in r57307.