Bug 909129 - stop leaking imported scripts and don't import duplicates, r=jgriffin, a=test-only
☠☠ backed out by 88fc2da6856b ☠ ☠
authorMalini Das <mdas@mozilla.com>
Tue, 08 Oct 2013 16:11:45 -0400
changeset 160814 ec442fac77c552085a95948ee6ffe3d031d0640f
parent 160813 0bbc34014227310301e7f55f7d36ce4ded9219e7
child 160815 ed279034d0661e7886c3abcb21e04e5758f25ce6
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgriffin, test-only
bugs909129
milestone26.0a2
Bug 909129 - stop leaking imported scripts and don't import duplicates, r=jgriffin, a=test-only
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
@@ -1175,16 +1175,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,30 @@ 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("""
+          let FileUtils = Components.utils.import("resource://gre/modules/FileUtils.jsm", {}).FileUtils;
+          let importedScripts = FileUtils.getFile('TmpD', ['marionetteChromeScripts']); 
+          marionetteScriptFinished(importedScripts.exists());
+        """)
+
+    def get_file_size(self):
+        return self.marionette.execute_async_script("""
+          let FileUtils = Components.utils.import("resource://gre/modules/FileUtils.jsm", {}).FileUtils;
+          let importedScripts = FileUtils.getFile('TmpD', ['marionetteChromeScripts']); 
+          marionetteScriptFinished(importedScripts.fileSize);
+        """)
+
--- a/testing/marionette/marionette-listener.js
+++ b/testing/marionette/marionette-listener.js
@@ -93,17 +93,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()
@@ -1926,17 +1937,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);
@@ -1983,30 +2000,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();
@@ -2047,16 +2067,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.parameters.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)
@@ -2072,16 +2109,33 @@ MarionetteServerConnection.prototype = {
     }
     else {
       this.sendAsync("importScript",
                      { script: aRequest.parameters.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",
                    {
@@ -2189,17 +2243,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);
@@ -2254,16 +2307,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,