Closed Bug 1217867 Opened 5 years ago Closed 5 years ago

Can't register a custom actor

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(firefox44 affected, firefox45 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox44 --- affected
firefox45 --- fixed

People

(Reporter: Honza, Assigned: ochameau)

Details

Attachments

(3 files)

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
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)
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)
Attached patch patch v1Splinter Review
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: 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+
(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.
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
Keywords: leave-open
Tested and fixes the problem for me, thanks!

Honza
Similar fix for setupInParent.
Attachment #8680636 - Flags: review?(jryans)
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
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/065703549497
Status: NEW → RESOLVED
Closed: 5 years ago
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
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.