Bug 909129 - fix leaking imported scripts from marionette, r=dburns
☠☠ backed out by c70064cbf84c ☠ ☠
authorMalini Das <mdas@mozilla.com>
Tue, 08 Oct 2013 16:11:45 -0400
changeset 150113 8ecb68f17618e7c213c20188828b3692d6e573d7
parent 150112 94fe8b4a15665375bb57693d48be572b8b0eba7a
child 150114 17ac833f18bfd493db600bf447a4cef5e676860a
push id34754
push usermdas@mozilla.com
push dateTue, 08 Oct 2013 20:12:10 +0000
treeherdermozilla-inbound@8ecb68f17618 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdburns
bugs909129
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 909129 - fix leaking imported scripts from marionette, r=dburns
testing/marionette/client/marionette/marionette.py
testing/marionette/client/marionette/tests/unit/test_import_script.py
testing/marionette/marionette-listener.js
testing/marionette/marionette-server.js
--- a/testing/marionette/client/marionette/marionette.py
+++ b/testing/marionette/client/marionette/marionette.py
@@ -1181,16 +1181,24 @@ class Marionette(object):
           marionette.import_script(js)
           assert "i'm a test function!" == self.marionette.execute_script("return testFunc();")
         '''
         js = ''
         with open(js_file, 'r') as f:
             js = f.read()
         return self._send_message('importScript', 'ok', script=js)
 
+    def clear_imported_scripts(self):
+        '''
+        Clears all imported scripts in this context, ie: calling clear_imported_scripts in chrome
+        context will clear only scripts you imported in chrome, and will leave the scripts
+        you imported in content context.
+        '''
+        return self._send_message('clearImportedScripts', 'ok')
+
     def add_cookie(self, cookie):
         """
         Adds a cookie to your current session.
 
         :param cookie: A dictionary object, with required keys - "name" and
          "value"; optional keys - "path", "domain", "secure", "expiry".
 
         Usage example:
--- a/testing/marionette/client/marionette/tests/unit/test_import_script.py
+++ b/testing/marionette/client/marionette/tests/unit/test_import_script.py
@@ -1,23 +1,110 @@
-
 # This Source Code Form is subject to the terms of the Mozilla Public
 # 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/.
 
 import os
 from marionette_test import MarionetteTestCase
+from errors import JavascriptException
 
 class TestImportScript(MarionetteTestCase):
+    def setUp(self):
+        MarionetteTestCase.setUp(self)
+
+    def clear_other_context(self):
+        self.marionette.set_context("chrome")
+        self.marionette.clear_imported_scripts()
+        self.marionette.set_context("content")
+
+    def check_file_exists(self):
+        return self.marionette.execute_script("""
+          let FileUtils = SpecialPowers.Cu.import("resource://gre/modules/FileUtils.jsm").FileUtils;
+          let importedScripts = FileUtils.getFile('TmpD', ['marionetteContentScripts']); 
+          return importedScripts.exists();
+        """, special_powers=True)
+
+    def get_file_size(self):
+        return self.marionette.execute_script("""
+          let FileUtils = SpecialPowers.Cu.import("resource://gre/modules/FileUtils.jsm").FileUtils;
+          let importedScripts = FileUtils.getFile('TmpD', ['marionetteContentScripts']); 
+          return importedScripts.fileSize;
+        """, special_powers=True)
+
     def test_import_script(self):
         js = os.path.abspath(os.path.join(__file__, os.path.pardir, "importscript.js"))
         self.marionette.import_script(js)
         self.assertEqual("i'm a test function!", self.marionette.execute_script("return testFunc();"))
         self.assertEqual("i'm a test function!", self.marionette.execute_async_script("marionetteScriptFinished(testFunc());"))
 
+    def test_import_script_twice(self):
+        js = os.path.abspath(os.path.join(__file__, os.path.pardir, "importscript.js"))
+        self.marionette.import_script(js)
+        self.assertEqual("i'm a test function!", self.marionette.execute_script("return testFunc();"))
+        self.assertEqual("i'm a test function!", self.marionette.execute_async_script("marionetteScriptFinished(testFunc());"))
+        self.assertTrue(self.check_file_exists())
+        file_size = self.get_file_size()
+        self.assertNotEqual(file_size, None)
+        self.marionette.import_script(js)
+        file_size = self.get_file_size()
+        self.assertEqual(file_size, self.get_file_size())
+        self.assertEqual("i'm a test function!", self.marionette.execute_script("return testFunc();"))
+        self.assertEqual("i'm a test function!", self.marionette.execute_async_script("marionetteScriptFinished(testFunc());"))
+
+    def test_import_two_scripts_twice(self):
+        js = os.path.abspath(os.path.join(__file__, os.path.pardir, "importscript.js"))
+        self.marionette.import_script(js)
+        self.assertEqual("i'm a test function!", self.marionette.execute_script("return testFunc();"))
+        self.assertEqual("i'm a test function!", self.marionette.execute_async_script("marionetteScriptFinished(testFunc());"))
+        self.assertTrue(self.check_file_exists())
+        file_size = self.get_file_size()
+        self.assertNotEqual(file_size, None)
+        self.marionette.import_script(js)
+        # same script should not append to file
+        self.assertEqual(file_size, self.get_file_size())
+        self.assertEqual("i'm a test function!", self.marionette.execute_script("return testFunc();"))
+        self.assertEqual("i'm a test function!", self.marionette.execute_async_script("marionetteScriptFinished(testFunc());"))
+        js = os.path.abspath(os.path.join(__file__, os.path.pardir, "importanotherscript.js"))
+        self.marionette.import_script(js)
+        new_size = self.get_file_size()
+        # new script should append to file
+        self.assertNotEqual(file_size, new_size)
+        file_size = new_size
+        self.assertEqual("i'm yet another test function!",
+                    self.marionette.execute_script("return testAnotherFunc();"))
+        self.assertEqual("i'm yet another test function!",
+                    self.marionette.execute_async_script("marionetteScriptFinished(testAnotherFunc());"))
+        self.marionette.import_script(js)
+        # same script should not append to file
+        self.assertEqual(file_size, self.get_file_size())
+
+    def test_import_script_and_clear(self):
+        js = os.path.abspath(os.path.join(__file__, os.path.pardir, "importscript.js"))
+        self.marionette.import_script(js)
+        self.assertEqual("i'm a test function!", self.marionette.execute_script("return testFunc();"))
+        self.assertEqual("i'm a test function!", self.marionette.execute_async_script("marionetteScriptFinished(testFunc());"))
+        self.marionette.clear_imported_scripts()
+        self.assertFalse(self.check_file_exists())
+        self.assertRaises(JavascriptException, self.marionette.execute_script, "return testFunc();")
+        self.assertRaises(JavascriptException, self.marionette.execute_async_script, "marionetteScriptFinished(testFunc());")
+
+    def test_import_script_and_clear_in_chrome(self):
+        js = os.path.abspath(os.path.join(__file__, os.path.pardir, "importscript.js"))
+        self.marionette.import_script(js)
+        self.assertTrue(self.check_file_exists())
+        file_size = self.get_file_size()
+        self.assertEqual("i'm a test function!", self.marionette.execute_script("return testFunc();"))
+        self.assertEqual("i'm a test function!", self.marionette.execute_async_script("marionetteScriptFinished(testFunc());"))
+        self.clear_other_context()
+        # clearing other context's script file should not affect ours
+        self.assertTrue(self.check_file_exists())
+        self.assertEqual(file_size, self.get_file_size())
+        self.assertEqual("i'm a test function!", self.marionette.execute_script("return testFunc();"))
+        self.assertEqual("i'm a test function!", self.marionette.execute_async_script("marionetteScriptFinished(testFunc());"))
+
     def test_importing_another_script_and_check_they_append(self):
         firstjs = os.path.abspath(
                 os.path.join(__file__, os.path.pardir, "importscript.js"))
         secondjs = os.path.abspath(
                 os.path.join(__file__, os.path.pardir, "importanotherscript.js"))
 
         self.marionette.import_script(firstjs)
         self.marionette.import_script(secondjs)
@@ -26,9 +113,46 @@ class TestImportScript(MarionetteTestCas
                 self.marionette.execute_script("return testFunc();"))
 
         self.assertEqual("i'm yet another test function!",
                     self.marionette.execute_script("return testAnotherFunc();"))
 
 class TestImportScriptChrome(TestImportScript):
     def setUp(self):
         MarionetteTestCase.setUp(self)
+        self.marionette.set_script_timeout(30000)
         self.marionette.set_context("chrome")
+
+    def clear_other_context(self):
+        self.marionette.set_context("content")
+        self.marionette.clear_imported_scripts()
+        self.marionette.set_context("chrome")
+
+    def check_file_exists(self):
+        return self.marionette.execute_async_script("""
+          Components.utils.import("resource://gre/modules/FileUtils.jsm");
+          let checkTimer = Components.classes["@mozilla.org/timer;1"].createInstance(Components.interfaces.nsITimer);
+          let f = function() {
+            if (typeof FileUtils === 'undefined') {
+              checkTimer.initWithCallback(f, 100, Components.interfaces.nsITimer.TYPE_ONE_SHOT);
+              return;
+            }
+            let importedScripts = FileUtils.getFile('TmpD', ['marionetteChromeScripts']); 
+            marionetteScriptFinished(importedScripts.exists());
+          };
+          checkTimer.initWithCallback(f, 100, Components.interfaces.nsITimer.TYPE_ONE_SHOT);
+        """)
+
+    def get_file_size(self):
+        return self.marionette.execute_async_script("""
+          Components.utils.import("resource://gre/modules/FileUtils.jsm");
+          let checkTimer = Components.classes["@mozilla.org/timer;1"].createInstance(Components.interfaces.nsITimer);
+          let f = function() {
+            if (typeof FileUtils === 'undefined') {
+              checkTimer.initWithCallback(f, 100, Components.interfaces.nsITimer.TYPE_ONE_SHOT);
+              return;
+            }
+            let importedScripts = FileUtils.getFile('TmpD', ['marionetteChromeScripts']); 
+            marionetteScriptFinished(importedScripts.fileSize);
+          };
+          checkTimer.initWithCallback(f, 100, Components.interfaces.nsITimer.TYPE_ONE_SHOT);
+        """)
+
--- a/testing/marionette/marionette-listener.js
+++ b/testing/marionette/marionette-listener.js
@@ -91,17 +91,17 @@ let modalHandler = function() {
  * If the actor returns an ID, we start the listeners. Otherwise, nothing happens.
  */
 function registerSelf() {
   let msg = {value: winUtil.outerWindowID, href: content.location.href};
   let register = sendSyncMessage("Marionette:register", msg);
 
   if (register[0]) {
     listenerId = register[0].id;
-    importedScripts = FileUtils.File(register[0].importedScripts);
+    importedScripts = FileUtils.getFile('TmpD', ['marionetteContentScripts']);
     startListeners();
   }
 }
 
 /**
  * Add a message listener that's tied to our listenerId.
  */
 function addMessageListenerId(messageName, handler) {
--- a/testing/marionette/marionette-server.js
+++ b/testing/marionette/marionette-server.js
@@ -129,17 +129,18 @@ function MarionetteServerConnection(aPre
   this.pageTimeout = null;
   this.timer = null;
   this.inactivityTimer = null;
   this.heartbeatCallback = function () {}; // called by simpletest methods
   this.marionetteLog = new MarionetteLogObj();
   this.command_id = null;
   this.mainFrame = null; //topmost chrome frame
   this.curFrame = null; // chrome iframe that currently has focus
-  this.importedScripts = FileUtils.getFile('TmpD', ['marionettescriptchrome']);
+  this.importedScripts = FileUtils.getFile('TmpD', ['marionetteChromeScripts']);
+  this.importedScriptHashes = {"chrome" : [], "content": []};
   this.currentFrameElement = null;
   this.testName = null;
   this.mozBrowserClose = null;
 }
 
 MarionetteServerConnection.prototype = {
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIMessageListener,
@@ -461,16 +462,26 @@ MarionetteServerConnection.prototype = {
     }
   },
 
   getCommandId: function MDA_getCommandId() {
     return this.uuidGen.generateUUID().toString();
   },
 
   /**
+    * Given a file name, this will delete the file from the temp directory if it exists
+    */
+  deleteFile: function(filename) {
+    let file = FileUtils.getFile('TmpD', [filename.toString()]);
+    if (file.exists()) {
+      file.remove(true);
+    }
+  },
+
+  /**
    * Marionette API:
    *
    * All methods implementing a command from the client should create a
    * command_id, and then use this command_id in all messages exchanged with
    * the frame scripts and with responses sent to the client.  This prevents
    * commands and responses from getting out-of-sync, which can happen in
    * the case of execute_async calls that timeout and then later send a
    * response, and other situations.  See bug 779011. See setScriptTimeout()
@@ -1928,17 +1939,23 @@ MarionetteServerConnection.prototype = {
       let winEnum = this.getWinEnumerator();
       while (winEnum.hasMoreElements()) {
         numOpenWindows += 1;
         winEnum.getNext(); 
       }
 
       // if there is only 1 window left, delete the session
       if (numOpenWindows === 1){
-        this.sessionTearDown();
+        try {
+          this.sessionTearDown();
+        }
+        catch (e) {
+          this.sendError("Could not clear session", 500, e.name + ": " + e.message, command_id);
+          return;
+        }
         this.sendOk(command_id);
         return;
       }
 
       try{
         this.messageManager.removeDelayedFrameScript(FRAME_SCRIPT); 
         this.getCurrentWindow().close();
         this.sendOk(command_id);
@@ -1985,30 +2002,33 @@ MarionetteServerConnection.prototype = {
       this.curBrowser.frameManager.removeMessageManagerListeners(this.globalMessageManager);
     }
     this.switchToGlobalMessageManager();
     // reset frame to the top-most frame
     this.curFrame = null;
     if (this.mainFrame) {
       this.mainFrame.focus();
     }
-    try {
-      this.importedScripts.remove(false);
-    }
-    catch (e) {
-    }
+    this.deleteFile('marionetteChromeScripts');
+    this.deleteFile('marionetteContentScripts');
   },
 
   /**
    * Processes the 'deleteSession' request from the client by tearing down
    * the session and responding 'ok'.
    */
-  deleteSession: function MDA_sessionTearDown() {
+  deleteSession: function MDA_deleteSession() {
     let command_id = this.command_id = this.getCommandId();
-    this.sessionTearDown();
+    try {
+      this.sessionTearDown();
+    }
+    catch (e) {
+      this.sendError("Could not delete session", 500, e.name + ": " + e.message, command_id);
+      return;
+    }
     this.sendOk(command_id);
   },
 
   /**
    * Returns the current status of the Application Cache
    */
   getAppCacheStatus: function MDA_getAppCacheStatus(aRequest) {
     this.command_id = this.getCommandId();
@@ -2049,16 +2069,33 @@ MarionetteServerConnection.prototype = {
     catch(e) {
       this.sendError(e.message, e.code, e.stack, -1);
       return;
     }
   },
   
   importScript: function MDA_importScript(aRequest) {
     let command_id = this.command_id = this.getCommandId();
+    let converter =
+      Components.classes["@mozilla.org/intl/scriptableunicodeconverter"].
+          createInstance(Components.interfaces.nsIScriptableUnicodeConverter);
+    converter.charset = "UTF-8";
+    let result = {};
+    let data = converter.convertToByteArray(aRequest.script, result);
+    let ch = Components.classes["@mozilla.org/security/hash;1"]
+                       .createInstance(Components.interfaces.nsICryptoHash);
+    ch.init(ch.MD5);
+    ch.update(data, data.length);
+    let hash = ch.finish(true);
+    if (this.importedScriptHashes[this.context].indexOf(hash) > -1) {
+        //we have already imported this script
+        this.sendOk(command_id);
+        return;
+    }
+    this.importedScriptHashes[this.context].push(hash);
     if (this.context == "chrome") {
       let file;
       if (this.importedScripts.exists()) {
         file = FileUtils.openFileOutputStream(this.importedScripts,
             FileUtils.MODE_APPEND | FileUtils.MODE_WRONLY);
       }
       else {
         //Note: The permission bits here don't actually get set (bug 804563)
@@ -2074,16 +2111,33 @@ MarionetteServerConnection.prototype = {
     }
     else {
       this.sendAsync("importScript",
                      { script: aRequest.script },
                      command_id);
     }
   },
 
+  clearImportedScripts: function MDA_clearImportedScripts(aRequest) {
+    let command_id = this.command_id = this.getCommandId();
+    try {
+      if (this.context == "chrome") {
+        this.deleteFile('marionetteChromeScripts');
+      }
+      else {
+        this.deleteFile('marionetteContentScripts');
+      }
+    }
+    catch (e) {
+      this.sendError("Could not clear imported scripts", 500, e.name + ": " + e.message, command_id);
+      return;
+    }
+    this.sendOk(command_id);
+  },
+
   /**
    * Takes a screenshot of a DOM node. If there is no node given a screenshot
    * of the window will be taken.
    */
   screenShot: function MDA_saveScreenshot(aRequest) {
     this.command_id = this.getCommandId();
     this.sendAsync("screenShot",
                    {
@@ -2191,17 +2245,16 @@ MarionetteServerConnection.prototype = {
         }
         let reg = {};
         if (!browserType || browserType != "content") {
           //curBrowser holds all the registered frames in knownFrames
           reg.id = this.curBrowser.register(this.generateFrameId(message.json.value),
                                             listenerWindow);
         }
         this.curBrowser.elementManager.seenItems[reg.id] = Cu.getWeakReference(listenerWindow);
-        reg.importedScripts = this.importedScripts.path;
         if (nullPrevious && (this.curBrowser.curFrameId != null)) {
           if (!this.sendAsync("newSession",
                               { B2G: (appName == "B2G") },
                               this.newSessionCommandId)) {
             return;
           }
           if (this.curBrowser.newSession) {
             this.sendResponse(reg.id, this.newSessionCommandId);
@@ -2256,16 +2309,17 @@ MarionetteServerConnection.prototype.req
   "getWindow":  MarionetteServerConnection.prototype.getWindow,
   "getWindows":  MarionetteServerConnection.prototype.getWindows,
   "getActiveFrame": MarionetteServerConnection.prototype.getActiveFrame,
   "switchToFrame": MarionetteServerConnection.prototype.switchToFrame,
   "switchToWindow": MarionetteServerConnection.prototype.switchToWindow,
   "deleteSession": MarionetteServerConnection.prototype.deleteSession,
   "emulatorCmdResult": MarionetteServerConnection.prototype.emulatorCmdResult,
   "importScript": MarionetteServerConnection.prototype.importScript,
+  "clearImportedScripts": MarionetteServerConnection.prototype.clearImportedScripts,
   "getAppCacheStatus": MarionetteServerConnection.prototype.getAppCacheStatus,
   "closeWindow": MarionetteServerConnection.prototype.closeWindow,
   "setTestName": MarionetteServerConnection.prototype.setTestName,
   "screenShot": MarionetteServerConnection.prototype.screenShot,
   "addCookie": MarionetteServerConnection.prototype.addCookie,
   "getAllCookies": MarionetteServerConnection.prototype.getAllCookies,
   "deleteAllCookies": MarionetteServerConnection.prototype.deleteAllCookies,
   "deleteCookie": MarionetteServerConnection.prototype.deleteCookie,