1016073 - mozdevice's stopApplication should work on older versions of Android

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[William Lachance (:wlach)]

While hacking on bug 1016041 for eideticker, I realized that I had written some compatibility type code which tries to kill a browser in a similar way to "am force-stop" (i.e. trying repeatedly over a duration to kill all instances of a process), since just running killProcess seems not to be 100% reliable (because background processes might occasionally be launched when you're in the process of killing the browser).

We may as well try to consolidate this code inside devicemanager so other people can use it.

Comment 1

[William Lachance (:wlach)]

Attachment: Add Android 2.x support to stopApplication

Comment 2

[Bob Clary [:bc] (inactive)]

Comment on attachment 8428892 [details] [diff] [review]
Add Android 2.x support to stopApplication

Review of attachment 8428892 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mozbase/mozdevice/mozdevice/droid.py
@@ +141,5 @@
> +                if num_tries > max_tries:
> +                    raise DMError("Couldn't successfully kill %s after %s "
> +                                  "tries" % (appName, max_tries))
> +                self.killProcess(appName)
> +                num_tries+=1

spaces

@@ +144,5 @@
> +                self.killProcess(appName)
> +                num_tries+=1
> +                # sleep for a short duration to make sure there are no
> +                # additional processes in the process of being launched
> +                time.sleep(0.1)

How did you decide on 0.1 seconds?

Comment 3

[William Lachance (:wlach)]

(In reply to Bob Clary [:bc:] from comment #2)
> Comment on attachment 8428892 [details] [diff] [review]
> Add Android 2.x support to stopApplication
> 
> Review of attachment 8428892 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/mozbase/mozdevice/mozdevice/droid.py
> @@ +141,5 @@
> > +                if num_tries > max_tries:
> > +                    raise DMError("Couldn't successfully kill %s after %s "
> > +                                  "tries" % (appName, max_tries))
> > +                self.killProcess(appName)
> > +                num_tries+=1
> 
> spaces

How do you mean?

> @@ +144,5 @@
> > +                self.killProcess(appName)
> > +                num_tries+=1
> > +                # sleep for a short duration to make sure there are no
> > +                # additional processes in the process of being launched
> > +                time.sleep(0.1)
> 
> How did you decide on 0.1 seconds?

It's pretty much arbitrary and thus probably flakey. I'll update the comment to admit this, and maybe lengthen the timeout to a second. Fortunately we only need to use this functionality on Android 2.3 and below.

Comment 4

[William Lachance (:wlach)]

Attachment: Updated patch

Updating patch, carrying forward r+

Comment 5

[Bob Clary [:bc] (inactive)]

(In reply to William Lachance (:wlach) from comment #3)

> > > +                num_tries+=1
> > 
> > spaces
> 
> How do you mean?

spaces around the operator? num_tries+=1 -> num_tries += 1

The new patch is fine if you are ok with the spaces.

Comment 6

[William Lachance (:wlach)]

Attachment: Update once more for spacing

Comment 7

[William Lachance (:wlach)]

Note for sheriffs; no need for try run as nothing in the tree actually uses this method (
http://mxr.mozilla.org/mozilla-central/search?string=stopApplication). I have tested this locally and it works well.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8dff41f0ca9c

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8dff41f0ca9c