Switching JSClass.mark in XPConnect to the tracing semantics

RESOLVED FIXED

Status

()

--
enhancement
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: igor, Assigned: igor)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 10 obsolete attachments)

(Assignee)

Description

12 years ago
This is a continuation of work from bug 375270 comment 38 to switch all JSMarkOp functions to the new JSTraceOp semantics.
(Assignee)

Updated

12 years ago
Depends on: 375270
(Assignee)

Comment 1

12 years ago
Created attachment 261787 [details] [diff] [review]
Implementation v1

The implementation is a mostly rename exercise. But also pints to a deficiency of the GC and new trace infrastructure as the code would not trace auto roots during non-GC tracing. Effectively any calls to JS_GetGCMarkingTracer means that potentially something is not exposed to the generic tracer and the patch has to add the call the API in 3 places.

I will fix that in bug 340212.
Attachment #261787 - Flags: review?(brendan)
(Assignee)

Comment 2

12 years ago
Comment on attachment 261787 [details] [diff] [review]
Implementation v1

Who should I ask for sr= ?
(Assignee)

Comment 3

12 years ago
Recording patch dependency of the fix for bug 340212 on this bug.
Blocks: 340212
(Assignee)

Comment 4

12 years ago
Created attachment 261945 [details] [diff] [review]
Implementation v2

The new version consistently uses TraceJS for all methods that should trace JS things in a paerticular class/structure. The tracing code from XPCWrappedNativeScope::FinishedMarkPhaseOfGC and JSBool XPCJSRuntime::GCCallback is also moved to separated TraceJS methods to simplify a fix for bug 340212.
Attachment #261787 - Attachment is obsolete: true
Attachment #261945 - Flags: review?(brendan)
Attachment #261787 - Flags: review?(brendan)
Blocks: 377884
Comment on attachment 261945 [details] [diff] [review]
Implementation v2

>+ * Is called to trace things that the object helds.

s/helds/holds/

