Remove AtomicString's dependency on ThreadGlobalData so that we can move WebCore's string classes up to WTF.
Created attachment 53590 [details] The patch Mainly uploading this patch to get the build bot to take a sniff at it, probably not worth reviewing right now.
Created attachment 53591 [details] More useful patch, with build file changes – still for the build bot.
Attachment 53590 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/1605587
Attachment 53591 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/1605589
Attachment 53590 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/1676363
The WTF prefix on the file and type name seems redundant, and completely inconsistent with other files in WTF.
Created attachment 53620 [details] Chromium build fix
(In reply to comment #6) > The WTF prefix on the file and type name seems redundant, and completely > inconsistent with other files in WTF. Most classes in the WTF directories are classes to be used by clients of WTF, not parts of WTF's internal implementation (for example ThreadSpecific, which is used instantiated by clients of WTF), and as such the name ThreadData seemed too generic - it did not seem clear that this was not something to be used by clients of WTF but instead something that WTF would use internally. I added the prefix to try to make this distinction a little more clear. What do you suggest for a class name, do you like just "ThreadData"? or any other suggestions?
(In reply to comment #8) > (In reply to comment #6) > > The WTF prefix on the file and type name seems redundant, and completely > > inconsistent with other files in WTF. > > Most classes in the WTF directories are classes to be used by clients of WTF, > not parts of WTF's internal implementation (for example ThreadSpecific, which > is used instantiated by clients of WTF), and as such the name ThreadData seemed > too generic - it did not seem clear that this was not something to be used by > clients of WTF but instead something that WTF would use internally. I added > the prefix to try to make this distinction a little more clear. A word like “private” or “internal” would bring with it the connotation that this is not supposed to be used outside of WTF. Using WTF as a prefix just makes me wonder WTF this file happens to start with WTF.
(In reply to comment #9) > A word like “private” or “internal” would bring with it the connotation that > this is not supposed to be used outside of WTF. Using WTF as a prefix just > makes me wonder WTF this file happens to start with WTF. While it is a part of WTF, it is also accessed from outside of WTF where certain thread-specific pieces of information need to be accessed (currently just the string uniquing tables), so the word 'private' seems inappropriate. While it is a part of WTF's implementation rather than a class vended to clients, it is a public piece of implementation. We also have a ThreadGlobalData object in WebCore, and I wanted to be nice and clear and unambiguous about which thread specific data object was being accessed, for example consider the following line from the patch from within WebCore: WTFThreadData& data = wtfThreadData(); It seems nice and clear that this is accessing the thread data from and pertaining to WTF, rather than WebCore's own thread data. Would you suggest: InternalThreadData& data = wtfThreadData(); ? InternalThreadData& data = internalThreadData(); ? I'd suggest that having wtf in the name does make sense for this class and function, since it makes it completely clear whose thread data is being accessed (WTF's as opposed than WebCore's).
Comment on attachment 53620 [details] Chromium build fix > +namespace JSC { > +class IdentifierTable; > +} > + > namespace WebCore { > +class AtomicStringTable; > +} How weird. Please put a comment explaining how this is a temporary measure while we finish the great string merger. Otherwise, r=me.
This broke windows.
Sadly the windows EWS bot is offline otherwise it would have told us this. :(
I got frustrated with windows case insensitive file system. I can try bringing it back online.
landed in http://trac.webkit.org/changeset/57829
http://trac.webkit.org/changeset/57829 might have broken Windows Debug (Tests) The following changes are on the blame list: http://trac.webkit.org/changeset/57829 http://trac.webkit.org/changeset/57830 http://trac.webkit.org/changeset/57831 http://trac.webkit.org/changeset/57832 http://trac.webkit.org/changeset/57833 http://trac.webkit.org/changeset/57834 http://trac.webkit.org/changeset/57835