Bug 899367 - Only use JSOPTION_UNROOTED_GLOBAL for DOM JSContexts. r=mccr8
authorBobby Holley <bobbyholley@gmail.com>
Wed, 04 Sep 2013 14:06:54 -0700
changeset 158482 6490e2abb8b3a3aec14cdf39f657b60b7052cb32
parent 158481 7ccebf05488c23702234daffaae17676aa3af4a7
child 158483 001f423a94e8fedf82591e2f7cb9e225cf679cd6
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs899367
milestone26.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 899367 - Only use JSOPTION_UNROOTED_GLOBAL for DOM JSContexts. r=mccr8 We don't cycle collect any other kind, so there's no difference between marking them gray and letting the JS engine mark them black. This also gets rid of that nasty code which reset the flag, which billm theorizes has to do with faulty logic in ContextHolder when creating ephemeral sandbox cxes. The assertion in this patch should catch us if anything goes wrong.
dom/base/nsJSEnvironment.cpp
dom/workers/RuntimeService.cpp
js/xpconnect/src/XPCJSRuntime.cpp
xpcom/base/CycleCollectedJSRuntime.cpp
xpcom/base/CycleCollectedJSRuntime.h
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -846,17 +846,18 @@ nsJSContext::nsJSContext(bool aGCOnDestr
   mPrev = &sContextList;
   if (sContextList) {
     sContextList->mPrev = &mNext;
   }
   sContextList = this;
 
   ++sContextCount;
 
-  mDefaultJSOptions = JSOPTION_PRIVATE_IS_NSISUPPORTS;
+  mDefaultJSOptions = JSOPTION_PRIVATE_IS_NSISUPPORTS |
+                      JSOPTION_UNROOTED_GLOBAL;
 
   mContext = ::JS_NewContext(sRuntime, gStackSize);
   if (mContext) {
     ::JS_SetContextPrivate(mContext, static_cast<nsIScriptContext *>(this));
 
     // Preserve any flags the context callback might have set.
     mDefaultJSOptions |= ::JS_GetOptions(mContext);
 
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -826,18 +826,17 @@ CreateJSContextForWorker(WorkerPrivate* 
 
 class WorkerJSRuntime : public mozilla::CycleCollectedJSRuntime
 {
 public:
   // The heap size passed here doesn't matter, we will change it later in the
   // call to JS_SetGCParameter inside CreateJSContextForWorker.
   WorkerJSRuntime(WorkerPrivate* aWorkerPrivate)
   : CycleCollectedJSRuntime(WORKER_DEFAULT_RUNTIME_HEAPSIZE,
-                            JS_NO_HELPER_THREADS,
-                            false),
+                            JS_NO_HELPER_THREADS),
     mWorkerPrivate(aWorkerPrivate)
   {
     // We need to ensure that a JSContext outlives the cycle collector, and
     // that the internal JSContext created by ctypes is not the last JSContext
     // to die.  So we create an unused JSContext here and destroy it after
     // the cycle collector shuts down.  Thus all cycles will be broken before
     // the last GC and all finalizers will be run.
     mLastJSContext = JS_NewContext(Runtime(), 0);
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -2869,17 +2869,17 @@ SourceHook(JSContext *cx, JS::Handle<JSS
     xpc::Throw(cx, rv);
     return false;
   }
 
   return true;
 }
 
 XPCJSRuntime::XPCJSRuntime(nsXPConnect* aXPConnect)
-   : CycleCollectedJSRuntime(32L * 1024L * 1024L, JS_USE_HELPER_THREADS, true),
+   : CycleCollectedJSRuntime(32L * 1024L * 1024L, JS_USE_HELPER_THREADS),
    mJSContextStack(new XPCJSContextStack()),
    mCallContext(nullptr),
    mAutoRoots(nullptr),
    mResolveName(JSID_VOID),
    mResolvingWrapper(nullptr),
    mWrappedJSMap(JSObject2WrappedJSMap::newMap(XPC_JS_MAP_SIZE)),
    mWrappedJSClassMap(IID2WrappedJSClassMap::newMap(XPC_JS_CLASS_MAP_SIZE)),
    mIID2NativeInterfaceMap(IID2NativeInterfaceMap::newMap(XPC_NATIVE_INTERFACE_MAP_SIZE)),
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -470,22 +470,20 @@ NoteJSChildGrayWrapperShim(void* aData, 
  * cycle.)
  */
 
 // NB: This is only used to initialize the participant in
 // CycleCollectedJSRuntime. It should never be used directly.
 static const JSZoneParticipant sJSZoneCycleCollectorGlobal;
 
 CycleCollectedJSRuntime::CycleCollectedJSRuntime(uint32_t aMaxbytes,
-                                                 JSUseHelperThreads aUseHelperThreads,
-                                                 bool aExpectUnrootedGlobals)
+                                                 JSUseHelperThreads aUseHelperThreads)
   : mGCThingCycleCollectorGlobal(sGCThingCycleCollectorGlobal),
     mJSZoneCycleCollectorGlobal(sJSZoneCycleCollectorGlobal),
-    mJSRuntime(nullptr),
-    mExpectUnrootedGlobals(aExpectUnrootedGlobals)
+    mJSRuntime(nullptr)
 #ifdef DEBUG
   , mObjectToUnlink(nullptr)
 #endif
 {
   mJSRuntime = JS_NewRuntime(aMaxbytes, aUseHelperThreads);
   if (!mJSRuntime) {
     MOZ_CRASH();
   }
@@ -540,17 +538,20 @@ CycleCollectedJSRuntime::UnmarkSkippable
   mJSHolders.Enumerate(UnmarkJSHolder, nullptr);
 }
 
 void
 CycleCollectedJSRuntime::MaybeTraceGlobals(JSTracer* aTracer) const
 {
   JSContext* iter = nullptr;
   while (JSContext* acx = JS_ContextIterator(Runtime(), &iter)) {
-    MOZ_ASSERT(js::HasUnrootedGlobal(acx) == mExpectUnrootedGlobals);
+    // DOM JSContexts are the only JSContexts that cycle-collect their default
+    // compartment object, so they're the only ones that we need to do the
+    // JSOPTION_UNROOTED_GLOBAL dance for. The other ones are just marked black.
+    MOZ_ASSERT(js::HasUnrootedGlobal(acx) == !!GetScriptContextFromJSContext(acx));
     if (!js::HasUnrootedGlobal(acx)) {
       continue;
     }
 
     if (JSObject* global = js::DefaultObjectForContextOrNull(acx)) {
       JS::AssertGCThingMustBeTenured(global);
       JS_CallObjectTracer(aTracer, &global, "Global Object");
     }
@@ -1182,31 +1183,17 @@ CycleCollectedJSRuntime::FinalizeDeferre
   }
 }
 
 void
 CycleCollectedJSRuntime::OnGC(JSGCStatus aStatus)
 {
   switch (aStatus) {
     case JSGC_BEGIN:
-    {
-      // XXXkhuey do we still need this?
-      // We seem to sometime lose the unrooted global flag. Restore it
-      // here. FIXME: bug 584495.
-      if (mExpectUnrootedGlobals){
-        JSContext* iter = nullptr;
-        while (JSContext* acx = JS_ContextIterator(Runtime(), &iter)) {
-          if (!js::HasUnrootedGlobal(acx)) {
-            JS_ToggleOptions(acx, JSOPTION_UNROOTED_GLOBAL);
-          }
-        }
-      }
-
       break;
-    }
     case JSGC_END:
     {
       /*
        * If the previous GC created a runnable to finalize objects
        * incrementally, and if it hasn't finished yet, finish it now. We
        * don't want these to build up. We also don't want to allow any
        * existing incremental finalize runnables to run after a
        * non-incremental GC, since they are often used to detect leaks.
@@ -1225,16 +1212,10 @@ CycleCollectedJSRuntime::OnGC(JSGCStatus
   }
 
   CustomGCCallback(aStatus);
 }
 
 bool
 CycleCollectedJSRuntime::OnContext(JSContext* aCx, unsigned aOperation)
 {
-  if (mExpectUnrootedGlobals && aOperation == JSCONTEXT_NEW) {
-    // XXXkhuey bholley is going to make this go away, but for now XPConnect
-    // needs it.
-    JS_ToggleOptions(aCx, JSOPTION_UNROOTED_GLOBAL);
-  }
-
   return CustomContextCallback(aCx, aOperation);
 }
--- a/xpcom/base/CycleCollectedJSRuntime.h
+++ b/xpcom/base/CycleCollectedJSRuntime.h
@@ -78,18 +78,17 @@ class IncrementalFinalizeRunnable;
 
 class CycleCollectedJSRuntime
 {
   friend class JSGCThingParticipant;
   friend class JSZoneParticipant;
   friend class IncrementalFinalizeRunnable;
 protected:
   CycleCollectedJSRuntime(uint32_t aMaxbytes,
-                          JSUseHelperThreads aUseHelperThreads,
-                          bool aExpectUnrootedGlobals);
+                          JSUseHelperThreads aUseHelperThreads);
   virtual ~CycleCollectedJSRuntime();
 
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
   void UnmarkSkippableJSHolders();
 
   virtual void TraverseAdditionalNativeRoots(nsCycleCollectionNoteRootCallback& aCb) {}
   virtual void TraceAdditionalNativeGrayRoots(JSTracer* aTracer) {}
 
@@ -218,18 +217,16 @@ private:
 
   nsTArray<nsISupports*> mDeferredSupports;
   typedef nsDataHashtable<nsFuncPtrHashKey<DeferredFinalizeFunction>, void*>
     DeferredFinalizerTable;
   DeferredFinalizerTable mDeferredFinalizerTable;
 
   nsRefPtr<IncrementalFinalizeRunnable> mFinalizeRunnable;
 
-  bool mExpectUnrootedGlobals;
-
 #ifdef DEBUG
   void* mObjectToUnlink;
 #endif
 };
 
 } // namespace mozilla
 
 #endif // mozilla_CycleCollectedJSRuntime_h__