1378854 - Stop using sdk/core/heritage in DevTools stack utils

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P1)

Description

[Soledad Penades [:sole] [:spenades]]

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

More details to follow as we triage.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1378854 - Stop using sdk/core/heritage in DevTools stack utils;

Review commit: https://reviewboard.mozilla.org/r/155220/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/155220/

Comment 2

[Matteo Ferretti [:zer0] [:matteo]]

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.

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8884270 [details]
Bug 1378854 - Stop using sdk/core/heritage in DevTools stack utils;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155220/diff/1-2/

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8884270 [details]
Bug 1378854 - Stop using sdk/core/heritage in DevTools stack utils;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155220/diff/2-3/

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

All fixed, thanks for the review!
Honza

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8884270 [details]
Bug 1378854 - Stop using sdk/core/heritage in DevTools stack utils;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155220/diff/3-4/

Comment 7

[Matteo Ferretti [:zer0] [:matteo]]

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!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8163e29a92f
Stop using sdk/core/heritage in DevTools stack utils; r=zer0

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d8163e29a92f