Bug 909129 - stop leaking imported scripts and don't import duplicates, r=jgriffin
authorMalini Das <mdas@mozilla.com>
Tue, 08 Oct 2013 16:11:45 -0400
changeset 165407 4334336fae4d15862375d1e8addc0ca9f7b73306
parent 165317 4503553be6de13103b9661c6198a7fd9bf5334a3
child 165408 654ffd9d6b0904b2ddbe2b77e3003c36c2d0d4dc
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgriffin
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 - stop leaking imported scripts and don't import duplicates, r=jgriffin
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
@@ -1186,16 +1186,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()
@@ -1942,17 +1953,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);
@@ -1999,30 +2016,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();
@@ -2063,16 +2083,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)
@@ -2088,16 +2125,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",
                    {
@@ -2205,17 +2259,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);
@@ -2271,16 +2324,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,