Use resource manager in gecko_driver and nits

etc/ci/performance/gecko_driver.py

@@ -4,75 +4,94 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+from contextlib import contextmanager
+import json
 from selenium import webdriver
 from selenium.common.exceptions import TimeoutException
 
 
-def run_gecko_test(testcase, timeout):
+@contextmanager
+def create_gecko_session():
     firefox_binary = "./firefox/firefox/firefox"
     driver = webdriver.Firefox(firefox_binary=firefox_binary)
-    driver.set_page_load_timeout(timeout)
+    yield driver
-    failed = False
-    try:
-        driver.get(testcase)
-    except TimeoutException:
-        print("Timeout!")
-        driver.quit()
-        failed = True
-
-    timings = {
-        "testcase": testcase,
-        "title": driver.title.replace(",", ",")
-    }
-
-    timing_names = [
-        "navigationStart",
-        "unloadEventStart",
-        "domLoading",
-        "fetchStart",
-        "responseStart",
-        "loadEventEnd",
-        "connectStart",
-        "domainLookupStart",
-        "redirectStart",
-        "domContentLoadedEventEnd",
-        "requestStart",
-        "secureConnectionStart",
-        "connectEnd",
-        "loadEventStart",
-        "domInteractive",
-        "domContentLoadedEventStart",
-        "redirectEnd",
-        "domainLookupEnd",
-        "unloadEventEnd",
-        "responseEnd",
-        "domComplete",
-    ]
-
-    # We could have just run
-    #     timings = driver.execute_script("return performance.timing")
-    # But for some test case the webdriver will fail with "too much recursion"
-    # So we need to get the timing fields one by one
-    for name in timing_names:
-        if failed:
-            # We need to still include the failed tests, otherwise Treeherder will
-            # consider the result to be a new test series, and thus a new graph. So we
-            # use a placeholder with values = -1 to make Treeherder happy, and still be
-            # able to identify failed tests (successful tests have time >=0).
-            timings[name] = 0 if name == "navigationStart" else -1
-        else:
-            timings[name] = driver.execute_script(
-                "return performance.timing.{0}".format(name)
-            )
-
     # driver.quit() gives an "'NoneType' object has no attribute 'path'" error.
     # Fixed in
     # https://github.com/SeleniumHQ/selenium/commit/9157c7071f9900c2608f5ca40ae4f518ed373b96
     driver.quit()
+
+
+def generate_placeholder(testcase):
+        # We need to still include the failed tests, otherwise Treeherder will
+        # consider the result to be a new test series, and thus a new graph. So we
+        # use a placeholder with values = -1 to make Treeherder happy, and still be
+        # able to identify failed tests (successful tests have time >=0).
+
+        timings = {
+            "testcase": testcase,
+            "title": ""
+        }
+
+        timing_names = [
+            "navigationStart",
+            "unloadEventStart",
+            "domLoading",
+            "fetchStart",
+            "responseStart",
+            "loadEventEnd",
+            "connectStart",
+            "domainLookupStart",
+            "redirectStart",
+            "domContentLoadedEventEnd",
+            "requestStart",
+            "secureConnectionStart",
+            "connectEnd",
+            "loadEventStart",
+            "domInteractive",
+            "domContentLoadedEventStart",
+            "redirectEnd",
+            "domainLookupEnd",
+            "unloadEventEnd",
+            "responseEnd",
+            "domComplete",
+        ]
+
+        for name in timing_names:
+            timings[name] = 0 if name == "navigationStart" else -1
+
+        return [timings]
+
+
+def run_gecko_test(testcase, timeout):
+    with create_gecko_session() as driver:
+        driver.set_page_load_timeout(timeout)
+        try:
+            driver.get(testcase)
+        except TimeoutException:
+            print("Timeout!")
+            return generate_placeholder(testcase)
+
+        try:
+            timings = {
+                "testcase": testcase,
+                "title": driver.title.replace(",", ",")
+            }
+
+            timings.update(json.loads(
+                driver.execute_script(
+                    "return JSON.stringify(performance.timing)"
+                )
+            ))
+        except:
+            # We need to return a timing object no matter what happened.
+            # See the comment in generate_placeholder() for explanation
+            print("Failed to get a valid timing measurement.")
+            return generate_placeholder(testcase)
+
     return [timings]
 
 if __name__ == '__main__':
     # Just for manual testing
     from pprint import pprint
     url = "http://localhost:8000/page_load_test/tp5n/dailymail.co.uk/www.dailymail.co.uk/ushome/index.html"
-    pprint(execute_gecko_test(url, 15))
+    pprint(run_gecko_test(url, 15))

etc/ci/performance/runner.py

@@ -251,7 +251,7 @@ def main():
     if args.engine == 'servo':
         run_test = run_servo_test
     elif args.engine == 'gecko':
-        import gecko_driver # Load this only when we need gecko test
+        import gecko_driver  # Load this only when we need gecko test
         run_test = gecko_driver.run_gecko_test
     try:
         # Assume the server is up and running