Move the creation of a js object from combinedStacks into a static function.

RESOLVED FIXED in mozilla20

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: espindola, Assigned: espindola)

Tracking

unspecified
mozilla20
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 684705 [details] [diff] [review]
patch

I need exactly the same function for adding late writes to the telemetry ping.
Assignee: nobody → respindola
Status: NEW → ASSIGNED
Attachment #684705 - Attachment is patch: true
Attachment #684705 - Flags: review?(vdjeric)
Comment on attachment 684705 [details] [diff] [review]
patch

Looks good, just a few small changes

> class HangReports {
...
>-  const Telemetry::ProcessedStack::Module& GetModule(unsigned aIndex) const;
>+  const CombinedStacks& getStacks() const;

GetStacks instead

>+static JSObject *
>+CreateJSStackObject(JSContext *cx, const CombinedStacks &stacks);

Put this near the top of the file

>+  JSObject *durationArray = JS_NewArrayObject(cx, 0, nullptr);

Check for nullptr


>+  for (size_t i = 0, n = stacks.GetStackCount(); i < n; ++i) {
>+    jsval duration = INT_TO_JSVAL(mHangReports.GetDuration(i));
>+    if (!JS_SetElement(cx, durationArray, i, &duration)) {
>+      return NS_ERROR_FAILURE;
>+    }
>+  }

Nitpick: declare "const size_t length = stacks.GetStackCount();" before the for-loop for readability. Same for others

>-  const uint32_t moduleCount = mHangReports.GetModuleCount();
>+  const uint32_t moduleCount = stacks.GetModuleCount();

Nit: size_t
Attachment #684705 - Flags: review?(vdjeric) → review-
Created attachment 685174 [details] [diff] [review]
Updated patch
Attachment #684705 - Attachment is obsolete: true
Attachment #685174 - Flags: review?(vdjeric)
Attachment #685174 - Flags: review?(vdjeric) → review+
Blocks: 814765
https://hg.mozilla.org/mozilla-central/rev/7e6ac3bfbae9
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.