Bug 1299306 part 1. Refactor the error handling in CGWrapNonWrapperCacheMethod and CGWrapWithCacheMethod to have less duplication. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 02 Sep 2016 17:55:38 -0400
changeset 312518 2b20981109d68bfc483317bd531111aee171dd47
parent 312517 256ea0d0de34213b4a6f295900b9892d99b77c3e
child 312519 f3c975a2bc7eb37a566b3917a4502e12111160c6
push id81392
push userbzbarsky@mozilla.com
push dateFri, 02 Sep 2016 21:55:53 +0000
treeherdermozilla-inbound@f52595d3b9f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1299306
milestone51.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 1299306 part 1. Refactor the error handling in CGWrapNonWrapperCacheMethod and CGWrapWithCacheMethod to have less duplication. r=peterv
dom/bindings/Codegen.py
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -3397,17 +3397,17 @@ def InitUnforgeablePropertiesOnHolder(de
             }
             """,
             failureCode=failureCode,
             holderName=holderName)))
 
     return CGWrapper(CGList(unforgeables), pre="\n")
 
 
-def CopyUnforgeablePropertiesToInstance(descriptor, wrapperCache):
+def CopyUnforgeablePropertiesToInstance(descriptor, failureCode):
     """
     Copy the unforgeable properties from the unforgeable holder for
     this interface to the instance object we have.
     """
     assert not descriptor.isGlobal();
 
     if not descriptor.hasUnforgeableMembers:
         return ""
@@ -3417,53 +3417,42 @@ def CopyUnforgeablePropertiesToInstance(
             """
             // Important: do unforgeable property setup after we have handed
             // over ownership of the C++ object to obj as needed, so that if
             // we fail and it ends up GCed it won't have problems in the
             // finalizer trying to drop its ownership of the C++ object.
             """))
     ]
 
-    if wrapperCache:
-        cleanup = dedent(
-            """
-            aCache->ReleaseWrapper(aObject);
-            aCache->ClearWrapper();
-            """)
-    else:
-        cleanup = ""
-
     # For proxies, we want to define on the expando object, not directly on the
     # reflector, so we can make sure we don't get confused by named getters.
     if descriptor.proxy:
         copyCode.append(CGGeneric(fill(
             """
             JS::Rooted<JSObject*> expando(aCx,
               DOMProxyHandler::EnsureExpandoObject(aCx, aReflector));
             if (!expando) {
-              $*{cleanup}
-              return false;
-            }
-            """,
-            cleanup=cleanup)))
+              $*{failureCode}
+            }
+            """,
+            failureCode=failureCode)))
         obj = "expando"
     else:
         obj = "aReflector"
 
     copyCode.append(CGGeneric(fill(
         """
         JS::Rooted<JSObject*> unforgeableHolder(aCx,
           &js::GetReservedSlot(canonicalProto, DOM_INTERFACE_PROTO_SLOTS_BASE).toObject());
         if (!JS_InitializePropertiesFromCompatibleNativeObject(aCx, ${obj}, unforgeableHolder)) {
-          $*{cleanup}
-          return false;
+          $*{failureCode}
         }
         """,
         obj=obj,
-        cleanup=cleanup)))
+        failureCode=failureCode)))
 
     return CGWrapper(CGList(copyCode), pre="\n").define()
 
 
 def AssertInheritanceChain(descriptor):
     asserts = ""
     iface = descriptor.interface
     while iface:
@@ -3473,43 +3462,34 @@ def AssertInheritanceChain(descriptor):
             "           reinterpret_cast<%s*>(aObject),\n"
             "           \"Multiple inheritance for %s is broken.\");\n" %
             (desc.nativeType, desc.nativeType, desc.nativeType))
         iface = iface.parent
     asserts += "MOZ_ASSERT(ToSupportsIsCorrect(aObject));\n"
     return asserts
 
 
-def InitMemberSlots(descriptor, wrapperCache):
+def InitMemberSlots(descriptor, failureCode):
     """
     Initialize member slots on our JS object if we're supposed to have some.
 
     Note that this is called after the SetWrapper() call in the
     wrapperCache case, since that can affect how our getters behave
     and we plan to invoke them here.  So if we fail, we need to
     ClearWrapper.
     """
     if not descriptor.interface.hasMembersInSlots():
         return ""
