Closed Bug 1451018 Opened 2 years ago Closed 2 years ago

Convert SymbolActor to protocol.js

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: yulia, Assigned: nchevobbe)

References

Details

Attachments

(1 file)

SymbolActor -> via createValueGrip, so similar to ObjectActor and PauseScopedObjectActor
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
DAMP showed no regression on this (the actor is used in the objectexpand test)
Comment on attachment 8968925 [details]
Bug 1451018 - Convert SymbolActor to protocol.js; .

https://reviewboard.mozilla.org/r/237640/#review244200

::: devtools/server/actors/object/symbol.js:41
(Diff revision 1)
>    /**
>     * Returns a grip for this actor for returning in a protocol message.
>     */
> -  grip: function() {
> +  form: function() {
>      let form = {
> -      type: "symbol",
> +      type: this.typeName,

is this.typeName defined anywhere? i have noticed that quite a few of our protocol.js actors are missing this!
Attachment #8968925 - Flags: review?(ystartsev)
Comment on attachment 8968925 [details]
Bug 1451018 - Convert SymbolActor to protocol.js; .

https://reviewboard.mozilla.org/r/237640/#review244200

> is this.typeName defined anywhere? i have noticed that quite a few of our protocol.js actors are missing this!

I think it's yet another corner of p.js that is hard to follow...

When creating an actor, the `typeName` field is copied from the spec onto the actor proto[1].  So, if it's defined on the spec (which is required via a check[2]) it should appear on the actor as well.

[1]: https://searchfox.org/mozilla-central/rev/36dec78aecc40539ecc8d78e91612e38810f963c/devtools/shared/protocol.js#1117
[2]: https://searchfox.org/mozilla-central/rev/36dec78aecc40539ecc8d78e91612e38810f963c/devtools/shared/protocol.js#1192
Comment on attachment 8968925 [details]
Bug 1451018 - Convert SymbolActor to protocol.js; .

Ooops sorry yulia, I forgot about this patch.
The typeName is set in the spec file, and as jryans said it is then propagated.
Attachment #8968925 - Flags: review?(ystartsev)
Comment on attachment 8968925 [details]
Bug 1451018 - Convert SymbolActor to protocol.js; .

https://reviewboard.mozilla.org/r/237640/#review245658

::: devtools/server/tests/unit/test_symbolactor.js:40
(Diff revision 1)
>    strictEqual(TEST_SYMBOL in actor.registeredPool.symbolActors, false);
>  }
>  
>  function test_SA_grip() {
>    let actor = makeMockSymbolActor();
> -  let grip = actor.grip();
> +  let grip = actor.form();

would it make sense to update all 'grip' naming to 'form'?
Attachment #8968925 - Flags: review?(ystartsev) → review+
Comment on attachment 8968925 [details]
Bug 1451018 - Convert SymbolActor to protocol.js; .

https://reviewboard.mozilla.org/r/237640/#review245658

> would it make sense to update all 'grip' naming to 'form'?

do you mean, in the test ?
Comment on attachment 8968925 [details]
Bug 1451018 - Convert SymbolActor to protocol.js; .

https://reviewboard.mozilla.org/r/237640/#review245658

> do you mean, in the test ?

yep!
sure
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s fa43ed1242bfff3639b8bdc3075bafd58141640a -d 7e1f48fed0da: rebasing 460627:fa43ed1242bf "Bug 1451018 - Convert SymbolActor to protocol.js; r=yulia." (tip)
merging devtools/shared/specs/index.js
merging devtools/shared/specs/moz.build
warning: conflicts while merging devtools/shared/specs/moz.build! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s d0ef7c2da0ee8c9bcede470be12a732f2a6a11c8 -d dd9ba54d497c: rebasing 460722:d0ef7c2da0ee "Bug 1451018 - Convert SymbolActor to protocol.js; r=yulia." (tip)
merging devtools/shared/specs/index.js
merging devtools/shared/specs/moz.build
warning: conflicts while merging devtools/shared/specs/moz.build! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1d4d0038e10a
Convert SymbolActor to protocol.js; r=yulia.
https://hg.mozilla.org/mozilla-central/rev/1d4d0038e10a
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.