[Cocoa] Update legacy WebKit thread checks to prepare for use on iOS and modern WebKit
Created attachment 423263 [details] Patch
Comment on attachment 423263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423263&action=review Looks good to me. > Source/WebCore/platform/mac/ThreadCheck.mm:74 > + NSLog(@"WebKit Threading Violation - %s was called from secondary thread", function); Maybe we should be using RELEASE_LOG_ERROR() so that its stands out in console logging as an error? > Source/WebCore/platform/mac/ThreadCheck.mm:75 > + NSLog(@"Additional threading violations for this function will not be logged."); ditto. > Source/WebCore/platform/mac/ThreadCheck.mm:78 > + NSLog(@"WebKit Threading Violation - %s was called from secondary thread", function); Ditto.
Comment on attachment 423263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423263&action=review > Source/WebCore/platform/ThreadCheck.h:40 > + using BehaviorArray = std::array<Behavior, std::numeric_limits<uint8_t>::max()>; It might be better to have a single global round / behavior versions so that there is no confusion about "which round one" we're talking about. > Source/WebCore/platform/ThreadCheck.h:53 > + static std::once_flag flag; > + std::call_once(flag, [] { > + initializeBehaviors(setDefaultBehaviors); > + }); Hm... I think it would be more clear to have this code in the constructor of ThreadViolationChecker.
I wonder if we can write API tests for these...
Comment on attachment 423263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423263&action=review >> Source/WebCore/platform/ThreadCheck.h:53 >> + }); > > Hm... I think it would be more clear to have this code in the constructor of ThreadViolationChecker. The reason it’s here is so that the caller can pass in the function as a template argument. I didn’t want the class itself to be a class template.
Comment on attachment 423263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423263&action=review >> Source/WebCore/platform/ThreadCheck.h:40 >> + using BehaviorArray = std::array<Behavior, std::numeric_limits<uint8_t>::max()>; > > It might be better to have a single global round / behavior versions > so that there is no confusion about "which round one" we're talking about. The design here is that WebKit has a set of rounds, and WebKitLegacy has a separate set. That’s why each object has a different behavior array. One for WebKit and one for WebKitLegacy. I thought it would be clearer to keep them separate, since they are based on different linked-on-or-after checks. Could go another way, of course.
<rdar://problem/75714190>