-    if wrapperCache:
-        clearWrapper = dedent(
-            """
-            aCache->ReleaseWrapper(aObject);
-            aCache->ClearWrapper();
-            """)
-    else:
-        clearWrapper = ""
     return fill(
         """
         if (!UpdateMemberSlots(aCx, aReflector, aObject)) {
-          $*{clearWrapper}
-          return false;
+          $*{failureCode}
         }
         """,
-        clearWrapper=clearWrapper)
+        failureCode=failureCode)
 
 
 def DeclareProto():
     """
     Declare the canonicalProto and proto we have for our wrapping operation.
     """
     return dedent(
         """
@@ -3556,16 +3536,24 @@ class CGWrapWithCacheMethod(CGAbstractMe
                 """
                 // For DOM proxies, the only reliable way to preserve the wrapper
                 // is to force creation of the expando object.
                 JS::Rooted<JSObject*> unused(aCx,
                   DOMProxyHandler::EnsureExpandoObject(aCx, aReflector));
                 """)
         else:
             preserveWrapper = "PreserveWrapper(aObject);\n"
+
+        failureCode = dedent(
+            """
+            aCache->ReleaseWrapper(aObject);
+            aCache->ClearWrapper();
+            return false;
+            """)
+
         return fill(
             """
             $*{assertInheritance}
             MOZ_ASSERT(!aCache->GetWrapper(),
                        "You should probably not be using Wrap() directly; use "
                        "GetOrCreateDOMReflector instead");
 
             MOZ_ASSERT(ToSupportsIsOnPrimaryInheritanceChain(aObject, aCache),
@@ -3610,18 +3598,19 @@ class CGWrapWithCacheMethod(CGAbstractMe
               $*{preserveWrapper}
             }
 
             return true;
             """,
             assertInheritance=AssertInheritanceChain(self.descriptor),
             declareProto=DeclareProto(),
             createObject=CreateBindingJSObject(self.descriptor, self.properties),
-            unforgeable=CopyUnforgeablePropertiesToInstance(self.descriptor, True),
-            slots=InitMemberSlots(self.descriptor, True),
+            unforgeable=CopyUnforgeablePropertiesToInstance(self.descriptor,
+                                                            failureCode),
+            slots=InitMemberSlots(self.descriptor, failureCode),
             preserveWrapper=preserveWrapper)
 
 
 class CGWrapMethod(CGAbstractMethod):
     def __init__(self, descriptor):
         # XXX can we wrap if we don't have an interface prototype object?
         assert descriptor.interface.hasInterfacePrototypeObject()
         args = [Argument('JSContext*', 'aCx'),
@@ -3650,16 +3639,18 @@ class CGWrapNonWrapperCacheMethod(CGAbst
         args = [Argument('JSContext*', 'aCx'),
                 Argument(descriptor.nativeType + '*', 'aObject'),
                 Argument('JS::Handle<JSObject*>', 'aGivenProto'),
                 Argument('JS::MutableHandle<JSObject*>', 'aReflector')]
         CGAbstractMethod.__init__(self, descriptor, 'Wrap', 'bool', args)
         self.properties = properties
 
     def definition_body(self):
+        failureCode = "return false;\n"
+
         return fill(
             """
             $*{assertions}
 
             JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
             $*{declareProto}
 
             $*{createObject}
@@ -3668,18 +3659,19 @@ class CGWrapNonWrapperCacheMethod(CGAbst
 
             $*{slots}
             creator.InitializationSucceeded();
             return true;
             """,
             assertions=AssertInheritanceChain(self.descriptor),
             declareProto=DeclareProto(),
             createObject=CreateBindingJSObject(self.descriptor, self.properties),
-            unforgeable=CopyUnforgeablePropertiesToInstance(self.descriptor, False),
-            slots=InitMemberSlots(self.descriptor, False))
+            unforgeable=CopyUnforgeablePropertiesToInstance(self.descriptor,
+                                                            failureCode),
+            slots=InitMemberSlots(self.descriptor, failureCode))
 
 
 class CGWrapGlobalMethod(CGAbstractMethod):
     """
     Create a wrapper JSObject for a global.  The global must implement
     nsWrapperCache.
 
     properties should be a PropertyArrays instance.
@@ -3752,17 +3744,17 @@ class CGWrapGlobalMethod(CGAbstractMetho
             return true;
             """,
             assertions=AssertInheritanceChain(self.descriptor),
             nativeType=self.descriptor.nativeType,
             properties=properties,
             chromeProperties=chromeProperties,
             failureCode=failureCode,
             unforgeable=unforgeable,
-            slots=InitMemberSlots(self.descriptor, True))
+            slots=InitMemberSlots(self.descriptor, failureCode))
 
 
 class CGUpdateMemberSlotsMethod(CGAbstractStaticMethod):
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'aCx'),
                 Argument('JS::Handle<JSObject*>', 'aWrapper'),
                 Argument(descriptor.nativeType + '*', 'aObject')]
         CGAbstractStaticMethod.__init__(self, descriptor, 'UpdateMemberSlots', 'bool', args)