280089 - [FIX]Add debug-only mCompiled members to XBL classes that care

Status: RESOLVED FIXED

(Core :: XBL, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

Some XBL classes have a union that holds different things for compiled and
uncompiled members.  This bug is about adding a debug-only member to make sure
that we're not getting confused about whether we're compiled.  See discussion in
bug 275232.

Note that when working on this I found a simple way to make properties crash due
to precisely this issue.  The patch will fix that.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: XBL for crash testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Crash testcase

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

This adds the asserts, fixes the crash that the testcase shows, and fixes a bug
in nsXBLProtoImpl caught by the asserts.

Comment 4

[Brian Ryner (not reading)]

Comment on attachment 172601 [details] [diff] [review]
Patch

>--- content/xbl/src/nsXBLProtoImplProperty.cpp	16 Sep 2004 18:30:04 -0000	1.16
>+++ content/xbl/src/nsXBLProtoImplProperty.cpp	27 Jan 2005 21:08:54 -0000
>@@ -237,25 +256,33 @@ nsXBLProtoImplProperty::CompileMember(ns
>         mJSGetterObject = nsnull;
>         mJSAttributes &= ~JSPROP_GETTER;
>         /*chaining to return failure*/
>       }
>     }
>   } // if getter is not empty
> 
>   if (!deletedGetter) {  // Empty getter
>     delete mGetterText;
>     mJSGetterObject = nsnull;
>   }
>   
>-  nsresult rvG=rv;
>+  if (NS_FAILED(rv)) {
>+    // We failed to compile our getter.  So either we've set it to null, or
>+    // it's still set to the text object.  In either case, it's safe to return
>+    // the error her, since then we'll be cleaned up as uncompiled and that

s/her/here/

Looks fine otherwise.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Checked in.