Bug 1524687 - Part 0: Fix more dodgy component mocking code. r=froydnj
☠☠ backed out by 3b1b94e39795 ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Thu, 31 Jan 2019 12:27:15 -0800
changeset 458960 e96e61bd282f
parent 458959 d09c7dc93f20
child 458961 219b84a58f50
push id111908
push usermaglione.k@gmail.com
push dateThu, 14 Feb 2019 02:29:45 +0000
treeherdermozilla-inbound@625f71135038 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1524687
milestone67.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 1524687 - Part 0: Fix more dodgy component mocking code. r=froydnj Differential Revision: https://phabricator.services.mozilla.com/D18395
testing/mochitest/tests/SimpleTest/MockObjects.js
testing/specialpowers/content/specialpowersAPI.js
xpcom/components/nsComponentManager.cpp
--- a/testing/mochitest/tests/SimpleTest/MockObjects.js
+++ b/testing/mochitest/tests/SimpleTest/MockObjects.js
@@ -28,17 +28,17 @@ MockObjectRegisterer.prototype = {
   /**
    * Replaces the current factory with one that returns a new mock object.
    *
    * After register() has been called, it is mandatory to call unregister() to
    * restore the original component. Usually, you should use a try-catch block
    * to ensure that unregister() is called.
    */
   register: function MOR_register() {
-    if (this._originalFactory)
+    if (this._originalCID)
       throw new Exception("Invalid object state when calling register()");
 
     // Define a factory that creates a new object using the given constructor.
     var isChrome = location.protocol == "chrome:";
     var providedConstructor = this._replacementCtor;
     this._mockFactory = {
       createInstance: function MF_createInstance(aOuter, aIid) {
         if (aOuter != null)
@@ -51,45 +51,43 @@ MockObjectRegisterer.prototype = {
         }
         return inst.QueryInterface(aIid);
       }
     };
     if (!isChrome) {
       this._mockFactory = SpecialPowers.wrapCallbackObject(this._mockFactory);
     }
 
-    var retVal = SpecialPowers.swapFactoryRegistration(null, this._contractID, this._mockFactory, this._originalFactory);
+    var retVal = SpecialPowers.swapFactoryRegistration(null, this._contractID, this._mockFactory);
     if ('error' in retVal) {
       throw new Exception("ERROR: " + retVal.error);
-    } else if (!isChrome) {
-      this._originalFactory = SpecialPowers.wrap(retVal).originalFactory;
     } else {
-      this._originalFactory = retVal.originalFactory;
+      this._originalCID = retVal.originalCID;
     }
   },
 
   /**
    * Restores the original factory.
    */
   unregister: function MOR_unregister() {
-    if (!this._originalFactory)
+    if (!this._originalCID)
       throw new Exception("Invalid object state when calling unregister()");
 
     // Free references to the mock factory.
-    SpecialPowers.swapFactoryRegistration(null, this._contractID, this._originalFactory, this._mockFactory);
+    SpecialPowers.swapFactoryRegistration(this._originalCID, this._contractID);
 
     // Allow registering a mock factory again later.
-    this._originalFactory = null;
+    this._originalCID = null;
     this._mockFactory = null;
   },
 
   // --- Private methods and properties ---
 
   /**
    * The factory of the component being replaced.
    */
-  _originalFactory: null,
+  _originalCID: null,
 
   /**
    * The nsIFactory that was automatically generated by this object.
    */
   _mockFactory: null
 }
--- a/testing/specialpowers/content/specialpowersAPI.js
+++ b/testing/specialpowers/content/specialpowersAPI.js
@@ -1793,46 +1793,38 @@ SpecialPowersAPI.prototype = {
       getService(Ci.nsIClipboardHelper).
       copyString(str);
   },
 
   supportsSelectionClipboard() {
     return Services.clipboard.supportsSelectionClipboard();
   },
 
-  swapFactoryRegistration(cid, contractID, newFactory, oldFactory) {
+  swapFactoryRegistration(cid, contractID, newFactory) {
     newFactory = Cu.waiveXrays(newFactory);
-    oldFactory = Cu.waiveXrays(oldFactory);
 
     var componentRegistrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
 
-    var unregisterFactory = newFactory;
-    var registerFactory = oldFactory;
-
-    if (cid == null) {
-      if (contractID != null) {
-        cid = componentRegistrar.contractIDToCID(contractID);
-        oldFactory = Components.manager.getClassObject(Cc[contractID],
-                                                            Ci.nsIFactory);
-      } else {
-        return {"error": "trying to register a new contract ID: Missing contractID"};
-      }
-
-      unregisterFactory = oldFactory;
-      registerFactory = newFactory;
+    var currentCID = componentRegistrar.contractIDToCID(contractID);
+    var currentFactory = Components.manager.getClassObject(Cc[contractID],
+                                                           Ci.nsIFactory);
+    if (cid) {
+      componentRegistrar.unregisterFactory(currentCID, currentFactory);
+    } else {
+      let uuidGenerator = Cc["@mozilla.org/uuid-generator;1"]
+                            .getService(Ci.nsIUUIDGenerator);
+      cid = uuidGenerator.generateUUID();
     }
-    componentRegistrar.unregisterFactory(cid,
-                                         unregisterFactory);
 
     // Restore the original factory.
     componentRegistrar.registerFactory(cid,
                                        "",
                                        contractID,
-                                       registerFactory);
-    return {"cid": cid, "originalFactory": oldFactory};
+                                       newFactory);
+    return {"originalCID": currentCID};
   },
 
   _getElement(aWindow, id) {
     return ((typeof(id) == "string") ?
         aWindow.document.getElementById(id) : id);
   },
 
   dispatchEvent(aWindow, target, event) {
--- a/xpcom/components/nsComponentManager.cpp
+++ b/xpcom/components/nsComponentManager.cpp
@@ -1589,28 +1589,30 @@ nsComponentManagerImpl::RegisterFactory(
                                         nsIFactory* aFactory) {
   if (!aFactory) {
     // If a null factory is passed in, this call just wants to reset
     // the contract ID to point to an existing CID entry.
     if (!aContractID) {
       return NS_ERROR_INVALID_ARG;
     }
 
+    nsDependentCString contractID(aContractID);
+
     SafeMutexAutoLock lock(mLock);
     nsFactoryEntry* oldf = mFactories.Get(&aClass);
     if (oldf) {
-      mContractIDs.Put(nsDependentCString(aContractID), oldf);
+      StaticComponents::InvalidateContractID(contractID);
+      mContractIDs.Put(contractID, oldf);
       return NS_OK;
     }
 
     if (StaticComponents::LookupByCID(aClass)) {
       // If this is the CID of a static module, just reset the invalid bit of
       // the static entry for this contract ID, and assume it points to the
       // correct class.
-      nsDependentCString contractID(aContractID);
       if (StaticComponents::InvalidateContractID(contractID, false)) {
         mContractIDs.Remove(contractID);
         return NS_OK;
       }
     }
     return NS_ERROR_FACTORY_NOT_REGISTERED;
   }