Layering violations in ThreadGlobalData
Created attachment 202926 [details] Patch
Attachment 202926 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/DOMImplementation.cpp', u'Source/WebCore/dom/DOMImplementation.h', u'Source/WebCore/dom/EventNames.h', u'Source/WebCore/inspector/InspectorCounters.cpp', u'Source/WebCore/inspector/InspectorCounters.h', u'Source/WebCore/loader/cache/CachedResourceRequestInitiators.h', u'Source/WebCore/platform/ThreadGlobalData.cpp', u'Source/WebCore/platform/ThreadGlobalData.h', u'Source/WebCore/platform/ThreadTimers.cpp', u'Source/WebCore/platform/ThreadTimers.h', u'Source/WebCore/platform/Timer.cpp', u'Source/WebCore/platform/text/TextCodecICU.cpp', u'Source/WebCore/platform/text/TextCodecICU.h', u'Source/WebCore/platform/text/mac/TextCodecMac.cpp', u'Source/WebCore/platform/text/mac/TextCodecMac.h', u'Source/WebCore/workers/WorkerRunLoop.cpp']" exit_code: 1 Source/WebCore/platform/ThreadGlobalData.h:45: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 202926 [details] Patch Attachment 202926 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/739354
Comment on attachment 202926 [details] Patch Attachment 202926 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/739356
Comment on attachment 202926 [details] Patch Attachment 202926 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/686529
Comment on attachment 202926 [details] Patch Attachment 202926 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/718216
Comment on attachment 202926 [details] Patch Attachment 202926 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/686532
Comment on attachment 202926 [details] Patch Attachment 202926 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/665221
Comment on attachment 202926 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202926&action=review We need to find a way to solve this without introducing a new hash map lookup for every eventNames() access. > Source/WebCore/platform/ThreadGlobalData.h:-60 > - const CachedResourceRequestInitiators& cachedResourceRequestInitiators() { return *m_cachedResourceRequestInitiators; } Why was this ever needed? This looks like nearly dead code (only used by RESOURCE_TIMING), and never needed to use per-thread AtomicStrings anyway - it could use a simple plain enum. > Source/WebCore/platform/ThreadGlobalData.h:-61 > - EventNames& eventNames() { return *m_eventNames; } This one we probably need, to support event dispatch in workers. But if we can make AtomicStrings thread safe, then there will be no need to have them in thread local storage. > Source/WebCore/platform/ThreadGlobalData.h:-62 > - ThreadTimers& threadTimers() { return *m_threadTimers; } Timers are a platforms concept, and should be fine to keep. > Source/WebCore/platform/ThreadGlobalData.h:-63 > - XMLMIMETypeRegExp& xmlTypeRegExp() { return *m_xmlTypeRegExp; } This just seems completely useless. We shouldn't use regular expressions for parsing, and it's certainly not some kind of super hot code AFAICT. > Source/WebCore/platform/ThreadGlobalData.h:-66 > - ICUConverterWrapper& cachedConverterICU() { return *m_cachedConverterICU; } This is also part of platform. > Source/WebCore/platform/ThreadGlobalData.h:-70 > - TECConverterWrapper& cachedConverterTEC() { return *m_cachedConverterTEC; } Ditto. > Source/WebCore/platform/ThreadGlobalData.h:-74 > - ThreadLocalInspectorCounters& inspectorCounters() { return *m_inspectorCounters; } Unsure what this is. Does Inspector actually hook up to multiple threads?
Removing XMLMIMETypeRegExp in bug #116861.