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

RESOLVED FIXED in mozilla1.8beta1

Status

()

Core
XBL
P1
normal
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

Trunk
mozilla1.8beta1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

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.
Created attachment 172595 [details]
XBL for crash testcase
Created attachment 172596 [details]
Crash testcase
Created attachment 172601 [details] [diff] [review]
Patch

This adds the asserts, fixes the crash that the testcase shows, and fixes a bug
in nsXBLProtoImpl caught by the asserts.
Attachment #172601 - Flags: superreview?(bryner)
Attachment #172601 - Flags: review?(bryner)
Priority: -- → P1
Summary: Add debug-only mCompiled members to XBL classes that care → [FIX]Add debug-only mCompiled members to XBL classes that care
Target Milestone: --- → mozilla1.8beta
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.
Attachment #172601 - Flags: superreview?(bryner)
Attachment #172601 - Flags: superreview+
Attachment #172601 - Flags: review?(bryner)
Attachment #172601 - Flags: review+
Checked in.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.