Bug 49897 – Refactoring of the custom allocation framework

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

[Zoltan Horvath]

Inheriting from FastAllocBase can result in objects getting larger (bug #33896, bug #46589). Change Noncopyable and FastAllocBase inheriting to their macro implementation.

[Zoltan Horvath]

Created attachment 74534 [details]
proposed patch

All in one patch. A bit large.

[Zoltan Horvath]

Comment on attachment 74534 [details]
proposed patch

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

> JavaScriptCore/runtime/DateInstanceCache.h:40
> +

I'll remove this.

> WebKitTools/DumpRenderTree/ForwardingHeaders/wtf/Noncopyable.h:-1
> -#include <JavaScriptCore/Noncopyable.h>

prepare-ChangeLog didn't generate changelog for WebKitTools. I'll make up.

[Zoltan Horvath]

I won't update the patch until Darin's comment.

[Csaba Osztrogonác]

(In reply to comment #3)
> I won't update the patch until Darin's comment.

Only 3 files should be updated to make this patch appliable:
JavaScriptCore/bytecode/CodeBlock.h
WebCore/bindings/js/JSSVGContextCache.h
WebCore/svg/DeprecatedSVGAnimatedPropertyTraits.h

We would like to know what EWS bots say. ;)

[Zoltan Horvath]

We would not want to know yet.

[Darin Adler]

(In reply to comment #3)
> I won't update the patch until Darin's comment.

If it builds, I think it’s fine to do this all at once.

[Zoltan Horvath]

Created attachment 74640 [details]
proposed patch

[WebKit Review Bot]

Attachment 74640 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6404012

[Early Warning System Bot]

Attachment 74640 [details] did not build on qt:
Build output: http://queues.webkit.org/results/6373010

[Eric Seidel (no email)]

Attachment 74640 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6420010

[Zoltan Horvath]

Created attachment 74652 [details]
playing with EWSs

[WebKit Review Bot]

Attachment 74652 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6389012

[Early Warning System Bot]

Attachment 74652 [details] did not build on qt:
Build output: http://queues.webkit.org/results/6408014

[Eric Seidel (no email)]

Attachment 74652 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6391012

[Zoltan Horvath]

Created attachment 74660 [details]
playing with EWSs round #2

I can't do anything on win applying.

[WebKit Review Bot]

Attachment 74660 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6348018

[Zoltan Horvath]

Created attachment 74664 [details]
playing with Chromium's EWS bots round #3

[Zoltan Horvath]

Created attachment 74665 [details]
playing with Chromium's EWS bots round #3

FontCustomPlatformDataCairo.h fixed.

[Zoltan Horvath]

To be continued tomorrow.

[Eric Seidel (no email)]

Attachment 74660 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6404015

[WebKit Review Bot]

Attachment 74665 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6337018

[Zoltan Horvath]

Created attachment 74729 [details]
playing with Chromium's EWS bot round #4

[Zoltan Horvath]

Created attachment 74876 [details]
proposed patch (review candidate)

I tested it with Linux-Chrome, Linux-Gtk and others.

[David Levin]

It would be awesome if WTF_MAKE_FAST_ALLOCATED required a semicolon at the end of it for two reasons:
1. It just follows the typical (mental) pattern in C++ for statements (otherwise it looks like a modifier for the next item.
2. It will help with things that do regex for simple parsing. For example,  editors (like emacs) that help with indenting or check-webkit-style.

A simple way to do this would be to add the following as the last line of WTF_MAKE_FAST_ALLOCATED :
  typedef int ThisIsHereToForceASemicolonAfterThisMacro

[Eric Seidel (no email)]

Attachment 74876 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6342043

[Zoltan Horvath]

Created attachment 74926 [details]
updated rc

[Zoltan Horvath]

(In reply to comment #24)
> It would be awesome if WTF_MAKE_FAST_ALLOCATED required a semicolon at the end of it for two reasons:
> 1. It just follows the typical (mental) pattern in C++ for statements (otherwise it looks like a modifier for the next item.
> 2. It will help with things that do regex for simple parsing. For example,  editors (like emacs) that help with indenting or check-webkit-style.
> 
> A simple way to do this would be to add the following as the last line of WTF_MAKE_FAST_ALLOCATED :
>   typedef int ThisIsHereToForceASemicolonAfterThisMacro

Good idea, I agree with you. I modified the patch.

[Zoltan Horvath]

Created attachment 75113 [details]
updated proposed patch rc

Updated to latest.

[David Levin]

Comment on attachment 75113 [details]
updated proposed patch rc

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

It isn't clear why the noncopyable attribute wasn't retained in various places.

It also isn't clear why noncopyable was converted to noncopyable+fastallocated in some places but only noncopyable in others.

Lastly, I have some concerns about the macro placement and about some side effects of what is hidden in those macros (private:/public: , etc).

> JavaScriptCore/bytecode/CodeBlock.h:582
> +    public:

Remove "public:" here. (Not equivalent to what was here before.)

> JavaScriptCore/bytecode/CodeBlock.h:606
> +    private:

Remove "private:" here.

> JavaScriptCore/parser/Nodes.h:116
> +        WTF_MAKE_FAST_ALLOCATED;

Remove "WTF_MAKE_FAST_ALLOCATED" (because it appears to me that this is not equivalent to what was here before.)

> JavaScriptCore/parser/Nodes.h:1476
> +        WTF_MAKE_FAST_ALLOCATED;

Remove "WTF_MAKE_FAST_ALLOCATED"

> JavaScriptCore/pcre/pcre_exec.cpp:51
> +#include <string.h>

Why is this getting added?

> JavaScriptCore/runtime/MarkStack.h:31
> +#include <string.h>

Why did this get added?

> JavaScriptCore/runtime/RegExpObject.h:63
> +        struct RegExpObjectData {

What happened to the fast allocated quality here?

> JavaScriptCore/runtime/RegExpObject.h:64
> +        public:

Remove "public:"

> JavaScriptCore/runtime/SmallStrings.h:41
> +        WTF_MAKE_NONCOPYABLE(SmallStrings); WTF_MAKE_FAST_ALLOCATED;

Remove "WTF_MAKE_FAST_ALLOCATED"

> JavaScriptCore/runtime/SymbolTable.h:125
> +        WTF_MAKE_FAST_ALLOCATED;

Remove "WTF_MAKE_FAST_ALLOCATED"

> JavaScriptCore/wtf/MD5.h:35
> +#include <stdint.h>

Why is this being added?

> JavaScriptCore/wtf/OwnArrayPtr.h:37
> +template <typename T> class OwnArrayPtr {

Needs  WTF_MAKE_NONCOPYABLE

> JavaScriptCore/wtf/OwnPtr.h:41
> +    template<typename T> class OwnPtr {

Needs  WTF_MAKE_NONCOPYABLE

> JavaScriptCore/wtf/Platform.h:1122
> +/* Noncopyable's implementation as a macro */

Use C++ style comments.

platform.h seems like the wrong header file for this.  Why not put them in the header files where they were?

> JavaScriptCore/wtf/Platform.h:1133
> +        private: \

Hiding private: inside of this macro is unexpected and will lead to people scratching their heads when their class members become private after this is put in. (Much like hiding goto or return in a macro).

Instead how about just putting this macro at the end of classes and explicitly adding a private: section if there isn't one.

> JavaScriptCore/wtf/Platform.h:1147
> +public: \

Same comment as above but instead of putting this macro at the end of the class, one could put it at the end of the public section.

I tend to expect important information at the top of the class and whether the class is fast allocated or not really doesn't matter to me when I trying to use it.

> JavaScriptCore/wtf/Platform.h:1176
> +private: \

Ditto.

> WebKitTools/WebKitAPITest/HostWindow.h:32
> +class HostWindow {

What happened to Noncopyable?

> WebKitTools/WebKitTestRunner/TestInvocation.h:33
> +class TestInvocation {

What happened to Noncopyable?

[Zoltan Horvath]

Created attachment 75696 [details]
updated patch

(In reply to comment #29)
> (From update of attachment 75113 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=75113&action=review
> 
> It isn't clear why the noncopyable attribute wasn't retained in various places.
> 
> It also isn't clear why noncopyable was converted to noncopyable+fastallocated in some places but only noncopyable in others.

When we implemented custom allocation framework into WebKit we made a decision to add FastAllocBase as a base class of Noncopyable because it is a superclass and a lot of heap allocated classes are inherited from it, so some classes needlessly got FastAllocBase's methods. In this patch I corrected these things, because we can choose exactly which class has to be either fast allocated or noncopyable or both. I use our static code analyzer tool (called Columbus, check http://www.inf.u-szeged.hu/sed/softwarequality) to determine these things.

> Lastly, I have some concerns about the macro placement and about some side effects of what is hidden in those macros (private:/public: , etc).
> 
> > JavaScriptCore/bytecode/CodeBlock.h:582
> > +    public:
> 
> Remove "public:" here. (Not equivalent to what was here before.)
> 
> > JavaScriptCore/bytecode/CodeBlock.h:606
> > +    private:
>
> Remove "private:" here.

Adding of WTF_MAKE_FAST_ALLOCATED to RareData struct causes compile errors on windows:

2>c:\cygwin\home\webkit\webkit\javascriptcore\wtf\OwnPtrCommon.h(59) : error C2248: 'JSC::CodeBlock::RareData' : cannot access private struct declared in class 'JSC::CodeBlock'
2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\bytecode\CodeBlock.h(582) : see declaration of 'JSC::CodeBlock::RareData'
2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\bytecode\CodeBlock.h(245) : see declaration of 'JSC::CodeBlock'
2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\wtf/PassOwnPtr.h(58) : see reference to function template instantiation 'void WTF::deleteOwnedPtr<JSC::CodeBlock::RareData>(T *)' being compiled
2>        with
2>        [
2>            T=JSC::CodeBlock::RareData
2>        ]
2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\wtf/PassOwnPtr.h(58) : while compiling class template member function 'WTF::PassOwnPtr<T>::~PassOwnPtr(void)'
2>        with
2>        [
2>            T=JSC::CodeBlock::RareData
2>        ]
2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\bytecode\CodeBlock.h(535) : see reference to class template instantiation 'WTF::PassOwnPtr<T>' being compiled
2>        with
2>        [
2>            T=JSC::CodeBlock::RareData
2>        ]

I don't know what causes this. Modifying its access level has solved the problem.
 
> > JavaScriptCore/parser/Nodes.h:116
> > +        WTF_MAKE_FAST_ALLOCATED;
> 
> Remove "WTF_MAKE_FAST_ALLOCATED" (because it appears to me that this is not equivalent to what was here before.)

Removed.

> > JavaScriptCore/parser/Nodes.h:1476
> > +        WTF_MAKE_FAST_ALLOCATED;
> 
> Remove "WTF_MAKE_FAST_ALLOCATED"

FunctionParameters is a Vector and is a RefCounted. Both Vector and RefCounted is fast allocated, so we need to add the macro to make the methods unambigous for the compilers.

> > JavaScriptCore/pcre/pcre_exec.cpp:51
> > +#include <string.h>
> 
> Why is this getting added?

It is needed for memcpy on Chromium build.

> > JavaScriptCore/runtime/MarkStack.h:31
> > +#include <string.h>
> 
> Why did this get added?

When I removed include of Noncopyable.h memcpy missed string.h. 
 
> > JavaScriptCore/runtime/RegExpObject.h:63
> > +        struct RegExpObjectData {
>
> What happened to the fast allocated quality here?
>

Same problem on Windows like at RareData struct. In the new patch I did the same as with RareData.

> 
> > JavaScriptCore/runtime/RegExpObject.h:64
> > +        public:
> 
> Remove "public:"

Ups. It's a mistake, I removed it.
 
> > JavaScriptCore/runtime/SmallStrings.h:41
> > +        WTF_MAKE_NONCOPYABLE(SmallStrings); WTF_MAKE_FAST_ALLOCATED;
> 
> Remove "WTF_MAKE_FAST_ALLOCATED"

Yeap, removed. SmallStringsStorage should be fast allocated not SmallStrings, changed.

> > JavaScriptCore/runtime/SymbolTable.h:125
> > +        WTF_MAKE_FAST_ALLOCATED;
> 
> Remove "WTF_MAKE_FAST_ALLOCATED"

SharedSymbolTable is a SymbolTable which is HashMap (HashMap is fast allocated), and it is RefCounted as well. 
The reason is same as for FunctionParameters class.

> > JavaScriptCore/wtf/MD5.h:35
> > +#include <stdint.h>
> 
> Why is this being added?

It is needed on windows because of uint8_t and etc.
MD5.cpp also needs string.h because of memcpy.

> > JavaScriptCore/wtf/OwnArrayPtr.h:37
> > +template <typename T> class OwnArrayPtr {
> 
> Needs  WTF_MAKE_NONCOPYABLE

It doesn't need because it has it's own copy constructor: 
JavaScriptCore/wtf/OwnArrayPtr.h:50

> > JavaScriptCore/wtf/OwnPtr.h:41
> > +    template<typename T> class OwnPtr {
> 
> Needs  WTF_MAKE_NONCOPYABLE

Same case as in OwnArrayPtr. CC: JavaScriptCore/wtf/OwnPtr.h:55

> > JavaScriptCore/wtf/Platform.h:1122
> > +/* Noncopyable's implementation as a macro */
> 
> Use C++ style comments.

Gtk build won't compile with C++ style comments. Platform.h contains only C style comments.

> platform.h seems like the wrong header file for this.  Why not put them in the header files where they were?

I think it is simplier to put these to platform.h, in this way we can avoid including Noncopyable.h and FastAllocBase.h headers.

> > JavaScriptCore/wtf/Platform.h:1133
> > +        private: \
> 
> Hiding private: inside of this macro is unexpected and will lead to people scratching their heads when their class members become private after this is put in. (Much like hiding goto or return in a macro).

This is the original Noncopyable macro implementation made by Anders Carlsson in bug #46589.

> 
> Instead how about just putting this macro at the end of classes and explicitly adding a private: section if there isn't one.
>
> > JavaScriptCore/wtf/Platform.h:1147
> > +public: \
> 
> Same comment as above but instead of putting this macro at the end of the class, one could put it at the end of the public section.

Then I think it's better to putting to the end of the classes. Btw, I had a related patch in bug #47887, but I closed it because I didn't get response and I wanted to go ahead with this big patch.

> I tend to expect important information at the top of the class and whether the class is fast allocated or not really doesn't matter to me when I trying to use it.

That is true. I think we should wait for others opinion as well!
I haven't changed these in my new patch yet.

> > JavaScriptCore/wtf/Platform.h:1176
> > +private: \
> 
> Ditto.

Bug #47887 was related to this.
 
> > WebKitTools/WebKitAPITest/HostWindow.h:32
> > +class HostWindow {
> 
> What happened to Noncopyable?

I did't want to include Platform.h. Corrected.

> > WebKitTools/WebKitTestRunner/TestInvocation.h:33
> > +class TestInvocation {
> 
> What happened to Noncopyable?

Ditto.
I think these are the cases which indicate separate header files for noncopyable and fast allocated macros.

[Early Warning System Bot]

Attachment 75696 [details] did not build on qt:
Build output: http://queues.webkit.org/results/6746081

[Zoltan Horvath]

Created attachment 75700 [details]
updated proposed patch

[David Levin]

When I look at http://trac.webkit.org/browser/trunk/JavaScriptCore/wtf/Platform.h, it has no macros of the type that you are adding. It is solely focused on defining what platforms use, enable, and support. The added macros have nothing to do with that.


(In reply to comment #30)
> Created an attachment (id=75696) [details]
> updated patch
> 
> (In reply to comment #29)
> > (From update of attachment 75113 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=75113&action=review
> >  
> Adding of WTF_MAKE_FAST_ALLOCATED to RareData struct causes compile errors on windows:
> 
> 2>c:\cygwin\home\webkit\webkit\javascriptcore\wtf\OwnPtrCommon.h(59) : error C2248: 'JSC::CodeBlock::RareData' : cannot access private struct declared in class 'JSC::CodeBlock'
> 2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\bytecode\CodeBlock.h(582) : see declaration of 'JSC::CodeBlock::RareData'
> 2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\bytecode\CodeBlock.h(245) : see declaration of 'JSC::CodeBlock'
> 2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\wtf/PassOwnPtr.h(58) : see reference to function template instantiation 'void WTF::deleteOwnedPtr<JSC::CodeBlock::RareData>(T *)' being compiled
> 2>        with
> 2>        [
> 2>            T=JSC::CodeBlock::RareData
> 2>        ]
> 2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\wtf/PassOwnPtr.h(58) : while compiling class template member function 'WTF::PassOwnPtr<T>::~PassOwnPtr(void)'
> 2>        with
> 2>        [
> 2>            T=JSC::CodeBlock::RareData
> 2>        ]
> 2>        c:\cygwin\home\webkit\WebKit\JavaScriptCore\bytecode\CodeBlock.h(535) : see reference to class template instantiation 'WTF::PassOwnPtr<T>' being compiled
> 2>        with
> 2>        [
> 2>            T=JSC::CodeBlock::RareData
> 2>        ]
> 
> I don't know what causes this. Modifying its access level has solved the problem.

It would be good to try to understand why you are doing what you are doing rather than just do something to shut up a compiler.

> 
> > > JavaScriptCore/wtf/Platform.h:1133
> > > +        private: \
> > 
> > Hiding private: inside of this macro is unexpected and will lead to people scratching their heads when their class members become private after this is put in. (Much like hiding goto or return in a macro).
> 
> This is the original Noncopyable macro implementation made by Anders Carlsson in bug #46589.

I guess I'll have to take this one up with him.


> > 
> > Instead how about just putting this macro at the end of classes and explicitly adding a private: section if there isn't one.
> >
> > > JavaScriptCore/wtf/Platform.h:1147
> > > +public: \
> > 
> > Same comment as above but instead of putting this macro at the end of the class, one could put it at the end of the public section.
> 
> Then I think it's better to putting to the end of the classes. Btw, I had a related patch in bug #47887, but I closed it because I didn't get response and I wanted to go ahead with this big patch.

Well, I'd support doing it in this patch (or I'm willing to re-visit that other patch). Hiding a change in access level makes reading code harder which is bad.

[David Levin]

(In reply to comment #30)
> Created an attachment (id=75696) [details]
> updated patch
> 
> (In reply to comment #29)
> > (From update of attachment 75113 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=75113&action=review
> > 
> > It isn't clear why the noncopyable attribute wasn't retained in various places.
> > 
> > It also isn't clear why noncopyable was converted to noncopyable+fastallocated in some places but only noncopyable in others.
> 
> When we implemented custom allocation framework into WebKit we made a decision to add FastAllocBase as a base class of Noncopyable because it is a superclass and a lot of heap allocated classes are inherited from it, so some classes needlessly got FastAllocBase's methods. In this patch I corrected these things, because we can choose exactly which class has to be either fast allocated or noncopyable or both. I use our static code analyzer tool (called Columbus, check http://www.inf.u-szeged.hu/sed/softwarequality) to determine these things.

Your ChangeLog has not indication of what is happening here. Also this is not reflected in the bug title.

Ideally you'd keep equivalence with respect to this aspect for this patch (since the patch is not about fixing FastAllocBase inheritance -- it is about getting rid of those classes -- and there are enough changes in here to do that without introducing a separate change for something else at the same time). Immediately after this change, get rid of the places where it isn't needed with an appropriate bug, etc.

[Zoltan Horvath]

Created attachment 75808 [details]
updated, ews test

[Zoltan Horvath]

(In reply to comment #33)
> When I look at http://trac.webkit.org/browser/trunk/JavaScriptCore/wtf/Platform.h, it has no macros of the type that you are adding. It is solely focused on defining what platforms use, enable, and support. The added macros have nothing to do with that.

Yeap, in the past we talked about to put these into platform.h. Currently, I don't know the bug's number.

> It would be good to try to understand why you are doing what you are doing rather than just do something to shut up a compiler.

Done. Patch fixed. More about the build problem: http://msdn.microsoft.com/en-us/library/tsbce2bh(VS.80).aspx


> > 
> > This is the original Noncopyable macro implementation made by Anders Carlsson in bug #46589.
> 
> I guess I'll have to take this one up with him.
> 
> 
> > > 
> > > Instead how about just putting this macro at the end of classes and explicitly adding a private: section if there isn't one.
> > >
> > > > JavaScriptCore/wtf/Platform.h:1147
> > > > +public: \
> > > 
> > > Same comment as above but instead of putting this macro at the end of the class, one could put it at the end of the public section.
> > 
> > Then I think it's better to putting to the end of the classes. Btw, I had a related patch in bug #47887, but I closed it because I didn't get response and I wanted to go ahead with this big patch.
> 
> Well, I'd support doing it in this patch (or I'm willing to re-visit that other patch). Hiding a change in access level makes reading code harder which is bad.

I have started to put the macro's to the end of the classes. It looks terrible. Do we really want to do that?

[Zoltan Horvath]

(In reply to comment #34)

> > When we implemented custom allocation framework into WebKit we made a decision to add FastAllocBase as a base class of Noncopyable because it is a superclass and a lot of heap allocated classes are inherited from it, so some classes needlessly got FastAllocBase's methods. In this patch I corrected these things, because we can choose exactly which class has to be either fast allocated or noncopyable or both. I use our static code analyzer tool (called Columbus, check http://www.inf.u-szeged.hu/sed/softwarequality) to determine these things.
> 
> Your ChangeLog has not indication of what is happening here. Also this is not reflected in the bug title.

Yes. What do you think about change the bug and the changelog title to Refactoring custom allocation framework and its inheritances?
 
> Ideally you'd keep equivalence with respect to this aspect for this patch (since the patch is not about fixing FastAllocBase inheritance -- it is about getting rid of those classes -- and there are enough changes in here to do that without introducing a separate change for something else at the same time). Immediately after this change, get rid of the places where it isn't needed with an appropriate bug, etc.

I'd like to do this one patch if it's possible. Changing from inheriting to use macros indicates extra changes than just a simple batch replace either way. Of course, I need to extend my changelog! Btw, custom allocation framework has effect only on platforms which defines ENABLE_GLOBAL_FASTMALLOC_NEW to 0.

[WebKit Review Bot]

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

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


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

[WebKit Review Bot]

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

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


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

[Early Warning System Bot]

Attachment 75808 [details] did not build on qt:
Build output: http://queues.webkit.org/results/6774094

[WebKit Review Bot]

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

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


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

[WebKit Review Bot]

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

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


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

[Build Bot]

Attachment 75808 [details] did not build on win:
Build output: http://queues.webkit.org/results/6853064

[Build Bot]

Attachment 75808 [details] did not build on win:
Build output: http://queues.webkit.org/results/6735090

[WebKit Review Bot]

Attachment 75808 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6790080

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2
Updating OpenSource
Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061

Died at WebKitTools/Scripts/update-webkit line 132.


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

[Eric Seidel (no email)]

Comment on attachment 75808 [details]
updated, ews test

Sorry about the style-bot noise.  The stylebot has been fixed.  Since this doens't pass any EWS (Except gtk) seems this is unlandable as-is.

[Zoltan Horvath]

Created attachment 77887 [details]
updated, ews test, for prereview

In this patch I don't remove Noncopyable.h, but marcos are still the same with 'private:'.

[Build Bot]

Attachment 77887 [details] did not build on win:
Build output: http://queues.webkit.org/results/7236421

[Zoltan Horvath]

Created attachment 77993 [details]
proposed patch (review candidate)

Windows build and Chromium win build fixed. Patch is updated.

[Zoltan Horvath]

Hey, 

is there any thoughts on this patch?

Z

[Zoltan Horvath]

Created attachment 79429 [details]
updated to latest, playing with ews-s

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSClassRef.h', u..." exit_code: 1

Last 3072 characters of output:
Core/loader/CrossOriginPreflightResultCache.h:68:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/css/MediaQueryEvaluator.h:53:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:67:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadingPrimitives.h:101:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/ValidityState.h:33:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGFontData.h:29:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadSafeShared.h:70:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/ColumnInfo.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/dom/NodeRareData.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/dom/NodeRareData.h:73:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/page/UserStyleSheet.h:39:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLPreloadScanner.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/OriginQuotaManager.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/parser/Parser.h:49:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/canvas/CanvasRenderingContext.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLElementStack.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/bridge/jsc/BridgeJSC.h:63:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/icon/IconDatabase.h:66:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathBlender.h:30:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/platform/ContextMenu.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/NavigationScheduler.cpp:57:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/ImageLoader.cpp:61:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/DatabaseTracker.h:60:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathByteStream.h:47:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderImageResource.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/cache/MemoryCache.h:74:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 193 in 443 files


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

[WebKit Review Bot]

Attachment 79429 [details] did not build on mac:
Build output: http://queues.webkit.org/results/7551213

[Zoltan Horvath]

Created attachment 79564 [details]
proposed patch (review candidate)

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSClassRef.h', u..." exit_code: 1

Last 3072 characters of output:
Core/loader/CrossOriginPreflightResultCache.h:68:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/css/MediaQueryEvaluator.h:53:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:67:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadingPrimitives.h:101:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/ValidityState.h:33:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGFontData.h:29:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadSafeShared.h:70:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/ColumnInfo.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/dom/NodeRareData.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/dom/NodeRareData.h:73:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/page/UserStyleSheet.h:39:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLPreloadScanner.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/OriginQuotaManager.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/parser/Parser.h:49:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/canvas/CanvasRenderingContext.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLElementStack.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/bridge/jsc/BridgeJSC.h:63:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/icon/IconDatabase.h:66:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathBlender.h:30:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/platform/ContextMenu.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/NavigationScheduler.cpp:57:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/ImageLoader.cpp:61:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/DatabaseTracker.h:60:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathByteStream.h:47:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderImageResource.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/cache/MemoryCache.h:74:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 192 in 445 files


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

[Zoltan Horvath]

Created attachment 79566 [details]
proposed patch (review candidate) + efl fix

[WebKit Review Bot]

Attachment 79564 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7613242

[Early Warning System Bot]

Attachment 79564 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7534250

[Zoltan Horvath]

Created attachment 79568 [details]
proposed patch (review candidate)

[WebKit Review Bot]

Attachment 79564 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7499244

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSClassRef.h', u..." exit_code: 1

Last 3072 characters of output:
Core/loader/CrossOriginPreflightResultCache.h:68:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/css/MediaQueryEvaluator.h:53:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:67:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadingPrimitives.h:101:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/ValidityState.h:33:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGFontData.h:29:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadSafeShared.h:70:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/ColumnInfo.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/dom/NodeRareData.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/dom/NodeRareData.h:73:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/page/UserStyleSheet.h:39:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLPreloadScanner.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/OriginQuotaManager.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/parser/Parser.h:49:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/canvas/CanvasRenderingContext.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLElementStack.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/bridge/jsc/BridgeJSC.h:63:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/icon/IconDatabase.h:66:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathBlender.h:30:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/platform/ContextMenu.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/NavigationScheduler.cpp:57:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/ImageLoader.cpp:61:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/DatabaseTracker.h:60:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathByteStream.h:47:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderImageResource.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/cache/MemoryCache.h:74:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 192 in 445 files


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

[Zoltan Horvath]

Created attachment 79571 [details]
proposed patch (review candidate)

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSClassRef.h', u..." exit_code: 1

Last 3072 characters of output:
Core/loader/CrossOriginPreflightResultCache.h:68:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/css/MediaQueryEvaluator.h:53:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:67:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadingPrimitives.h:101:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/ValidityState.h:33:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGFontData.h:29:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadSafeShared.h:70:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/ColumnInfo.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/dom/NodeRareData.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/dom/NodeRareData.h:73:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/page/UserStyleSheet.h:39:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLPreloadScanner.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/OriginQuotaManager.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/parser/Parser.h:49:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/canvas/CanvasRenderingContext.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLElementStack.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/bridge/jsc/BridgeJSC.h:63:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/icon/IconDatabase.h:66:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathBlender.h:30:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/platform/ContextMenu.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/NavigationScheduler.cpp:57:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/ImageLoader.cpp:61:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/DatabaseTracker.h:60:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathByteStream.h:47:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderImageResource.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/cache/MemoryCache.h:74:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 192 in 445 files


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

[WebKit Review Bot]

Attachment 79568 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7549242

[Zoltan Horvath]

Created attachment 79572 [details]
proposed patch (review candidate)

EFL fix.

[WebKit Review Bot]

Attachment 79571 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7591225

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSClassRef.h', u..." exit_code: 1

Last 3072 characters of output:
Core/loader/CrossOriginPreflightResultCache.h:68:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/css/MediaQueryEvaluator.h:53:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:67:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadingPrimitives.h:101:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/ValidityState.h:33:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGFontData.h:29:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadSafeShared.h:70:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/ColumnInfo.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/dom/NodeRareData.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/dom/NodeRareData.h:73:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/page/UserStyleSheet.h:39:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLPreloadScanner.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/OriginQuotaManager.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/parser/Parser.h:49:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/canvas/CanvasRenderingContext.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLElementStack.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/bridge/jsc/BridgeJSC.h:63:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/icon/IconDatabase.h:66:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathBlender.h:30:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/platform/ContextMenu.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/NavigationScheduler.cpp:57:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/ImageLoader.cpp:61:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/DatabaseTracker.h:60:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathByteStream.h:47:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderImageResource.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/cache/MemoryCache.h:74:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 192 in 446 files


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

[Early Warning System Bot]

Attachment 79571 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7613245

[WebKit Review Bot]

Attachment 79564 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7510220

[WebKit Review Bot]

Attachment 79572 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7493236

[Early Warning System Bot]

Attachment 79572 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7627207

[WebKit Review Bot]

Attachment 79572 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7584224

[Zoltan Horvath]

Created attachment 79581 [details]
proposed patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSClassRef.h', u..." exit_code: 1

Last 3072 characters of output:
Core/loader/CrossOriginPreflightResultCache.h:68:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/css/MediaQueryEvaluator.h:53:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:67:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadingPrimitives.h:101:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/ValidityState.h:33:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGFontData.h:29:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/wtf/ThreadSafeShared.h:70:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/ColumnInfo.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/dom/NodeRareData.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/dom/NodeRareData.h:73:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/page/UserStyleSheet.h:39:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLPreloadScanner.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/OriginQuotaManager.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/JavaScriptCore/parser/Parser.h:49:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/canvas/CanvasRenderingContext.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/html/parser/HTMLElementStack.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/bridge/jsc/BridgeJSC.h:63:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/icon/IconDatabase.h:66:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathBlender.h:30:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/platform/ContextMenu.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/NavigationScheduler.cpp:57:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/ImageLoader.cpp:61:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/storage/DatabaseTracker.h:60:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/svg/SVGPathByteStream.h:47:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/rendering/RenderImageResource.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WebCore/loader/cache/MemoryCache.h:74:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 192 in 446 files


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

[Csaba Osztrogonác]

Comment on attachment 79581 [details]
proposed patch

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

Zoltan fixed everything what David mentioned, except putting macros at the end of classes.
I think these macros should be at the top to get quick if a class is fast allocated or not.

Great work, r=me. Please be sure not to break any platform.

> Source/JavaScriptCore/wtf/FastAllocBase.h:60
>  // Example usage:
> -//    class Widget : public FastAllocBase { ... };
> +//    class Widget {
> +//        WTF_MAKE_FAST_ALLOCATED
> +//    ...
> +//    };
> +//
> +//    struct Data {
> +//        WTF_MAKE_FAST_ALLOCATED
> +//    public:
> +//    ...
> +//    };

This documentation is should be enough for developers not to surprise 
at private struct members if they miss to use explicit public modifier.

Additionally please write a mail to webkit-dev about this change. (and maybe a wiki page too)

[WebKit Review Bot]

http://trac.webkit.org/changeset/76248 might have broken Windows Debug (Build)

[Csaba Osztrogonác]

Patch landed in http://trac.webkit.org/changeset/76248
Qt-V8 buildfix landed in http://trac.webkit.org/changeset/76251
Mac and Win DRT buildfix landed in http://trac.webkit.org/changeset/76253

[Csaba Osztrogonác]

Comment on attachment 79581 [details]
proposed patch

remove r+ from landed patch