1247084 - Actor methods and events about sources should move from ThreadActor to TabActor-ish parents

Status: NEW

(DevTools :: Debugger, task, P5)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

A while ago, the TabSources object was moved from the ThreadActor to TabActor-ish parents who now own it.

However, we left all the actor methods and events on the ThreadActor (not sure why).

We should finally do the work to clean this up, using traits as needed to preserve compatibility.

Comment 1

[Eddy Bruel [:ejpbruel]]

We had a conversation on the mailing list about this a while ago. I think the change to move the TabSources object to TabActor didn't make sense in the first place, because the SourceActors that it stores are still owned by the ThreadActor. With the introduction of workers, we now have multiple ThreadActors, and the sources for those threads will go away when the corresponding thread does.

I think the TabSources object should be moved back to ThreadActor, and we should rethink our strategy for giving other tools access to the sources for each thread. With that in mind, I'm voting to wontfix this bug.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

It does not seem to me that any conclusion was reached on the mailing list.  Eddy posted a thread titled "Sources should be scoped by thread, not by tab" and offered the view he mentions in the comment above.  Nick replied with the opposite perspective.  No conclusion or action has so far been described on the mailing list as far as I can see.

Comment 3

[Eddy Bruel [:ejpbruel]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #2)
> It does not seem to me that any conclusion was reached on the mailing list. 
> Eddy posted a thread titled "Sources should be scoped by thread, not by tab"
> and offered the view he mentions in the comment above.  Nick replied with
> the opposite perspective.  No conclusion or action has so far been described
> on the mailing list as far as I can see.

You're right. I was overzealous here. Let's reopen this bug pending a decision on the mailing list.

Comment 4

[Alexandre Poirot [:ochameau]]

I'm trying to speedup the browser toolbox, and I see tons of newSource messages. Like double-tons of them

This bug seems to be about that.
These duplicated messages seems to be introduced in bug 1177279, which is recent.

Could we get rid of them? Is this only to support older clients? We are not supporting old client on a more recent server, only the opposite. So we could get rid of it...

Comment 5

[Alexandre Poirot [:ochameau]]

I'm talking about this:
https://hg.mozilla.org/mozilla-central/annotate/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/devtools/server/actors/script.js#l1903
  onSourceEvent: function (name, source) {
    this.conn.send({
      from: this._parent.actorID,
      type: name,
      source: source.form()
    });

    // For compatibility and debugger still using `newSource` on the thread client,
    // still emit this event here. Clean up in bug 1247084
    if (name === "newSource") {
      this.conn.send({
        from: this.actorID,
        type: name,
        source: source.form()
      });
    }

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Alexandre Poirot [:ochameau] from comment #4)
> I'm trying to speedup the browser toolbox, and I see tons of newSource
> messages. Like double-tons of them
> 
> This bug seems to be about that.
> These duplicated messages seems to be introduced in bug 1177279, which is
> recent.
> 
> Could we get rid of them? Is this only to support older clients? We are not
> supporting old client on a more recent server, only the opposite. So we
> could get rid of it...

This bug is about completing the transition to TabActor.  The specific issue you are seeing with duplicated events seems like a side issue, so let's move it to a different bug.  I filed bug 1269919.

Comment 7

[Jason Laster [:jlast]]

Yulia, Is this still relevant?

Comment 8

[Yulia Startsev [:yulia]]

It looks like the discussion here was never resolved. We should come back to it when there is time for working on server topics again.