Bug 135272 – Allow for multiple DumpRenderTree and WebKitTestRunner instances in the iOS Simulator

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Farler]

To support multiple DRT/WKTR running in the simulator, the relay pipes should not be hard-coded but based on their bundle identifiers, which are uniqued when installing multiple copies into the simulator. In addition, they will need a renewing background task in order to keep running while the other copies are spawned.

[David Farler]

Created attachment 235477 [details]
Patch

[Simon Fraser (smfr)]

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.

[David Farler]

(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.

[David Farler]

> > 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.

[David Farler]

Committed r171687: <http://trac.webkit.org/changeset/171687>