Last Comment Bug 711240 - Expose BAD_TLS_INDEX and xpc_qsStringToJsval publicly
: Expose BAD_TLS_INDEX and xpc_qsStringToJsval publicly
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: :Ms2ger
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-15 13:43 PST by :Ms2ger
Modified: 2011-12-24 04:33 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (17.97 KB, patch)
2011-12-15 13:44 PST, :Ms2ger
bobbyholley: review+
Details | Diff | Splinter Review

Description :Ms2ger 2011-12-15 13:43:08 PST
These are used all over Gecko, and currently requires including xpcprivate.h and XPCQuickStubs.h (which aren't exported). I think it makes more sense to just put them in xpcpublic.h.

Also, this allows:

--- a/dom/indexedDB/Makefile.in
+++ b/dom/indexedDB/Makefile.in
@@ -88,17 +88,16 @@ EXPORTS_mozilla/dom/indexedDB = \
   $(NULL)
 
 LOCAL_INCLUDES = \
   -I$(topsrcdir)/xpcom/build \
   -I$(topsrcdir)/dom/base \
   -I$(topsrcdir)/dom/src/storage \
   -I$(topsrcdir)/content/base/src \
   -I$(topsrcdir)/content/events/src \
-  -I$(topsrcdir)/js/xpconnect/src \
Comment 1 :Ms2ger 2011-12-15 13:44:07 PST
Created attachment 582098 [details] [diff] [review]
Patch v1
Comment 2 Bobby Holley (:bholley) (busy with Stylo) 2011-12-20 12:15:14 PST
Comment on attachment 582098 [details] [diff] [review]
Patch v1

>diff --git a/dom/indexedDB/IDBObjectStore.cpp b/dom/indexedDB/IDBObjectStore.cpp

>+
>+#define BAD_TLS_INDEX (PRUintn)-1
>+

This should really be be defined in the same header as PR_NewThreadPrivateIndex. But I guess modifying NSPR is hard...


>+inline uint64_t
>+DoubleToUint64(double doubleval)
>+{
>+#ifdef XP_WIN
>+    // Note: Win32 can't handle double to uint64_t directly
>+    return static_cast<uint64_t>(static_cast<int64_t>(doubleval));
>+#else
>+    return static_cast<uint64_t>(doubleval);
>+#endif
>+}
>+
>+} // namespace xpc
>+

Makoto just fixed this over in bug 711404. Given that, we don't really need a function to do this anymore, as a simple cast will suffice. Can you wait for that bug to land, and then just kill xpc_qsDoubleToUint64 entirely?

r+ on the xpc_qsStringToJsval changes.
Comment 3 :Ms2ger 2011-12-21 01:13:20 PST
(In reply to Bobby Holley (:bholley) from comment #2)
> Comment on attachment 582098 [details] [diff] [review]
> Patch v1
> 
> >diff --git a/dom/indexedDB/IDBObjectStore.cpp b/dom/indexedDB/IDBObjectStore.cpp
> 
> >+
> >+#define BAD_TLS_INDEX (PRUintn)-1
> >+
> 
> This should really be be defined in the same header as
> PR_NewThreadPrivateIndex. But I guess modifying NSPR is hard...

How about adding this in a #ifndef/#endif block while I try to get a patch into NSPR?

> 
> >+inline uint64_t
> >+DoubleToUint64(double doubleval)
> >+{
> >+#ifdef XP_WIN
> >+    // Note: Win32 can't handle double to uint64_t directly
> >+    return static_cast<uint64_t>(static_cast<int64_t>(doubleval));
> >+#else
> >+    return static_cast<uint64_t>(doubleval);
> >+#endif
> >+}
> >+
> >+} // namespace xpc
> >+
> 
> Makoto just fixed this over in bug 711404. Given that, we don't really need
> a function to do this anymore, as a simple cast will suffice. Can you wait
> for that bug to land, and then just kill xpc_qsDoubleToUint64 entirely?

Great.

> r+ on the xpc_qsStringToJsval changes.

Thanks.
Comment 4 Bobby Holley (:bholley) (busy with Stylo) 2011-12-21 10:39:16 PST
> How about adding this in a #ifndef/#endif block while I try to get a patch
> into NSPR?

Sounds good!

Note You need to log in before you can comment on or make changes to this bug.