Bug 140691

Summary: Teach run-webkit-app --simulator how to install custom built app
Product: WebKit Reporter: Daniel Bates <dbates>
Component: Tools / TestsAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: iPhone / iPad   
OS: iOS 8.1   
Attachments:
Description Flags
Patch
none
Patch ddkilzer: review+

Description Daniel Bates 2015-01-20 12:18:58 PST
Currently run-webkit-app --simulator can only run apps that are either system apps or have been installed (say, using simctl install) in the simulator device whose name has the form "iPhone 5s For WebKit Development." Towards making run-webkit-app more straightforward to use we should teach webkitdirs::runIOSWebKitAppInSimulator() how to install a custom built app. As a side effect of such a change run-safari --simulator will know how to install a custom built MobileSafari (if it exists).
Comment 1 Daniel Bates 2015-01-20 13:09:10 PST
Created attachment 245013 [details]
Patch
Comment 2 David Kilzer (:ddkilzer) 2015-01-26 10:27:26 PST
Comment on attachment 245013 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=245013&action=review

r=me, but please consider renaming openIOSSimulator() (in a follow-up patch if easier) and whether it's okay to remove the sleep() calls from the do/while loops.

> Tools/Scripts/webkitdirs.pm:2074
>  sub openIOSSimulator($)

This subroutine should have a better name if it always quits the iOS Simulator, and then relaunches it.

It will also make the comments below (near calls to openIOSSimulator()) make more sense.  :)

> Tools/Scripts/webkitdirs.pm:2086
>      my $device;
>      do {
> -        $device = iosSimulatorDeviceByName($simulatedDevice->{name});
> -        sleep(2);
> +        $device = iosSimulatorDeviceByUDID($simulatedDevice->{UDID});
>      } while ($device->{state} ne SIMULATOR_DEVICE_STATE_BOOTED);

Why is it okay to remove the sleep(2) statement here?  Do we really want to run this as quickly as possible?  (I'd like this answered before landing the patch.)

Also, this loop is duplicated (other than the UDID and the state being waited for) in quitIOSSimulator().  Might be a good candidate to extract a common subroutine?  (This does not block landing the patch.)

> Tools/Scripts/webkitdirs.pm:2103
> +    my $device;
> +    do {
> +        $device = iosSimulatorDeviceByUDID($waitForShutdownOfSimulatedDeviceUDID);
> +    } while ($device->{state} ne SIMULATOR_DEVICE_STATE_SHUTDOWN);

Ditto about no sleep() statement here, and extracting common code.
Comment 3 Daniel Bates 2015-01-26 11:18:19 PST
(In reply to comment #2)
> > Tools/Scripts/webkitdirs.pm:2074
> >  sub openIOSSimulator($)
> 
> This subroutine should have a better name if it always quits the iOS
> Simulator, and then relaunches it.
>

Will rename this function to relaunchIOSSimulator.

> 
> > Tools/Scripts/webkitdirs.pm:2086
> >      my $device;
> >      do {
> > -        $device = iosSimulatorDeviceByName($simulatedDevice->{name});
> > -        sleep(2);
> > +        $device = iosSimulatorDeviceByUDID($simulatedDevice->{UDID});
> >      } while ($device->{state} ne SIMULATOR_DEVICE_STATE_BOOTED);
> 
> Why is it okay to remove the sleep(2) statement here?  Do we really want to
> run this as quickly as possible?

As discussed on IRC today (01/26), the sleep() calls made run-safari feel sluggish since it takes units in seconds. Let's try sleeping for 500ms between polls to the file system for the simulator device state.

> Also, this loop is duplicated (other than the UDID and the state being
> waited for) in quitIOSSimulator().  Might be a good candidate to extract a
> common subroutine?  (This does not block landing the patch.)
> 
> > Tools/Scripts/webkitdirs.pm:2103
> > +    my $device;
> > +    do {
> > +        $device = iosSimulatorDeviceByUDID($waitForShutdownOfSimulatedDeviceUDID);
> > +    } while ($device->{state} ne SIMULATOR_DEVICE_STATE_SHUTDOWN);
> 
> Ditto about no sleep() statement here, and extracting common code.

Will extract common code into a shared function and sleep for 500ms between calls to iosSimulatorDeviceByUDID().
Comment 4 Daniel Bates 2015-01-26 11:26:29 PST
Created attachment 245359 [details]
Patch
Comment 5 David Kilzer (:ddkilzer) 2015-01-26 11:34:56 PST
Comment on attachment 245359 [details]
Patch

r=me
Comment 6 Daniel Bates 2015-01-26 11:45:47 PST
Committed r179130: <http://trac.webkit.org/changeset/179130>
Comment 7 Daniel Bates 2015-01-27 07:18:30 PST
I inadvertently omitted the directory test flag in hasUserInstalledAppInSimulatorDevice() when testing for the presence of the simulator device-specific user installed app directory.

Committed fix in <http://trac.webkit.org/changeset/179179>.