Bug 38060 – Split up Threading.h

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Sam Weinig]

Threading.h is getting a little large. Time for a split.

[Sam Weinig]

Created attachment 54194 [details]
patch

[David Levin]

Two comments:
1. The include<wtf/*> sorting is off for the newly added items. 
2. I'm surprised that the new headers don't have entries in the ForwardingHeaders/wtf directories in 
JavaScriptGlue, WebCore, WebKit/mac, WebKitTools/DumpRenderTree (which is where Threading.h appears). Is this not necessary?

[Sam Weinig]

(In reply to comment #2)
> Two comments:
> 1. The include<wtf/*> sorting is off for the newly added items. 

Will fix.

> 2. I'm surprised that the new headers don't have entries in the
> ForwardingHeaders/wtf directories in 
> JavaScriptGlue, WebCore, WebKit/mac, WebKitTools/DumpRenderTree (which is where
> Threading.h appears). Is this not necessary?

:( They do. I just forgot to upload that part of the patch.

[Sam Weinig]

Created attachment 54198 [details]
Updated patch

Updated.  The WebKit/mac forwarding headers are not needed.

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/wtf/ThreadSafeShared.h:65:  Alphabetical sorting problem.  [build/include_order] [4]
JavaScriptCore/wtf/Atomics.h:71:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 2 in 17 files


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

[David Levin]

Atomics.h is messed up in this version. "#ifndef Threading_h" etc.

Also, none of your fwd'ing headers have ifndef guards but that seems rather useless and just adds extra clutter so I'm fine with omitting that.

And the style bot noticed a sorting issue.

I would r+ except for the Atomics.h

[Sam Weinig]

(In reply to comment #6)
> Atomics.h is messed up in this version. "#ifndef Threading_h" etc.
> 

I don's see the issue you are referring to.

> Also, none of your fwd'ing headers have ifndef guards but that seems rather
> useless and just adds extra clutter so I'm fine with omitting that.

Some do (the ones in WebCore/ForwardingHeaders).  I tried to be consistent with the files around them.

> And the style bot noticed a sorting issue.

Will fix before landing.

[David Levin]

Comment on attachment 54198 [details]
Updated patch

You're missing the changes in WebCore, so please make sure to include that when you land it.

About this "Atomics.h is messed up in this version."... I got mixed up because it was in the patch twice (due to the svn cp I guess).

[Sam Weinig]

Landed in r58206.