>+void XPCDispIDArray::TraceJS(JSTracer* trc)
> {
>     // If already marked nothing to do
>-    if(IsMarked())
>-        return;
>-    mMarked = JS_TRUE;
>-    XPCCallContext ccx(NATIVE_CALLER);
>-    // Bail if our call context is bad
>-    if(!ccx.IsValid())
>-        return;
>+    if(JS_IsGCMarkingTracer(trc)) {

Nit: brace on new line as elsewhere in xpconnect/src.

r=me, as peer (am I listed? I'll check) of xpconnect.

/be
Attachment #261945 - Flags: superreview?(jst)
Attachment #261945 - Flags: review?(brendan)
Attachment #261945 - Flags: review+
(Assignee)

Comment 6

12 years ago
Created attachment 262659 [details] [diff] [review]
Implementation v2b

Here is a new version of the patch to address the couple of nits from the previous comment.
Attachment #261945 - Attachment is obsolete: true
Attachment #262659 - Flags: superreview?(jst)
Attachment #262659 - Flags: review+
Attachment #261945 - Flags: superreview?(jst)
(Assignee)

Updated

12 years ago
Blocks: 378742
(Assignee)

Comment 7

12 years ago
Ping for sr=something
(Assignee)

Comment 8

12 years ago
Comment on attachment 262659 [details] [diff] [review]
Implementation v2b

The patch is not complete, I missed the msIXPCScriptable.idl and friends.
Attachment #262659 - Attachment is obsolete: true
Attachment #262659 - Flags: superreview?(jst)
(Assignee)

Comment 9

12 years ago
Created attachment 262893 [details] [diff] [review]
Implementation v3

This is the previous patch + changes to nsIXPCScriptable.idl and dependent files   to replace mark by trace.

There are also changes to XPCDispIDArray where I removed duplicated TraceJS declaration. But since XPCDispTypeInfo.cpp is not part of the build, I do not now how to verify the changes there.
Attachment #262893 - Flags: superreview?(jst)
Attachment #262893 - Flags: review?(brendan)
Comment on attachment 262893 [details] [diff] [review]
Implementation v3

- In js/src/xpconnect/idl/nsIXPCScriptable.idl:

-    PRUint32 mark(in nsIXPConnectWrappedNative wrapper,
-                  in JSContextPtr cx, in JSObjectPtr obj, in voidPtr arg);
+    void trace(in nsIXPConnectWrappedNative wrapper,
+               in JSTracerPtr cx, in JSObjectPtr obj);

The JSTracerPtr argument should be named something other than "cx" here too.

And since you're changing the signature of this interface you'll need to generate a new IID.

sr=jst with that.
Attachment #262893 - Flags: superreview?(jst) → superreview+
Comment on attachment 262893 [details] [diff] [review]
Implementation v3

What jst said -- I'll check over a final patch when you attach.

/be
Attachment #262893 - Flags: review?(brendan) → review+
(Assignee)

Comment 12

12 years ago
Created attachment 262998 [details] [diff] [review]
Implementation v4

The new version to address comment 10.
Attachment #262893 - Attachment is obsolete: true
Attachment #262998 - Flags: review?(brendan)
(Assignee)

Comment 13

12 years ago
Created attachment 262999 [details] [diff] [review]
Implementation v4 for real

I forgot to regenerate CVS diff the last time, now this should address comment 10.
Attachment #262998 - Attachment is obsolete: true
Attachment #262999 - Flags: review?(brendan)
Attachment #262998 - Flags: review?(brendan)
Comment on attachment 262999 [details] [diff] [review]
Implementation v4 for real


>+++ js/src/xpconnect/src/XPCDispPrivate.h	27 Apr 2007 07:25:08 -0000
>@@ -302,30 +302,25 @@ public:
>      * @param cx a JS context
>      * @param index index into the array
>      * @return the ID as a jsval
>      */
>     jsval Item(JSContext* cx, PRUint32 index) const;
> 
>     /**
>-     * Called to mark the ID's during GC
>+     * Called to trace jsval associated with the ID's
>      */
>-    void Mark();
>+    void TraceJS(JSTracer* trc);
>     /**
>-     * Called to unmark the ID's after GC has been done
>+     * Called to unmark the ID's marked during GC marking trace
>      */
>     void Unmark();
>     /**
>      * Tests whether the ID is marked
>      */
>     JSBool IsMarked() const;

While you are here, how about a blank line before each doc-comment?

>+void XPCDispIDArray::TraceJS(JSTracer* trc)
> {
>     // If already marked nothing to do
>-    if(IsMarked())
>-        return;
>-    mMarked = JS_TRUE;
>-    XPCCallContext ccx(NATIVE_CALLER);
>-    // Bail if our call context is bad
>-    if(!ccx.IsValid())
>-        return;
>+    if(JS_IsGCMarkingTracer(trc))
>+    {
>+        if (IsMarked())
>+            return;
>+        mMarked = JS_TRUE;
>+    }
> 
>     PRInt32 count = Length();
>     jsval val;
>-    JSContext* cx = ccx;
>+
>     // Iterate each of the ID's and mark them
>     for(PRInt32 index = 0; index < count; ++index)
>     {
>         if(JS_IdToValue(cx,

This won't compile -- use trc->context or restore the cx local (can you test the IDispatch stuff at least to see that it compiles?).

> mozStorageStatementRow::Equality(nsIXPConnectWrappedNative *wrapper,
>@@ -997,18 +996,18 @@ mozStorageStatementParams::Construct(nsI
> NS_IMETHODIMP
> mozStorageStatementParams::HasInstance(nsIXPConnectWrappedNative *wrapper, JSContext * cx,
>                                     JSObject * obj, jsval val, PRBool *bp, PRBool *_retval)
> {
>     return NS_ERROR_NOT_IMPLEMENTED;
> }
> 
>-/* PRUint32 mark (in nsIXPConnectWrappedNative wrapper, in JSContextPtr cx, in JSObjectPtr obj, in voidPtr arg); */
>+/* void trace (in nsIXPConnectWrappedNative wrapper, in JSTracerPtr trc, in JSObjectPtr obj); */
> NS_IMETHODIMP
>-mozStorageStatementParams::Mark(nsIXPConnectWrappedNative *wrapper, JSContext * cx,
>-                             JSObject * obj, void * arg, PRUint32 *_retval)
>+mozStorageStatementParams::Trace(nsIXPConnectWrappedNative *wrapper,
>+                                JSTracer *trc, JSObject * obj)

Nit: could line up overflow params to underhang as usual (even for pre-existing nit above this one).

r=me with fixes, sorry I didn't catch them earlier.

/be
Attachment #262999 - Flags: review?(brendan) → review+
(Assignee)

Comment 15

12 years ago
(In reply to comment #14)
> >+void XPCDispIDArray::TraceJS(JSTracer* trc)
...
> This won't compile -- use trc->context or restore the cx local (can you test
> the IDispatch stuff at least to see that it compiles?).

I am fixing the bugs in the body of TraceJS, but that does not make XPCDispTypeInfo.cpp to compile (I used the same command that make uses to compile other files in js/src/xpconnect/src):
 
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:59: error: variable or field 'FillOutElemDesc' declared void
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:59: error: 'FillOutElemDesc' declared as an 'inline' variable
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:59: error: 'VARTYPE' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:59: error: expected primary-expression before 'paramFlags'
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:59: error: 'ELEMDESC' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:59: error: 'elemDesc' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:59: error: initializer expression list treated as compound expression
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:60: error: expected ',' or ';' before '{' token
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:69: error: expected constructor, destructor, or type conversion before '::' token
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:74: error: 'XPCDispJSPropertyInfo' has not been declared
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:74: error: 'ELEMDESC' has not been declared
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp: In function 'void GetReturnType(XPCCallContext&, int&)':
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:76: error: 'VARTYPE' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:76: error: expected `;' before 'vt'
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:77: error: 'IsSetter' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:79: error: 'vt' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:79: error: 'VT_EMPTY' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:81: error: 'IsProperty' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:83: error: 'vt' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:83: error: 'XPCDispConvert' has not been declared
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:83: error: 'mProperty' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:87: error: 'vt' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:87: error: 'VT_VARIANT' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:89: error: 'vt' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:89: error: 'PARAMFLAG_FRETVAL' was not declared in this scope
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:89: error: 'FillOutElemDesc' cannot be used as a function
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp: At global scope:
/home/igor/m/trunk/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp:92: error: expected constructor, destructor, or type conversion before '*'

Any particular reason for keeping XPCDispTypeInfo.cpp together with other sources?
(Assignee)

Comment 16

12 years ago
Created attachment 263162 [details] [diff] [review]
Implementation v5

This is the previous patch + changes to address nits from comment 14. 

In storage/src/mozStorageStatementWrapper.cpp the bad indentation exists throughout the whole file so I fixed only indentation only for Trace that replaces Mark keeping the rest of the source unchanged.
Attachment #262999 - Attachment is obsolete: true
Attachment #263162 - Flags: review?(brendan)
(Assignee)

Updated

12 years ago
Blocks: 379220

Updated

12 years ago
Attachment #263162 - Flags: review?(brendan) → review+
(Assignee)

Comment 17

12 years ago
I committed the patch from comment 16 to the trunk:

Waiting for Emacs...Done
Checking in dom/src/base/nsDOMClassInfo.cpp;
/cvsroot/mozilla/dom/src/base/nsDOMClassInfo.cpp,v  <--  nsDOMClassInfo.cpp
new revision: 1.437; previous revision: 1.436
done
Checking in js/src/xpconnect/idl/nsIXPCScriptable.idl;
/cvsroot/mozilla/js/src/xpconnect/idl/nsIXPCScriptable.idl,v  <--  nsIXPCScriptable.idl
new revision: 1.10; previous revision: 1.9
done
Checking in js/src/xpconnect/idl/nsIXPConnect.idl;
/cvsroot/mozilla/js/src/xpconnect/idl/nsIXPConnect.idl,v  <--  nsIXPConnect.idl
new revision: 1.48; previous revision: 1.47
done
Checking in js/src/xpconnect/public/xpc_map_end.h;
/cvsroot/mozilla/js/src/xpconnect/public/xpc_map_end.h,v  <--  xpc_map_end.h
new revision: 1.9; previous revision: 1.8
done
Checking in js/src/xpconnect/src/XPCDispInlines.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispInlines.h,v  <--  XPCDispInlines.h
new revision: 1.13; previous revision: 1.12
done
Checking in js/src/xpconnect/src/XPCDispParamPropJSClass.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispParamPropJSClass.cpp,v  <--  XPCDispParamPropJSClass.cpp
new revision: 1.6; previous revision: 1.5
done
Checking in js/src/xpconnect/src/XPCDispPrivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispPrivate.h,v  <--  XPCDispPrivate.h
new revision: 1.18; previous revision: 1.17
done
Checking in js/src/xpconnect/src/XPCDispTypeInfo.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp,v  <--  XPCDispTypeInfo.cpp
new revision: 1.10; previous revision: 1.9
done
Checking in js/src/xpconnect/src/XPCNativeWrapper.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp,v  <--  XPCNativeWrapper.cpp
new revision: 1.48; previous revision: 1.47
done
Checking in js/src/xpconnect/src/xpcjsruntime.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp,v  <--  xpcjsruntime.cpp
new revision: 1.48; previous revision: 1.47
done
Checking in js/src/xpconnect/src/xpcprivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/xpcprivate.h,v  <--  xpcprivate.h
new revision: 1.208; previous revision: 1.207
done
Checking in js/src/xpconnect/src/xpcthreadcontext.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcthreadcontext.cpp,v  <--  xpcthreadcontext.cpp
new revision: 1.45; previous revision: 1.44
done
Checking in js/src/xpconnect/src/xpcwrappednative.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,v  <--  xpcwrappednative.cpp
new revision: 1.134; previous revision: 1.133
done
Checking in js/src/xpconnect/src/xpcwrappednativejsops.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,v  <--  xpcwrappednativejsops.cpp
new revision: 1.67; previous revision: 1.66
done
Checking in js/src/xpconnect/src/xpcwrappednativescope.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp,v  <--  xpcwrappednativescope.cpp
new revision: 1.34; previous revision: 1.33
done
Checking in storage/src/mozStorageStatementWrapper.cpp;
/cvsroot/mozilla/storage/src/mozStorageStatementWrapper.cpp,v  <--  mozStorageStatementWrapper.cpp
new revision: 1.11; previous revision: 1.10
done
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 18

12 years ago
Created attachment 263392 [details] [diff] [review]
XPCDispprivate.h build fix

Extra patch to fix windows compilation problem. I wonder how I could make it compile on Linux?
(Assignee)

Comment 19

12 years ago
I committed the patch from comment 18 to the trunk to fix win compilation:

Waiting for Emacs...Done
Checking in XPCDispInlines.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispInlines.h,v  <--  XPCDispInlines.h
new revision: 1.14; previous revision: 1.13
done
Checking in XPCDispPrivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispPrivate.h,v  <--  XPCDispPrivate.h
new revision: 1.19; previous revision: 1.18
done
This broke all the Windows tinderboxes:

e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\js\src\xpconnect\src\XPCDispInlines.h(467) : error C2511: 'void XPCDispIDArray::Mark(void)' : overloaded member function not found in 'XPCDispIDArray'
        e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\js\src\xpconnect\src\XPCDispPrivate.h(287) : see declaration of 'XPCDispIDArray'
(Assignee)

Comment 21

12 years ago
Created attachment 263393 [details] [diff] [review]
another XPCDispprivate.h build fix

Another fix I just committed to the trunk:

Waiting for Emacs...Done
Checking in XPCDispPrivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispPrivate.h,v  <--  XPCDispPrivate.h
new revision: 1.20; previous revision: 1.19
done

If this still would lead to compilation probnlems, I will remove the whole patch.
(Assignee)

Comment 22

12 years ago
Created attachment 263395 [details] [diff] [review]
another XPCDispprivate.h build fix for real

This the committed fix for real:

Checking in XPCDispPrivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispPrivate.h,v  <--  XPCDispPrivate.h
new revision: 1.20; previous revision: 1.19
done
Attachment #263393 - Attachment is obsolete: true
(Assignee)

Comment 23

12 years ago
I removed the commited patch:

Waiting for Emacs...Done
Checking in dom/src/base/nsDOMClassInfo.cpp;
/cvsroot/mozilla/dom/src/base/nsDOMClassInfo.cpp,v  <--  nsDOMClassInfo.cpp
new revision: 1.438; previous revision: 1.437
done
Checking in js/src/xpconnect/idl/nsIXPCScriptable.idl;
/cvsroot/mozilla/js/src/xpconnect/idl/nsIXPCScriptable.idl,v  <--  nsIXPCScriptable.idl
new revision: 1.11; previous revision: 1.10
done
Checking in js/src/xpconnect/idl/nsIXPConnect.idl;
/cvsroot/mozilla/js/src/xpconnect/idl/nsIXPConnect.idl,v  <--  nsIXPConnect.idl
new revision: 1.49; previous revision: 1.48
done
Checking in js/src/xpconnect/public/xpc_map_end.h;
/cvsroot/mozilla/js/src/xpconnect/public/xpc_map_end.h,v  <--  xpc_map_end.h
new revision: 1.10; previous revision: 1.9
done
Checking in js/src/xpconnect/src/XPCDispInlines.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispInlines.h,v  <--  XPCDispInlines.h
new revision: 1.15; previous revision: 1.14
done
Checking in js/src/xpconnect/src/XPCDispParamPropJSClass.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispParamPropJSClass.cpp,v  <--  XPCDispParamPropJSClass.cpp
new revision: 1.7; previous revision: 1.6
done
Checking in js/src/xpconnect/src/XPCDispPrivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispPrivate.h,v  <--  XPCDispPrivate.h
new revision: 1.21; previous revision: 1.20
done
Checking in js/src/xpconnect/src/XPCDispTypeInfo.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp,v  <--  XPCDispTypeInfo.cpp
new revision: 1.11; previous revision: 1.10
done
Checking in js/src/xpconnect/src/XPCNativeWrapper.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp,v  <--  XPCNativeWrapper.cpp
new revision: 1.49; previous revision: 1.48
done
Checking in js/src/xpconnect/src/xpcjsruntime.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp,v  <--  xpcjsruntime.cpp
new revision: 1.49; previous revision: 1.48
done
Checking in js/src/xpconnect/src/xpcprivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/xpcprivate.h,v  <--  xpcprivate.h
new revision: 1.209; previous revision: 1.208
done
Checking in js/src/xpconnect/src/xpcthreadcontext.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcthreadcontext.cpp,v  <--  xpcthreadcontext.cpp
new revision: 1.46; previous revision: 1.45
done
Checking in js/src/xpconnect/src/xpcwrappednative.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,v  <--  xpcwrappednative.cpp
new revision: 1.135; previous revision: 1.134
done
Checking in js/src/xpconnect/src/xpcwrappednativejsops.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,v  <--  xpcwrappednativejsops.cpp
new revision: 1.68; previous revision: 1.67
done
Checking in js/src/xpconnect/src/xpcwrappednativescope.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp,v  <--  xpcwrappednativescope.cpp
new revision: 1.35; previous revision: 1.34
done
Checking in storage/src/mozStorageStatementWrapper.cpp;
/cvsroot/mozilla/storage/src/mozStorageStatementWrapper.cpp,v  <--  mozStorageStatementWrapper.cpp
new revision: 1.12; previous revision: 1.11
done
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 24

12 years ago
Created attachment 263400 [details] [diff] [review]
Implementation v6

XPCDispTypeInfo.cpp is a part of the build when XPC_IDISPATCH_SUPPORT is set. According to the configure script this is only enabled when:

case "$target_os" in
12492     msvc*|mks*|cygwin*|mingw*)
12493         if test -z "$GNU_CC"; then 
12494             XPC_IDISPATCH_SUPPORT=1

So AFAICS I can not verify that part of the patch on a Linux box and has to rely on just manually checking the code :(

In any case here is an updated patch that should fix the problems
time here is the updated fix that should fix the problems.

The patch removes the following lines in XPCDispPrivate.h:
-    /**
-     * NOP. This is just here to make the AutoMarkingPtr code compile.
-     */
-    inline void MarkBeforeJSFinalize(JSContext*);

The comment is wrong as XPCDispIDArray can not be used with AutoMarkingPtr as it does not define AutoMark which is called from AutoMarkingPtr. Moreover, AFAICS neither MarkBeforeJSFinalize no Mark are never in fact invoked so I do not know what purpose they have. So the patch tries to preserve the logic in the implementation providing only TraceJS method as a replacement for both MarkBeforeJSFinalize and Mark.
Attachment #263162 - Attachment is obsolete: true
Attachment #263392 - Attachment is obsolete: true
Attachment #263395 - Attachment is obsolete: true
Attachment #263400 - Flags: superreview?(jst)
Attachment #263400 - Flags: review?(brendan)
Comment on attachment 263400 [details] [diff] [review]
Implementation v6

sr=me, letting jst r= this one.

/be
Attachment #263400 - Flags: superreview?(jst)
Attachment #263400 - Flags: superreview+
Attachment #263400 - Flags: review?(jst)
Attachment #263400 - Flags: review?(brendan)
Comment on attachment 263400 [details] [diff] [review]
Implementation v6

r=jst
Attachment #263400 - Flags: review?(jst) → review+
(Assignee)

Comment 27

12 years ago
I committed the patch from comment 24 to the trunk:

Checking in dom/src/base/nsDOMClassInfo.cpp;
/cvsroot/mozilla/dom/src/base/nsDOMClassInfo.cpp,v  <--  nsDOMClassInfo.cpp
new revision: 1.440; previous revision: 1.439
done
Checking in js/src/xpconnect/idl/nsIXPCScriptable.idl;
/cvsroot/mozilla/js/src/xpconnect/idl/nsIXPCScriptable.idl,v  <--  nsIXPCScriptable.idl
new revision: 1.12; previous revision: 1.11
done
Checking in js/src/xpconnect/idl/nsIXPConnect.idl;
/cvsroot/mozilla/js/src/xpconnect/idl/nsIXPConnect.idl,v  <--  nsIXPConnect.idl
new revision: 1.50; previous revision: 1.49
done
Checking in js/src/xpconnect/public/xpc_map_end.h;
/cvsroot/mozilla/js/src/xpconnect/public/xpc_map_end.h,v  <--  xpc_map_end.h
new revision: 1.11; previous revision: 1.10
done
Checking in js/src/xpconnect/src/XPCDispInlines.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispInlines.h,v  <--  XPCDispInlines.h
new revision: 1.16; previous revision: 1.15
done
Checking in js/src/xpconnect/src/XPCDispParamPropJSClass.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispParamPropJSClass.cpp,v  <--  XPCDispParamPropJSClass.cpp
new revision: 1.8; previous revision: 1.7
done
Checking in js/src/xpconnect/src/XPCDispPrivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispPrivate.h,v  <--  XPCDispPrivate.h
new revision: 1.22; previous revision: 1.21
done
Checking in js/src/xpconnect/src/XPCDispTypeInfo.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCDispTypeInfo.cpp,v  <--  XPCDispTypeInfo.cpp
new revision: 1.12; previous revision: 1.11
done
Checking in js/src/xpconnect/src/XPCNativeWrapper.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp,v  <--  XPCNativeWrapper.cpp
new revision: 1.50; previous revision: 1.49
done
Checking in js/src/xpconnect/src/xpcjsruntime.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp,v  <--  xpcjsruntime.cpp
new revision: 1.50; previous revision: 1.49
done
Checking in js/src/xpconnect/src/xpcprivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/xpcprivate.h,v  <--  xpcprivate.h
new revision: 1.210; previous revision: 1.209
done
Checking in js/src/xpconnect/src/xpcthreadcontext.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcthreadcontext.cpp,v  <--  xpcthreadcontext.cpp
new revision: 1.47; previous revision: 1.46
done
Checking in js/src/xpconnect/src/xpcwrappednative.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,v  <--  xpcwrappednative.cpp
new revision: 1.136; previous revision: 1.135
done
Checking in js/src/xpconnect/src/xpcwrappednativejsops.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,v  <--  xpcwrappednativejsops.cpp
new revision: 1.69; previous revision: 1.68
done
Checking in js/src/xpconnect/src/xpcwrappednativescope.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp,v  <--  xpcwrappednativescope.cpp
new revision: 1.36; previous revision: 1.35
done
Checking in storage/src/mozStorageStatementWrapper.cpp;
/cvsroot/mozilla/storage/src/mozStorageStatementWrapper.cpp,v  <--  mozStorageStatementWrapper.cpp
new revision: 1.13; previous revision: 1.12
done
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
It seems this checkin has busted all the Sunbird and Lightning Tinderboxes [http://tinderbox.mozilla.org/Sunbird]>:

mozilla/calendar/base/src/calDateTime.cpp(943) : 
error C2039: 'Mark' : is not a member of 'calDateTime'
(Assignee)

Comment 29

12 years ago
(In reply to comment #28)
> It seems this checkin has busted all the Sunbird and Lightning Tinderboxes
> [http://tinderbox.mozilla.org/Sunbird]>:
> 
> mozilla/calendar/base/src/calDateTime.cpp(943) : 
> error C2039: 'Mark' : is not a member of 'calDateTime'

I will fix it ASAP.
(Assignee)

Comment 30

12 years ago
Created attachment 263634 [details] [diff] [review]
fix for mozilla/calendar/base/src/calDateTime.cpp

An extra patch to fix calendar compilation problem.
Attachment #263634 - Flags: review+
(Assignee)

Comment 31

12 years ago
I committed the patch from comment 30 fix the calendar:

Checking in calendar/base/src/calDateTime.cpp;
/cvsroot/mozilla/calendar/base/src/calDateTime.cpp,v  <--  calDateTime.cpp
new revision: 1.64; previous revision: 1.63
done

Updated

10 years ago
Component: xpidl → XPCOM
QA Contact: xpidl → xpcom
You need to log in before you can comment on or make changes to this bug.