Bug 719295 - Clean up telemetry JSObject construction. r=taras, f=luke
☠☠ backed out by 1c238e6cf7a1 ☠ ☠
authorNathan Froyd <froydnj@mozilla.com>
Wed, 15 Feb 2012 14:01:53 -0800
changeset 88129 e9c07d92a4a4717ac78b36a61cef3bfa44d73f7e
parent 88128 665891e3f09e7886bf12ba1ff9315fa95b4d7c9d
child 88130 01c08685ac65ade027ecf5bbac6f932a68c1e25c
push id22171
push usermak77@bonardo.net
push dateFri, 02 Mar 2012 13:56:30 +0000
treeherdermozilla-central@343ec916dfd5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstaras
bugs719295
milestone13.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 719295 - Clean up telemetry JSObject construction. r=taras, f=luke
toolkit/components/telemetry/Telemetry.cpp
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -313,36 +313,51 @@ enum reflectStatus
 ReflectHistogramAndSamples(JSContext *cx, JSObject *obj, Histogram *h,
                            const Histogram::SampleSet &ss)
 {
   // We don't want to reflect corrupt histograms.
   if (h->FindCorruption(ss) != Histogram::NO_INCONSISTENCIES) {
     return REFLECT_CORRUPT;
   }
 
-  JSObject *counts_array;
-  JSObject *rarray;
+  if (!JS_DefineProperty(cx, obj, "min", INT_TO_JSVAL(h->declared_min()), NULL, NULL, JSPROP_ENUMERATE)
+      && JS_DefineProperty(cx, obj, "max", INT_TO_JSVAL(h->declared_max()), NULL, NULL, JSPROP_ENUMERATE)
+      && JS_DefineProperty(cx, obj, "histogram_type", INT_TO_JSVAL(h->histogram_type()), NULL, NULL, JSPROP_ENUMERATE)
+      && JS_DefineProperty(cx, obj, "sum", DOUBLE_TO_JSVAL(ss.sum()), NULL, NULL, JSPROP_ENUMERATE)) {
+    return REFLECT_FAILURE;
+  }
+
   const size_t count = h->bucket_count();
-  if (!(JS_DefineProperty(cx, obj, "min", INT_TO_JSVAL(h->declared_min()), NULL, NULL, JSPROP_ENUMERATE)
-        && JS_DefineProperty(cx, obj, "max", INT_TO_JSVAL(h->declared_max()), NULL, NULL, JSPROP_ENUMERATE)
-        && JS_DefineProperty(cx, obj, "histogram_type", INT_TO_JSVAL(h->histogram_type()), NULL, NULL, JSPROP_ENUMERATE)
-        && JS_DefineProperty(cx, obj, "sum", DOUBLE_TO_JSVAL(ss.sum()), NULL, NULL, JSPROP_ENUMERATE)
-        && (rarray = JS_NewArrayObject(cx, count, NULL))
-        && JS_DefineProperty(cx, obj, "ranges", OBJECT_TO_JSVAL(rarray), NULL, NULL, JSPROP_ENUMERATE)
-        && FillRanges(cx, rarray, h)
-        && (counts_array = JS_NewArrayObject(cx, count, NULL))
-        && JS_DefineProperty(cx, obj, "counts", OBJECT_TO_JSVAL(counts_array), NULL, NULL, JSPROP_ENUMERATE)
-        )) {
+  JSObject *rarray = JS_NewArrayObject(cx, count, nsnull);
+  if (!rarray) {
+    return REFLECT_FAILURE;
+  }
+  JS::AutoObjectRooter aroot(cx, rarray);
+  if (!(FillRanges(cx, rarray, h)
+        && JS_DefineProperty(cx, obj, "ranges", OBJECT_TO_JSVAL(rarray),
+                             NULL, NULL, JSPROP_ENUMERATE))) {
+    return REFLECT_FAILURE;
+  }
+
+  JSObject *counts_array = JS_NewArrayObject(cx, count, NULL);
+  if (!counts_array) {
+    return REFLECT_FAILURE;
+  }
+  JS::AutoObjectRooter croot(cx, counts_array);
+  if (!JS_DefineProperty(cx, obj, "counts", OBJECT_TO_JSVAL(counts_array),
+                         NULL, NULL, JSPROP_ENUMERATE)) {
     return REFLECT_FAILURE;
   }
   for (size_t i = 0; i < count; i++) {
-    if (!JS_DefineElement(cx, counts_array, i, INT_TO_JSVAL(ss.counts(i)), NULL, NULL, JSPROP_ENUMERATE)) {
+    if (!JS_DefineElement(cx, counts_array, i, INT_TO_JSVAL(ss.counts(i)),
+                          NULL, NULL, JSPROP_ENUMERATE)) {
       return REFLECT_FAILURE;
     }
   }
+ 
   return REFLECT_OK;
 }
 
 enum reflectStatus
 ReflectHistogramSnapshot(JSContext *cx, JSObject *obj, Histogram *h)
 {
   Histogram::SampleSet ss;
   h->SnapshotSample(&ss);
@@ -388,19 +403,20 @@ JSBool
 JSHistogram_Snapshot(JSContext *cx, unsigned argc, jsval *vp)
 {
   JSObject *obj = JS_THIS_OBJECT(cx, vp);
   if (!obj) {
     return JS_FALSE;
   }
 
   Histogram *h = static_cast<Histogram*>(JS_GetPrivate(obj));
-  JSObject *snapshot = JS_NewObject(cx, NULL, NULL, NULL);
+  JSObject *snapshot = JS_NewObject(cx, nsnull, nsnull, nsnull);
   if (!snapshot)
     return JS_FALSE;
+  JS::AutoObjectRooter sroot(cx, snapshot);
 
   switch (ReflectHistogramSnapshot(cx, snapshot, h)) {
   case REFLECT_FAILURE:
     return JS_FALSE;
   case REFLECT_CORRUPT:
     JS_ReportError(cx, "Histogram is corrupt");
     return JS_FALSE;
   case REFLECT_OK:
@@ -421,20 +437,24 @@ WrapAndReturnHistogram(Histogram *h, JSC
     JS_PropertyStub, JS_PropertyStub, JS_PropertyStub, JS_StrictPropertyStub,
     JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, JS_FinalizeStub,
     JSCLASS_NO_OPTIONAL_MEMBERS
   };
 
   JSObject *obj = JS_NewObject(cx, &JSHistogram_class, NULL, NULL);
   if (!obj)
     return NS_ERROR_FAILURE;
+  JS::AutoObjectRooter root(cx, obj);
+  if (!(JS_DefineFunction (cx, obj, "add", JSHistogram_Add, 1, 0)
+        && JS_DefineFunction (cx, obj, "snapshot", JSHistogram_Snapshot, 1, 0))) {
+    return NS_ERROR_FAILURE;
+  }
   *ret = OBJECT_TO_JSVAL(obj);
   JS_SetPrivate(obj, h);
-  return (JS_DefineFunction (cx, obj, "add", JSHistogram_Add, 1, 0)
-          && JS_DefineFunction (cx, obj, "snapshot", JSHistogram_Snapshot, 1, 0)) ? NS_OK : NS_ERROR_FAILURE;
+  return NS_OK;
 }
 
 TelemetryImpl::TelemetryImpl():
 mHistogramMap(Telemetry::HistogramCount),
 mCanRecord(XRE_GetProcessType() == GeckoProcessType_Default),
 mHashMutex("Telemetry::mHashMutex")
 {
   // A whitelist to prevent Telemetry reporting on Addon & Thunderbird DBs
@@ -474,43 +494,47 @@ bool
 TelemetryImpl::StatementReflector(SlowSQLEntryType *entry, JSContext *cx,
                                   JSObject *obj)
 {
   const nsACString &sql = entry->GetKey();
   jsval hitCount = UINT_TO_JSVAL(entry->mData.hitCount);
   jsval totalTime = UINT_TO_JSVAL(entry->mData.totalTime);
 
   JSObject *arrayObj = JS_NewArrayObject(cx, 2, nsnull);
-  return (arrayObj
-          && JS_SetElement(cx, arrayObj, 0, &hitCount)
+  if (!arrayObj) {
+    return false;
+  }
+  JS::AutoObjectRooter root(cx, arrayObj);
+  return (JS_SetElement(cx, arrayObj, 0, &hitCount)
           && JS_SetElement(cx, arrayObj, 1, &totalTime)
           && JS_DefineProperty(cx, obj,
                                sql.BeginReading(),
                                OBJECT_TO_JSVAL(arrayObj),
                                NULL, NULL, JSPROP_ENUMERATE));
 }
 
 bool
 TelemetryImpl::AddSQLInfo(JSContext *cx, JSObject *rootObj, bool mainThread)
 {
   JSObject *statsObj = JS_NewObject(cx, NULL, NULL, NULL);
   if (!statsObj)
     return false;
-
-  JSBool ok = JS_DefineProperty(cx, rootObj,
-                                mainThread ? "mainThread" : "otherThreads",
-                                OBJECT_TO_JSVAL(statsObj),
-                                NULL, NULL, JSPROP_ENUMERATE);
-  if (!ok)
-    return false;
+  JS::AutoObjectRooter root(cx, statsObj);
 
   AutoHashtable<SlowSQLEntryType> &sqlMap = (mainThread
                                              ? mSlowSQLOnMainThread
                                              : mSlowSQLOnOtherThread);
-  return sqlMap.ReflectHashtable(StatementReflector, cx, statsObj);
+  if (!sqlMap.ReflectHashtable(StatementReflector, cx, statsObj)) {
+    return false;
+  }
+
+  return JS_DefineProperty(cx, rootObj,
+                           mainThread ? "mainThread" : "otherThreads",
+                           OBJECT_TO_JSVAL(statsObj),
+                           NULL, NULL, JSPROP_ENUMERATE);
 }
 
 nsresult
 TelemetryImpl::GetHistogramEnumId(const char *name, Telemetry::ID *id)
 {
   if (!sTelemetry) {
     return NS_ERROR_FAILURE;
   }
@@ -773,16 +797,17 @@ TelemetryImpl::GetHistogramSnapshots(JSC
     if (!ShouldReflectHistogram(h)) {
       continue;
     }
 
     JSObject *hobj = JS_NewObject(cx, NULL, NULL, NULL);
     if (!hobj) {
       return NS_ERROR_FAILURE;
     }
+    JS::AutoObjectRooter root(cx, hobj);
     switch (ReflectHistogramSnapshot(cx, hobj, h)) {
     case REFLECT_CORRUPT:
       // We can still hit this case even if ShouldReflectHistograms
       // returns true.  The histogram lies outside of our control
       // somehow; just skip it.
       continue;
     case REFLECT_FAILURE:
       return NS_ERROR_FAILURE;
@@ -801,17 +826,21 @@ TelemetryImpl::AddonHistogramReflector(A
                                        JSContext *cx, JSObject *obj)
 {
   // Never even accessed the histogram.
   if (!entry->mData.h) {
     return true;
   }
 
   JSObject *snapshot = JS_NewObject(cx, NULL, NULL, NULL);
-  js::AutoObjectRooter r(cx, snapshot);
+  if (!snapshot) {
+    // Just consider this to be skippable.
+    return true;
+  }
+  JS::AutoObjectRooter r(cx, snapshot);
   switch (ReflectHistogramSnapshot(cx, snapshot, entry->mData.h)) {
   case REFLECT_FAILURE:
   case REFLECT_CORRUPT:
     return false;
   case REFLECT_OK:
     const nsACString &histogramName = entry->GetKey();
     if (!JS_DefineProperty(cx, obj,
                            PromiseFlatCString(histogramName).get(),
@@ -828,17 +857,17 @@ bool
 TelemetryImpl::AddonReflector(AddonEntryType *entry,
                               JSContext *cx, JSObject *obj)
 {
   const nsACString &addonId = entry->GetKey();
   JSObject *subobj = JS_NewObject(cx, NULL, NULL, NULL);
   if (!subobj) {
     return false;
   }
-  js::AutoObjectRooter r(cx, subobj);
+  JS::AutoObjectRooter r(cx, subobj);
 
   AddonHistogramMapType *map = entry->mData;
   if (!(map->ReflectHashtable(AddonHistogramReflector, cx, subobj)
         && JS_DefineProperty(cx, obj,
                              PromiseFlatCString(addonId).get(),
                              OBJECT_TO_JSVAL(subobj), NULL, NULL,
                              JSPROP_ENUMERATE))) {
     return false;
@@ -849,17 +878,17 @@ TelemetryImpl::AddonReflector(AddonEntry
 NS_IMETHODIMP
 TelemetryImpl::GetAddonHistogramSnapshots(JSContext *cx, jsval *ret)
 {
   *ret = JSVAL_VOID;
   JSObject *obj = JS_NewObject(cx, NULL, NULL, NULL);
   if (!obj) {
     return NS_ERROR_FAILURE;
   }
-  js::AutoObjectRooter r(cx, obj);
+  JS::AutoObjectRooter r(cx, obj);
 
   if (!mAddonMap.ReflectHashtable(AddonReflector, cx, obj)) {
     return NS_ERROR_FAILURE;
   }
   *ret = OBJECT_TO_JSVAL(obj);
   return NS_OK;
 }
 
@@ -884,16 +913,17 @@ TelemetryImpl::GetSlowSQL(JSContext *cx,
 
 NS_IMETHODIMP
 TelemetryImpl::GetRegisteredHistograms(JSContext *cx, jsval *ret)
 {
   size_t count = ArrayLength(gHistograms);
   JSObject *info = JS_NewObject(cx, NULL, NULL, NULL);
   if (!info)
     return NS_ERROR_FAILURE;
+  JS::AutoObjectRooter root(cx, info);
 
   for (size_t i = 0; i < count; ++i) {
     JSString *comment = JS_InternString(cx, gHistograms[i].comment);
     
     if (!(comment
           && JS_DefineProperty(cx, info, gHistograms[i].id,
                                STRING_TO_JSVAL(comment), NULL, NULL,
                                JSPROP_ENUMERATE))) {
@@ -978,32 +1008,32 @@ TelemetrySessionData::SampleReflector(En
   if (NS_FAILED(rv)) {
     return true;
   }
 
   JSObject *snapshot = JS_NewObject(cx, NULL, NULL, NULL);
   if (!snapshot) {
     return false;
   }
-  js::AutoObjectRooter root(cx, snapshot);
+  JS::AutoObjectRooter root(cx, snapshot);
   return (ReflectHistogramAndSamples(cx, snapshot, h, entry->mData)
           && JS_DefineProperty(cx, snapshots,
                                h->histogram_name().c_str(),
                                OBJECT_TO_JSVAL(snapshot), NULL, NULL,
                                JSPROP_ENUMERATE));
 }
 
 NS_IMETHODIMP
 TelemetrySessionData::GetSnapshots(JSContext *cx, jsval *ret)
 {
   JSObject *snapshots = JS_NewObject(cx, NULL, NULL, NULL);
   if (!snapshots) {
     return NS_ERROR_FAILURE;
   }
-  js::AutoObjectRooter root(cx, snapshots);
+  JS::AutoObjectRooter root(cx, snapshots);
 
   if (!mSampleSetMap.ReflectHashtable(SampleReflector, cx, snapshots)) {
     return NS_ERROR_FAILURE;
   }
 
   *ret = OBJECT_TO_JSVAL(snapshots);
   return NS_OK;
 }