Implement console log reporting for workers

[jdm]

There's a TODO about workers in propagate_console_msg. The easiest way forward is:

• modify the SendConsoleMsg enum to include an Option<WorkerId> field
• add a worker_id field to WorkerGlobalScope that is passed in as an argument in the constructor (new and new_inherited)
• add a worker_id method to GlobalRef that returns None for a non-worker and the new field for a worker
• modify propagate_console_msg to use the required GlobalRef methods instead of matching on the specific type

After these changes, it should be possible to connect to a worker from the devtools, execute a console.log("hi") and see it appear in the devtools console log.

[HarryLovesCode]

@jdm I'd like to give this a shot, but I might need some help if you don't mind.

[jdm]

@HarryLovesCode of course! Note that you won't be able to test changes until #6829 merges.

[HarryLovesCode]

@jdm As a clarification, do you mean "anything except a worker" for the third point? Or do you mean a "non-worker" as in a window?

[jdm]

Well, there are only Workers and Windows in GlobalRef, so the difference is only a semantic one :)

[jdm]

Oh, the other part of this work is to modify handle_console_message (http://mxr.mozilla.org/servo/source/components/devtools/lib.rs#256) to use the new WorkerId information to find the appropriate ConsoleActor that is associated with a worker.

For that, I recommend:

• adding a console_actor field to WorkerActor and filling it appropriately in handle_new_global
• in handle_console_msg, pass in the actor_workers hashmap and use it to find the WorkerActor for the given WorkerId and PipelineId, and fetch the console actor based on the field added in the previous step

[HarryLovesCode]

@jdm Just making sure :). Also I don't see WorkerGlobalScope::new() only WorkerGlobalScope::new_inherited(). Did you mean DedicatedWorkerGlobalScope?

[jdm]

I did and I didn't. The field should be added to WorkerGlobalScope, which is inherited by DedicatedWorkerGlobalScope (for all practical purposes), so it should be passed in through the constructor of both DedicatedWorkerGlobalScope and WorkerGlobalScope.

Also, if you need to do any testing of the devtools stuff, https://github.com/servo/servo/wiki/Devtools should have you covered.

[HarryLovesCode]

@jdm I appreciate you helping me out as I'm new to Rust. Two more things:

1. Should the propagate_console_msg behave any differently if it matches to a worker?
2. Do you mind to explain your second point? I see what you're talking about, but I don't understand when we should find the WorkerActor unless we check for that and then get its ConsoleActor accordingly.

[jdm]

1. The code in propagate_console_msg should not behave any differently; there should be no match at all and it should get the values it requires directly from the GlobalRef methods.
2. in handle_console_message, if there's a non-None value for the worker id then we should search for a WorkerActor instead of a TabActor (using the worker id and pipeline id present in the message we received), and get the name of the appropriate ConsoleActor from this result. Does that make sense?

[HarryLovesCode]

It makes sense, but I'm not sure how to pattern match that... Sorry :/ Mind to lend a hand?

[jdm]

Assuming something like

     fn find_console_actor(actors: Arc<Mutex<ActorRegistry>>,
                           id: PipelineId,
                           worker_id: Option<WorkerId>,
                           worker_actors: &HashMap<(PipelineId, WorkerId), String>,
                           actor_pipelines: &HashMap<PipelineId, String>) -> Option<String> {

We can do this:

if let Some(worker_id) = worker_id {
    let worker_actor_name = match worker_actors.get(&(id, worker_id)) {
        Some(name) => name,
        None => return,
    };
    // find the worker actor, fetch the console_actor name from it, return the name
}

Is that more clear?

[wafflespeanut]

@HarryLovesCode If you're new to Rust, I suggest going through the issues labeled E-easy before you can get into E-less-easy ones, though the choice entirely up to you :)

[HarryLovesCode]

@wafflespeanut Yeah, I probably should have started with those. However, I'd like to try to finish this if that's ok.

@jdm Final thing, I tried to start servo and it panicked due to the output of this method being unwrapped...

// components/dom/binding/global.rs
pub fn get_worker_id(&self) -> Option<WorkerId> {
    match *self {
        GlobalRef::Window(ref window) => None, // <- Problem is here
        GlobalRef::Worker(ref worker) => Some(worker.get_worker_id()),
    }
}

Any idea why?

[jdm]

Well, the problem isn't that get_worker_id returns None - the problem is that some caller is calling unwrap, rather than passing around an Option<WorkerId> value as necessary.

[jdm]

In theory there should be a backtrace of the failure showing up when that happens, and it should show who's calling unwrap.

[HarryLovesCode]

@jdm Got it. Logging works from web workers (at least to the console), but devtools causes a crash. I assume #6829 will fix the issue with devtools?

[jdm]

It might! We should be able to find out soon.

[jdm]

It merged, so it should be possible to test this manually by connecting to a page with a worker (<script>new Worker('worker.js')</script>) and executing console.log('hi') in the web console that appears.

[wafflespeanut]

@HarryLovesCode @jdm It's been merged. Sorry for letting this sit around for a while - so many unanticipated, (but interesting) results along the way :)

[HarryLovesCode]

@jdm I'm trying to use the web console, but no logging appears at all (even a simple console.log("TEST") from outside of a worker).

[wafflespeanut]

@HarryLovesCode The logging occurs in your terminal (where you run the servo instance). For example, input something on the console and see if you get some JSON-like stuff (that contains your input) in the command-line. We won't get the reply back in the console (for now) because that's the sole purpose of this issue :)

It might as well be better if you could say what you actually did...

[HarryLovesCode]

@wafflespeanut Oh! I see. My bad. I didn't notice you said it would appear in the "JSON-like stuff."

To answer the second part, I've done everything, but handle_console_message.

[wafflespeanut]

@HarryLovesCode Right, all that I've said above assumes that you're running everything inside a worker (sorry I didn't mention that). While running a worker, the devtools console can be used to send messages (whose output will be displayed in the command-line, but not on your devtools console), something like this...

So, the devtools console (which usually logs when inside a non-worker) doesn't log when inside a worker. The aim of this issue is to fix that. Once we fix it, we'll be able to get the output just like the usual console.log outputs. Does that help?

[HarryLovesCode]

@wafflespeanut That helps a lot. I'll commit the changes in a few minutes. I've got it working properly now. Sorry about the confusion.

[wafflespeanut]

@HarryLovesCode No problem. And, please ensure that your local build succeeds (just to make sure that your changes haven't actually broken something).

[frewsxcv]

We should probably create a regression test for this before closing

EDIT: or close this and open a new issue

[jdm]

We can't without #5971, unfortunately.