The RuntimeEnabledFeatures pseudo-singleton class has a number of inline getters and setters. When these are used to access the static members of the class across a DLL boundary, we are encountering crashes under Windows. While this seems to be behavior specific to Visual Studio, it is not a good design and should be refactored to act as a 'real' Singleton object to avoid non-deterministic behavior for reading/writing this memory.
Created attachment 212529 [details] Patch
Comment on attachment 212529 [details] Patch Attachment 212529 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/2174074
Comment on attachment 212529 [details] Patch Attachment 212529 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/2165004
Comment on attachment 212529 [details] Patch Attachment 212529 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/2162073
Comment on attachment 212529 [details] Patch Attachment 212529 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/2217006
Comment on attachment 212529 [details] Patch Attachment 212529 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/2169077
Comment on attachment 212529 [details] Patch Attachment 212529 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/2174085
Comment on attachment 212529 [details] Patch Attachment 212529 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/2190067
Comment on attachment 212529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212529&action=review Awesome. Just a few nits. > Source/WebCore/bindings/generic/RuntimeEnabledFeatures.cpp:142 > +RuntimeEnabledFeatures& RuntimeEnabledFeatures::sharedRuntimeEnabledFeatures() RuntimeEnabledFeatures::sharedRuntimeEnabledFeatures() seems a bit redundant. How about RuntimeEnabledFeatures::sharedFeatures()? > Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:45 > class RuntimeEnabledFeatures { > public: You should add WTF_MAKE_NONCOPYABLE(RuntimeEnabledFeatures) here now that we have an instance. > Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:317 > + bool isLocalStorageEnabled; > + bool isSessionStorageEnabled; > + bool isWebkitNotificationsEnabled; > + bool isApplicationCacheEnabled; > + bool isDataTransferItemsEnabled; > + bool isGeolocationEnabled; > + bool isIndexedDBEnabled; > + bool isTouchEnabled; > + bool isDeviceMotionEnabled; > + bool isDeviceOrientationEnabled; > + bool isSpeechInputEnabled; > + bool isCanvasPathEnabled; > + bool isCSSExclusionsEnabled; > + bool isCSSShapesEnabled; > + bool isCSSRegionsEnabled; > + bool isCSSCompositingEnabled; > + bool isLangAttributeAwareFormControlUIEnabled; While we're cleaning up bad coding style, we should really make all these variables conform to WebKit Coding Style, with m_ prefixes for each.
Created attachment 212601 [details] Patch
Created attachment 212604 [details] Updated after removal of HTMLContentElement.cpp
Once this patch is in place we can reactivate CSS Shapes on Windows (i.e., revert Bug 121879).
Comment on attachment 212604 [details] Updated after removal of HTMLContentElement.cpp Attachment 212604 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/2212053
Comment on attachment 212604 [details] Updated after removal of HTMLContentElement.cpp Attachment 212604 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/2217054
Created attachment 212608 [details] Patch
Comment on attachment 212608 [details] Patch Attachment 212608 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/2217061
Created attachment 212612 [details] Removed offending WebCore.exp.in entries
Comment on attachment 212612 [details] Removed offending WebCore.exp.in entries Watching the mac/mac-wk2 bots. They will probably fail one more time to show me the symbol I need to add to WebCore.exp.in to support the new Singleton object.
Comment on attachment 212612 [details] Removed offending WebCore.exp.in entries Attachment 212612 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/2224054
Comment on attachment 212612 [details] Removed offending WebCore.exp.in entries Attachment 212612 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/2220014
Created attachment 212620 [details] Removed typo from WebCore.exp.in
Comment on attachment 212620 [details] Removed typo from WebCore.exp.in Attachment 212620 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/2240010
Created attachment 212625 [details] Updated with missing symbol in WebCore.exp.in
Comment on attachment 212625 [details] Updated with missing symbol in WebCore.exp.in r=me
Committed r156424: <http://trac.webkit.org/changeset/156424>
Comment on attachment 212625 [details] Updated with missing symbol in WebCore.exp.in View in context: https://bugs.webkit.org/attachment.cgi?id=212625&action=review > Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:419 > +#ifndef CLASS_IF_GCC > +#if COMPILER(GCC) > +#define CLASS_IF_GCC class > +#else > +#define CLASS_IF_GCC > +#endif > +#endif > + friend CLASS_IF_GCC NeverDestroyed<RuntimeEnabledFeatures>; Sooooooo ugly. :-/ Is it a well-known MSVC bug? It seems we ran into the same bug in https://bugs.webkit.org/show_bug.cgi?id=123442 But I prefer a general fix for it instead of adding workarounds everywhere.