Closed Bug 1378854 Opened 2 years ago Closed 2 years ago

Stop using sdk/core/heritage in DevTools stack utils

Categories

(DevTools :: General, enhancement, P1)

enhancement

Tracking

(firefox56 fixed)

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: sole, Assigned: Honza)

References

Details

(Whiteboard: [nosdk])

Attachments

(1 file)

Used in: devtools/server/actors/utils/stack.js

More details to follow as we triage.
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Flags: qe-verify-
Priority: -- → P3
Comment on attachment 8884270 [details]
Bug 1378854 - Stop using sdk/core/heritage in DevTools stack utils;

https://reviewboard.mozilla.org/r/155220/#review160248

::: devtools/server/actors/utils/stack.js:13
(Diff revision 1)
>   * A helper class that stores stack frame objects.  Each frame is
>   * assigned an index, and if a frame is added more than once, the same
>   * index is used.  Users of the class can get an array of all frames
>   * that have been added.
>   */
> -var StackFrameCache = Class({
> +function StackFrameCache() {

We should use ES classes here. So, something like:

```js
class StackFrameCache {
  constructor() {
    this._framesToIndices = null;
    this._framesToForms = null;
    this._lastEventSize = 0;
  }
  
  /* etc. */
}
```

Notice also that for heritage's `Class`, the `initialize` function is the same as `constructor` for ES6 classes, so this code won't initialize the properties (see: 
https://developer.mozilla.org/en-US/Add-ons/SDK/Low-Level_APIs/core_heritage)

If, for some reason, we have to use the prototyping approach, then we should have moved the `initialize`'s body function into `StackFrameCache`'s body.

I will add this info and the link to SDK documentation in the our doc.
Attachment #8884270 - Flags: review?(zer0) → review-
All fixed, thanks for the review!
Honza
Priority: P3 → P1
Whiteboard: [nosdk]
Target Milestone: --- → Firefox 56
Comment on attachment 8884270 [details]
Bug 1378854 - Stop using sdk/core/heritage in DevTools stack utils;

https://reviewboard.mozilla.org/r/155220/#review160298

Looks good to me! It seems you've already fixed the eslint issue too!
Attachment #8884270 - Flags: review?(zer0) → review+
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8163e29a92f
Stop using sdk/core/heritage in DevTools stack utils; r=zer0
https://hg.mozilla.org/mozilla-central/rev/d8163e29a92f
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.