Summary: | Allow for multiple DumpRenderTree and WebKitTestRunner instances in the iOS Simulator | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Farler <dfarler> | ||||
Component: | Tools / Tests | Assignee: | David Farler <dfarler> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dbates, ddkilzer, simon.fraser | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | iPhone / iPad | ||||||
OS: | All | ||||||
Attachments: |
|
Description
David Farler
2014-07-24 17:06:39 PDT
Created attachment 235477 [details]
Patch
Comment on attachment 235477 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235477&action=review > Tools/DumpRenderTree/mac/DumpRenderTree.mm:1126 > + const char *stdoutPath = [[NSString stringWithFormat:@"/tmp/%@_OUT", identifier] UTF8String]; > + const char *stderrPath = [[NSString stringWithFormat:@"/tmp/%@_ERROR", identifier] UTF8String]; > + Do we not need to unique these paths per DRT instance if several are running at once? > Tools/DumpRenderTree/mac/DumpRenderTree.mm:1249 > + NSTimeInterval timeRemaining; > + while (true) { > + timeRemaining = [application backgroundTimeRemaining]; > + if (timeRemaining <= 10.0 || bgTask == UIBackgroundTaskInvalid) { > + [application endBackgroundTask:bgTask]; > + bgTask = [application beginBackgroundTaskWithExpirationHandler:expirationHandler]; > + } > + sleep(5); Why is all this necessary? > Tools/DumpRenderTree/mac/DumpRenderTreeMac.h:80 > + UIBackgroundTaskIdentifier bgTask; bgTask -> backgroundTaskIdentifier. > Tools/WebKitTestRunner/ios/mainIOS.mm:35 > + UIBackgroundTaskIdentifier bgTask; backgroundTaskIdentifier > Tools/WebKitTestRunner/ios/mainIOS.mm:68 > + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ > + > + NSTimeInterval timeRemaining; > + while (true) { > + timeRemaining = [application backgroundTimeRemaining]; > + if (timeRemaining <= 10.0 || bgTask == UIBackgroundTaskInvalid) { > + [application endBackgroundTask:bgTask]; > + bgTask = [application beginBackgroundTaskWithExpirationHandler:expirationHandler]; > + } > + sleep(5); > + } Again, an explanatory comment would be useful. > Tools/WebKitTestRunner/ios/mainIOS.mm:79 > + UIApplicationMain(argc, (char**)argv, NSStringFromClass([WebKitTestRunnerApp class]), NSStringFromClass([WebKitTestRunnerApp class])); Seems odd not to just say @"WebKitTestRunnerApp" here. (In reply to comment #2) > (From update of attachment 235477 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235477&action=review > > > Tools/DumpRenderTree/mac/DumpRenderTree.mm:1126 > > + const char *stdoutPath = [[NSString stringWithFormat:@"/tmp/%@_OUT", identifier] UTF8String]; > > + const char *stderrPath = [[NSString stringWithFormat:@"/tmp/%@_ERROR", identifier] UTF8String]; > > + > > Do we not need to unique these paths per DRT instance if several are running at once? > > > Tools/DumpRenderTree/mac/DumpRenderTree.mm:1249 > > + NSTimeInterval timeRemaining; > > + while (true) { > > + timeRemaining = [application backgroundTimeRemaining]; > > + if (timeRemaining <= 10.0 || bgTask == UIBackgroundTaskInvalid) { > > + [application endBackgroundTask:bgTask]; > > + bgTask = [application beginBackgroundTaskWithExpirationHandler:expirationHandler]; > > + } > > + sleep(5); > > Why is all this necessary? The app will get suspended or killed some time after going into the background without some background task to keep the app alive. This is just a dummy task that does nothing but renew itself when the background time remaining dips below some safe number. 10 seconds was safe enough in my testing. > > > Tools/DumpRenderTree/mac/DumpRenderTreeMac.h:80 > > + UIBackgroundTaskIdentifier bgTask; > > bgTask -> backgroundTaskIdentifier. No problem, will fix. > > > Tools/WebKitTestRunner/ios/mainIOS.mm:35 > > + UIBackgroundTaskIdentifier bgTask; > > backgroundTaskIdentifier > > > Tools/WebKitTestRunner/ios/mainIOS.mm:68 > > + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ > > + > > + NSTimeInterval timeRemaining; > > + while (true) { > > + timeRemaining = [application backgroundTimeRemaining]; > > + if (timeRemaining <= 10.0 || bgTask == UIBackgroundTaskInvalid) { > > + [application endBackgroundTask:bgTask]; > > + bgTask = [application beginBackgroundTaskWithExpirationHandler:expirationHandler]; > > + } > > + sleep(5); > > + } > > Again, an explanatory comment would be useful. I’ll add comments in both apps for these blocks explaining why it’s necessary. > > > Tools/WebKitTestRunner/ios/mainIOS.mm:79 > > + UIApplicationMain(argc, (char**)argv, NSStringFromClass([WebKitTestRunnerApp class]), NSStringFromClass([WebKitTestRunnerApp class])); > > Seems odd not to just say @"WebKitTestRunnerApp" here. Yeah, it’s just a plain string in DRT. I was just following convention here, but I suppose I can change it. The only advantage to using this is to catch class renaming at compile time. Come to think of it, the class names between these two projects are a little weird but I doubt we’ll change them any time soon. Thanks for the review. > > Do we not need to unique these paths per DRT instance if several are running at once?
Forgot to mention - the unique app bundle with a renamed app identifier is handled by the relay and ultimately the existing worker shard mechanism in webkitpy.
Committed r171687: <http://trac.webkit.org/changeset/171687> |