668349 - add or update script to run xpcshell tests on Android

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Geoff Brown [:gbrown] (pto Apr 11-18)]

When running xpcshell tests, the command line options passed to xpcshell are often complex. On desktop, the script testing/xpcshell/runxpcshelltests.py can be used to generate the xpcshell command line and execute tests. A similar facility is required to run xpcshell tests on Android devices.

(It may be possible to leverage testing/xpcshell/remotexpcshelltests.py, developed for Windows Mobile.)

Comment 1

[cmtalbert]

(In reply to comment #0)
> When running xpcshell tests, the command line options passed to xpcshell are
> often complex. On desktop, the script testing/xpcshell/runxpcshelltests.py
> can be used to generate the xpcshell command line and execute tests. A
> similar facility is required to run xpcshell tests on Android devices.
> 
> (It may be possible to leverage testing/xpcshell/remotexpcshelltests.py,
> developed for Windows Mobile.)

Geoff,

It'd be awesome to have your help with updating the remotexpcshelltests.py.  You can follow the mechanism that we used in mochitest [1] and reftest[2] in order to easily update it to work with android.

It's been on our list for a while, but we've been helping get the rest of the buildbot automation to a reliable state for mochitest and reftest that xpcshell slipped off our list.

One nice thing about continuing to work with the remoted class structure is that it will make it easy to run the xpcshell tests in the buildbot automation once they are finished.

In general there are a few things that we'll need to change (from windows mobile):
* Process name cannot be hardcoded.  In the other harnesses we made it a command line option --app
* File names/paths cannot be depended to be writeable - we use the devicemanager::getDeviceRoot API to get to a writeable location on the phone  - if you are copying a profile or tests to the phone, create directories in that location.
* Feed devicemanager paths with forward slashes i.e. foo/bar/baz.  The underlying agents (ADB, SUTAgent, etc) are responsible for getting the slashes right on device.  (This was something we went back and forth about for windows mobile, so some of the slash swapping code might still be in the remoted xpcshell classes, if so feel free to remove it).

And if you have any other questions or if you need to ask what we were smoking when we first wrote that code, please feel free.  You can comment here or find Joel (jmaher) and I (ctalbert) in #mobile or #ateam on IRC.

[1] Mochitest remoting: http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsremote.py
[2] Reftest remoting: http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/remotereftest.py

Comment 2

[Geoff Brown [:gbrown] (pto Apr 11-18)]

I have started updating remotexpcshelltests.py. I tried using the same devicemanager selection found in the mochitest and reftest remoting and tested with devicemanagerADB, running ADB over USB. I found some problems in devicemanagerADB; I can probably work around these but filed bug 669549 anyway.

Comment 3

[Geoff Brown [:gbrown] (pto Apr 11-18)]

runxpcshelltests.py normally calls xpcshell "-g <xrePath>", where <xrePath> is typically the path to xpcshell. For the remote case, I think -g is unnecessary if I follow the run_xpctest.py (bug 661282) convention of setting LD_LIBRARY_PATH, GRE_HOME, and specifying --greomni; can anyone confirm?

Comment 4

[cmtalbert]

I think you're right.  From my read, the xrepath is replaced by the <objdir> parameter of the runxpcshell script.  The script is going to copy all your libraries into one directory, so you won't need xrepath.

Comment 5

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Can anyone tell me the cause of this error:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: jar:file:///data/local/tests/xpcshell/bin/fennec-8.0a1.en-US.eabi-arm.apk!/components/DirectoryProvider.js :: <TOP_LEVEL> :: line 60"  data: no]
************************************************************

I get this when I run xpcshell on Android with command line:


cd /data/local/tests/xpcshell/bin && LD_LIBRARY_PATH=/data/local/tests/xpcshell/bin && GRE_HOME=/data/data/org.mozilla.fennec_unofficial && XPCSHELL_TEST_PROFILE_DIR=/data/local/tests/xpcshell/profile && ./xpcshell -r /data/local/tests/xpcshell/components/httpd.manifest --greomni /data/local/tests/xpcshell/bin/fennec-8.0a1.en-US.eabi-arm.apk -j -s -e 'const _HTTPD_JS_PATH = "/data/local/tests/xpcshell/components/httpd.js";' -e 'const _HEAD_JS_PATH = "/data/local/tests/xpcshell/scripts/head.js";' -f /data/local/tests/xpcshell/scripts/head.js -e 'const _SERVER_ADDR = "localhost"' -e 'const _HEAD_FILES = ["/data/local/tests/xpcshell/scripts/unit/head_channels.js"];' -e 'const _TAIL_FILES = ["/data/local/tests/xpcshell/scripts/unit/"];' -e 'const _TEST_FILE = ["/data/local/tests/xpcshell/scripts/unit/test_307_redirect.js"];' -e '_execute_test(); quit(0);'

Comment 6

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: Log showing xpcshell output on device

Comment 7

[Josh Matthews [:jdm]]

That comes from someone trying to make use of the DirectoryProvider (http://mxr.mozilla.org/mozilla-central/source/mobile/components/DirectoryProvider.js) in the content process to get the APP_CACHE_PARENT_DIR, which tries to find out the profile dir and is disallowed in non-chrome processes.

Comment 8

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Thanks Josh.

Even simpler:

js> Components.classes["@mozilla.org/file/directory_service;1"].getService( Components.interfaces.nsIProperties).get("ProfD", Components.interfaces.nsIFile).path
uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: typein :: <TOP_LEVEL> :: line 2"  data: no]

...and other tests are giving me different errors, all pointing to ProfD not being set. But I have set the XPCSHELL_TEST_PROFILE_DIR environment variable. Looking into it...

Comment 9

[Geoff Brown [:gbrown] (pto Apr 11-18)]

testing/xpcshell/head.js, fn do_get_profile sets ProfD from XPCSHELL_TEST_PROFILE_DIR, but do_get_profile is not being called.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

I thought on windows mobile we had to hack this into the xpcshell binary during make time if --enable-tests was specified.

Comment 11

[Geoff Brown [:gbrown] (pto Apr 11-18)]

(In reply to comment #10)
I did have some "build irregularities" and --disable-tests may have been in use for some of the problem tests!

Comment 12

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: work in progress patch

With this patch, I can run tests with something like:
 
cd ~/src/mozilla-central/objdir-droid/netwerk/test
/usr/bin/python2.6 -u ../../../config/pythonpath.py -I../../../build -I../../build -I../../../build/mobile ../../../testing/xpcshell/remotexpcshelltests.py --symbols-path=../../dist/crashreporter-symbols --build-info-json=../../mozinfo.json --dm_trans=adb --test-path=test_simple.js --objdir=~/src/mozilla-central/objdir-droid --apk=~/src/mozilla-central/objdir-droid/dist/fennec-8.0a1.en-US.eabi-arm.apk ../../_tests/xpcshell/netwerk/test/unit ../../_tests/xpcshell/netwerk/test/unit_ipc

The test passes and output is very similar to make SOLO_FILE=test_simple.js -C netwerk/test check-one.

However, other tests fail -- cause(s) still under investigation.

Comment 13

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: work in progress patch

With these changes, I can run most tests successfully. I have commented out half a dozen or so tests from unit/xpcshell.ini and there are an additional 8 tests that fail, but otherwise, it looks good:

INFO | Result summary:
INFO | Passed: 132
INFO | Failed: 8
INFO | Todo: 0

Comment 14

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: log showing output for network tests

This shows the output from:
cd ~/src/mozilla-central/objdir-droid/netwerk/test
/usr/bin/python2.6 -u ../../../config/pythonpath.py -I../../../build -I../../build -I../../../build/mobile ../../../testing/xpcshell/remotexpcshelltests.py --symbols-path=../../dist/crashreporter-symbols --build-info-json=../../mozinfo.json --dm_trans=adb ../../_tests/xpcshell/netwerk/test/unit ../../_tests/xpcshell/netwerk/test/unit_ipc --objdir=../.. --apk=../../dist/fennec-8.0a1.en-US.eabi-arm.apk &> runall.log

Comment 15

[Brad Lassey [:blassey] (use needinfo?)]

This is great stuff Geoff. Rather than commenting out the tests, you should be able to use 'skip-if os == "android"'. Let's just skip what doesn't run or fails for now, get this checked in and file follow up bugs for what gets skipped.

Comment 16

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: patch for review

I found and fixed some problems in the earlier patch (not all files were being copied, not running in correct directory). I also discovered that the unit_ipc tests were not running, and that they hang if they are run. Now all unit_ipc and a few unit tests are skip-if'd.

Comment 17

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: log showing output for network tests

Log showing output from:

cd ~/src/mozilla-central/objdir-droid/netwerk/test
/usr/bin/python2.6 -u ../../../config/pythonpath.py -I../../../build -I../../../build/mobile ../../../testing/xpcshell/remotexpcshelltests.py --symbols-path=../../dist/crashreporter-symbols --build-info-json=../../mozinfo.json --dm_trans=adb ../../_tests/xpcshell/netwerk/test/unit ../../_tests/xpcshell/netwerk/test/unit_ipc --objdir=../.. --apk=../../dist/fennec-8.0a1.en-US.eabi-arm.apk

Now the overall results are:

INFO | Result summary:
INFO | Passed: 117
INFO | Failed: 13
INFO | Todo: 0

Comment 18

[(not currently active) Ted Mielczarek]

(In reply to comment #15)
> This is great stuff Geoff. Rather than commenting out the tests, you should
> be able to use 'skip-if os == "android"'. Let's just skip what doesn't run
> or fails for now, get this checked in and file follow up bugs for what gets
> skipped.

If something is hanging or crashing, you can skip it. If something is just failing, please mark it fails-if = os == "android", so we still run the tests. (Minor difference, but it'd help if we introduced a new crash, say.)

Comment 19

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 548331 [details] [diff] [review]
patch for review

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

I ended up doing this:
mkdir ${xpcshell_tests}
cd ${xpcshell_tests}
wget fennec-8.0a1.multi.eabi-arm.tests.zip
unzip fennec-8.0a1.multi.eabi-arm.tests.zip
mkdir fennec
cd fennec
wget fennec-8.0a1.multi.eabi-arm.apk
unzip fennec-8.0a1.multi.eabi-arm.apk
cd ../xpcshell
python remotexpcshelltests.py --build-info-json=mozinfo.json --dm_trans=adb --apk=${xpcshell_tests}/fennec/fennec-8.0a1.multi.eabi-arm.apk --objdir=${xpcshell_tests} tests/netwerk/test/unit

this was using /data/local/tests and setting packageName='' on an unrooted NexusS.

We should fix a lot of these little things and I will review again.

::: build/mobile/devicemanagerADB.py
@@ +30,5 @@
> +      files = self.listFiles("/data/data")
> +      if (len(files) == 1):
> +        if (files[0].find("Permission denied") != -1):
> +          print "NOT running as root"
> +          raise Exception("not running as root")

do we want to raise an exception here?  Is root required?

::: netwerk/test/unit_ipc/xpcshell.ini
@@ +43,2 @@
>  [test_xmlhttprequest_wrap.js]
> +skip-if = os == "android"

instead of skip-if on all of these conditions, we can do it in the master xpcshell.ini for the whole include.  Unless you think we will be fixing these tests one at a time in the near future?

::: testing/xpcshell/remotexpcshelltests.py
@@ +89,3 @@
>  
> +        local = os.path.join(objdir, "dist/bin/xpcshell")
> +        self.device.pushFile(local, self.remoteBinDir)

I found that in a packaged environment, we needed 'objdir/bin/xpcshell', so we should remove the hardcoded dist for now and append it to objdir if it exists (os.path.exists(os.path.join(objdir, 'dist'))

@@ +104,5 @@
> +        local = os.path.join(objdir, "dist/fennec")
> +        for file in os.listdir(local):
> +          if (file.endswith(".so")):
> +            self.device.pushFile(os.path.join(local, file), self.remoteBinDir)
> +

in a packaged build (unzip fennec.apk), we have the .so files in lib/ and libmozutils.js lives in lib/armeabi-v7a.  I am not sure of the reason for putting libmozutils.js in a different directory, but we should put some logic in here to handle an unpacked directory.  I think it is safe to assume objdir/fennec/lib/libmoz*.so

@@ +114,1 @@
>  

this is great for a single directory of tests, but I think it could be a lengthy setup process for all tests.  Maybe we can just move this to a different function and only copy the current directory of tests that we are running?

I am leaning towards a setupUtilities() and a setupTestDir().

@@ +166,5 @@
> +        shellArgs += " && GRE_HOME="+self.device.getAppRoot()
> +        shellArgs += " && XPCSHELL_TEST_PROFILE_DIR="+self.profileDir
> +        shellArgs += " && "+xpcshell+" "
> +        shellArgs += " ".join(cmd[1:])
> +

this is not going to work in automation, but the goal of landing this is for developer tests.  I will refactor this into various commands (cd, xpcshell) instead of a && command.  For sut agent, we do this:

['cd dir', '"LD_LIBRARY_PATH=dir;GRE_HOME=root;..." xpcshell args'].  I think it would be fair to ask that we setup a dict of env variables and do something like:
for var in envvars:
  shellArgs += " && %s=%s" % (var, envvars[var])

Then I think the rest of the changes needed for automation can be refactored into the runCmd() function.

Comment 20

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: updated patch - unit_ipc tests run now

Improved with:
 - copies plugin-container to device
 - sets CACHE_PATH
 - exports environment variables
 - fail-if added to failing tests in xpcshell.ini
 - fail-if and skip-if commented with bug numbers

Network test results now:
INFO | Result summary:
INFO | Passed: 139
INFO | Failed: 0
INFO | Todo: 4

But I haven't worked through all the review comments yet...

Comment 21

[Geoff Brown [:gbrown] (pto Apr 11-18)]

I have been trying to run the "global" set of xpcshell tests: make xpcshell-tests-remote from $objdir; this is the 1200 or so tests in all the manifests referenced by testing/xpcshell/xpcshell.ini. Currently about 100 tests hang and about 500 tests fail. I notice that many failures are related to missing files. For example: toolkit/components/places/tests/expiration/head_expiration.js references toolkit/components/places/tests/head_common.js, but head_common.js is not copied to device, because toolkit/components/places/tests does not contain any tests. 

The current setup strategy is to push every directory containing a test, as determined by the manifest reader...but that is not good enough if tests reference files in other directories. I thought I would try to push the entire $objdir/_tests/xpcshell directory, but this is also problematic since $objdir/_tests/xpcshell/tests is a symbolic link to $objdir/..! (And we have other cases where tests fail if symbolic links are not followed and pushed.)

Comment 22

[Joel Maher ( :jmaher ) (UTC -8)]

you could require a 'make package-tests' in order to run these.  That would ensure all the symlinks are resolved and related files are all in one directory.

Comment 23

[Geoff Brown [:gbrown] (pto Apr 11-18)]

I found that 'make package-tests' also failed: It went into an infinite loop of copying, again because of the symbolic link from $objdir/_tests/xpcshell/tests  to $objdir/..; the symbolic link was being created by my makefile patch (bug 668351), xpcshell-tests-remote target, which was copied from the reftest-remote target:

reftest-remote:
        @if test -f ${MOZ_HOST_BIN}/xpcshell && [ "${TEST_DEVICE}" != "" -o "$(DM_TRANS)" = "adb" ]; \
          then ln -s $(abspath $(topsrcdir)) _tests/reftest/tests;$(call REMOTE_REFTEST,tests/$(TEST_PATH)); $(CHECK_TEST_ERROR); \

Why do we do this?

Comment 24

[Geoff Brown [:gbrown] (pto Apr 11-18)]

I found that adb has a 1024 character limit to its arguments. That is, if len("shell " + args) > 1024, adb complains "service name too long" and does not execute the command. This is somewhat verified here: http://groups.google.com/group/android-developers/browse_thread/thread/6319be6a93f1efde#.

The current patch exceeds this limit for xpcshell tests in deep sub-directories. I will shorten path names where possible to reduce the length of the adb shell command used to launch the xpcshell binary. Also, I will add a warning to remotexpcshelltests.py, to be reported when the command length approaches 1024 characters.

Comment 25

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: updated patch for review

Patch updated based on review, with additional improvements for more successful test runs.

>::: build/mobile/devicemanagerADB.py
>@@ +30,5 @@
>> +      files = self.listFiles("/data/data")
>> +      if (len(files) == 1):
>> +        if (files[0].find("Permission denied") != -1):
>> +          print "NOT running as root"
>> +          raise Exception("not running as root")
>
>do we want to raise an exception here?  Is root required?

Root is not required for remote xpcshell, but this is in devicemanager, so root might be required in some cases. Note that the raise is inside an existing try/except; we just run "adb root" if it looks like we are not already running as root (and if "adb root" fails, we just warn about it and continue anyway).

> instead of skip-if on all of these conditions, we can do it in the master 
> xpcshell.ini for the whole include.  Unless you think we will be fixing these
> tests one at a time in the near future?

Thanks for the tip - now I have used this technique a few times, whenever there were a large number of tests failing in the same manifest.

> ::: testing/xpcshell/remotexpcshelltests.py
> @@ +89,3 @@
>>  
>> +        local = os.path.join(objdir, "dist/bin/xpcshell")
>> +        self.device.pushFile(local, self.remoteBinDir)
>
> I found that in a packaged environment, we needed 'objdir/bin/xpcshell', so 
> we should remove the hardcoded dist for now and append it to objdir if it
> exists (os.path.exists(os.path.join(objdir, 'dist'))

Done. (And similar allowance made for lib.)

@@ +114,1 @@
>  

> this is great for a single directory of tests, but I think it could be a
> lengthy setup process for all tests.  Maybe we can just move this to a
> different function and only copy the current directory of tests that we are
> running?
> 
> I am leaning towards a setupUtilities() and a setupTestDir().

Setup is definitely time consuming. I have split out setupUtilities/TestDir, as suggested, but currently, both are called unless the --noSetup argument is passed. Optimization is significantly complicated by the issues raised in comments 21 - 23: If tests reference files in other directories, we really need to push the whole $objdir/_tests/xpcshell directory and operate within the remote version of that, even if just one test is being run.

>@@ +166,5 @@
>> +        shellArgs += " && GRE_HOME="+self.device.getAppRoot()
>> +        shellArgs += " && XPCSHELL_TEST_PROFILE_DIR="+self.profileDir
>> +        shellArgs += " && "+xpcshell+" "
>> +        shellArgs += " ".join(cmd[1:])
>> +
>
> this is not going to work in automation, but the goal of landing this is for
> developer tests.  I will refactor this into various commands (cd, xpcshell)
> instead of a && command.  For sut agent, we do this:
>
> ['cd dir', '"LD_LIBRARY_PATH=dir;GRE_HOME=root;..." xpcshell args'].  I think
> it would be fair to ask that we setup a dict of env variables and do something
> like:
> for var in envvars:
>  shellArgs += " && %s=%s" % (var, envvars[var])
>
> Then I think the rest of the changes needed for automation can be refactored 
> into the runCmd() function.

I have made some minor changes to this but did not want to get bogged down by  the potential SUT agent / automation details...probably best if I leave that to the experts!

Comment 26

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: patch adding fail-if / skip-if "android" directives to all the xpcshell manifests

There are many tests that hang and some that fail, on Android. With this patch, "make xpcshell-tests-remote" runs to completion and ends cleanly with:

INFO | Result summary:
INFO | Passed: 837
INFO | Failed: 0
INFO | Todo: 49

Comment 27

[Geoff Brown [:gbrown] (pto Apr 11-18)]

For the benefit of others trying to run these tests, here are notes about my environment:

 - apply both patches for this bug, 668349
 - apply patch for bug 668351
 - apply follow-up patch for bug 661282
 - normal Linux/Android build environment
 - normal Android make with objdir .../objdir-droid
 - cd .../objdir-droid; make package
 - ensure adb is in your $PATH
 - ensure supported Android device is connected (I used a Samsung Galaxy S)
 - I used a rooted device, but this should not be necessary
 - make directory /data/local/tests on the device:
     adb shell
     $ cd /data/local
     $ mkdir tests
 - cd .../objdir-droid; make xpcshell-tests-remote
   OR make -C netwerk/test xpcshell-tests-remote
   OR make SOLO_FILE=test_simple.js -C netwerk/test check-one-remote

Setup takes a long time (over 30 minutes!); progress messages are displayed every few seconds.

Comment 28

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 551801 [details] [diff] [review]
updated patch for review

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

this looks good.  I don't see anything specific in here to stop us from getting this running for developers!  Thanks for getting these tests up and going!

Comment 29

[Geoff Brown [:gbrown] (pto Apr 11-18)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/40e392d958e0
http://hg.mozilla.org/integration/mozilla-inbound/rev/187ac0d7b154

Comment 30

[Brad Lassey [:blassey] (use needinfo?)]

backed out the pushes because of orange:
http://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d9456378c12d

Comment 31

[Geoff Brown [:gbrown] (pto Apr 11-18)]

One problem was a typo:

--- a/ipc/testshell/tests/xpcshell.ini
+++ b/ipc/testshell/tests/xpcshell.ini
@@ -1,6 +1,8 @@
+# Bug 676963: test fails consistently on Android
+fail-if = os = "android"
              ^ should be ==

Comment 32

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: update patch

Updated from reviewed version with just a couple of tweaks:
 - if the --apk argument is not specified, try searching $objdir for a likely APK
 - update devicemanagerADB's getAppRoot to allow for org.mozilla.fennec_$USER

Comment 33

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: updated patch

Oops - the getAppRoot change is handled in bug 678385; I have removed it from this patch.

Comment 34

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: updated patch

updated for bitrot only

Comment 35

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Attachment: patch adding fail-if / skip-if "android" directives to all the xpcshell manifests

updated for bitrot only

Comment 36

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Comment on attachment 553856 [details] [diff] [review]
updated patch

r=jmaher

Comment 37

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Comment on attachment 553857 [details] [diff] [review]
patch adding fail-if / skip-if "android" directives to all the xpcshell manifests

r=jmaher

Comment 38

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Pushed to try -- looks good to me.

http://hg.mozilla.org/try/pushloghtml?changeset=ff5c3f0b0f6a
http://tbpl.mozilla.org/?tree=Try&rev=ff5c3f0b0f6a

Comment 39

[Ed Morley [:emorley]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/925a6fb68f91
http://hg.mozilla.org/integration/mozilla-inbound/rev/4512e937ac29

Comment 40

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/4512e937ac29
http://hg.mozilla.org/mozilla-central/rev/925a6fb68f91