Bug 140691 – Teach run-webkit-app --simulator how to install custom built app

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Daniel Bates]

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

[Daniel Bates]

Created attachment 245013 [details]
Patch

[David Kilzer (:ddkilzer)]

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.

[Daniel Bates]

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

[Daniel Bates]

Created attachment 245359 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 245359 [details]
Patch

r=me

[Daniel Bates]

Committed r179130: <http://trac.webkit.org/changeset/179130>

[Daniel Bates]

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