Can't register a custom actor

RESOLVED FIXED in Firefox 45

Status

()

Firefox
Developer Tools
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Honza, Assigned: ochameau)

Tracking

Trunk
Firefox 45
Points:
---

Firefox Tracking Flags

(firefox44 affected, firefox45 fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
I am unable to register a custom actor using ActorRegistryActor

STR:

1) Use the following example to reproduce the erorr:
git clone https://github.com/firebug/devtools-extension-examples.git
cd TabActor
jpm run

2) Open the Toolbox (the actor should be registered at the moment)
3) Open the Browser Console, when the actor is properly registered you should see: "Response from the actor: Hello from a tab actor!"

It works in DevEdition, but not in Nightly.

Nightly says: 

Error occurred while creating actor 'undefined: Error: Unable to load actor module 'undefined'.
You must provide a module name when calling require() from devtools/server/actors/common
require@resource://gre/modules/commonjs/toolkit/loader.js:579:1
RegisteredActorFactory/this._getConstructor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:49:17
ObservedActorFactory.prototype.createActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:113:11
DebuggerServerConnection.prototype._getOrCreateActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1443:16
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1587:17
ChildDebuggerTransport.prototype.receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:742:5


Stack: RegisteredActorFactory/this._getConstructor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:51:17
ObservedActorFactory.prototype.createActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:113:11
DebuggerServerConnection.prototype._getOrCreateActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1443:16
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1587:17
ChildDebuggerTransport.prototype.receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:742:5
Line: 51, column: 17

Honza
(Reporter)

Comment 1

3 years ago
JRyans, could this be caused by the recent module paths changes?

Honza
Flags: needinfo?(jryans)
I believe the issue is with how we find the constructor function[1]:

  Cu.evalInSandbox(sourceText, sandbox, "1.8", fileName, 1);

Since we are not using the `exports` object during this phase, the object with the constructor name ("MyTabActor") needs to exist on the sandbox's global object.  After bug 1202902 in Nightly, let and const variables are no longer on the global object.  So, defining `MyTabActor` with `var` should resolve this.

[1]: https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/utils/actor-registry-utils.js#27
Flags: needinfo?(jryans)
(Reporter)

Comment 3

3 years ago
Ah, great!

Now, I am testing the same example (TabActor) in e10s enabled browser with multiple tabs.

Here are my STR:
1) Start your browser, make sure you have 'Show my windows and tabs from last time' option selected.
2) Also make sure you have two tabs open automatically
3) Open the Toolbox on the first tab (selected by default)
4) The browser console says: Response from the actor: Hello from a tab actor! -> OK
5) Switch to the second tab, open the Toolbox
6) The browser console says: No such actor for ID: server1.conn0.child3/mytabactor27 -> BUG


If you manually open a new tab and open the Toolbox on it, it works well.

Honza
Flags: needinfo?(jryans)
I'll refer you to Alex, he knows a lot more about this than I do.
Flags: needinfo?(jryans) → needinfo?(poirot.alex)
(Reporter)

Comment 5

3 years ago
Created attachment 8679964 [details]
tabactor-0.1.0.xpi

TabActor extension XPI
source: https://github.com/firebug/devtools-extension-examples/tree/master/TabActor

Honza
(Assignee)

Comment 6

3 years ago
Created attachment 8680081 [details] [diff] [review]
patch v1

Oh! That was quite a quest to trace this down to this...

actors/childtab.js listen for debug:form message
and emit updated form to whoever ask.
But in this very particular STR, we have
two different toolboxes
=> two different client
=> two different DebuggerServerConnection
=> two different set of tab actors for all tabs
=> two different call to listTabs results for each toolbox
=> two different ContentActor (actors/childtabs.js)

The code in webbrowser.js was sending a debug:form
and accept the first response.
Whereas, here, we expect the second one.

This patch ensure updating the form from the matching actor,
which is on the same DebuggerServerConnection,
has the same prefix and the same ID than the actor
we connectToChild-ed.
Attachment #8680081 - Flags: review?(jryans)
(Assignee)

Updated

3 years ago
Assignee: nobody → poirot.alex
Flags: needinfo?(poirot.alex)
Comment on attachment 8680081 [details] [diff] [review]
patch v1

Review of attachment 8680081 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for finding this!

Should other message listeners, for example in server/main for `connectToChild` or `connectToContent`, have a similar check?
Attachment #8680081 - Flags: review?(jryans) → review+
(Assignee)

Comment 8

3 years ago
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #7)
> Comment on attachment 8680081 [details] [diff] [review]
> patch v1
> 
> Review of attachment 8680081 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for finding this!
> 
> Should other message listeners, for example in server/main for
> `connectToChild` or `connectToContent`, have a similar check?

connectToChild looks mostly safe, we already have similar checks for debug:actor message.
But there may be something wrong with its debug:setup-in-parent message.
We may load the module more than once.
connectToContent is not supposed to be called more than once. There is various limitations with this method, like we can debug only one content process.
(Assignee)

Comment 9

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/692d0ed9f9eb102efb136fff4dd9df4fdb16a2ea
Bug 1217867 - Prevent actor id clash when debugging the same e10s tab with multiple clients. r=jryans
(Assignee)

Updated

3 years ago
Keywords: leave-open
(Reporter)

Comment 10

3 years ago
Tested and fixes the problem for me, thanks!

Honza
(Assignee)

Comment 13

3 years ago
Created attachment 8680636 [details] [diff] [review]
patch for setupInParent v1

Similar fix for setupInParent.
Attachment #8680636 - Flags: review?(jryans)
Attachment #8680636 - Flags: review?(jryans) → review+
(Assignee)

Comment 15

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/065703549497c74071b08bf393854ae79639e2cd
Bug 1217867 - Prevent duplicated setupInParent calls when debugging same e10s tab with multiple clients. r=jryans
(Assignee)

Updated

3 years ago
Keywords: leave-open

Comment 16

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/065703549497
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
status-b2g-v2.5: --- → ---
You need to log in before you can comment on or make changes to this bug.