Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upEvaluating objects in devtools panics #6724
Comments
samfoo
changed the title
samfoo
changed the title
|
Yep, the objects case is harder than others because we need to retain references to JS objects for as long as they might be able to be referenced by the devtools client. The fix here is:
This should get us to the point where we get a reasonable first response from a JS object return value (in the form "[object Whatever]") and set the stage for being able to investigate properties of the object in the future. |
|
@jdm I'll take this if you don't mind (unless it's actively being worked on of course). I assume it's ok to tell the linter to ignore |
|
Correct! Please do take it on :) |
|
@jdm Is there anyway a test could be written for this? |
|
Not without #5971 unfortunately. |
|
Should
I would have thought it should output |
|
Mmm, good point. I hoped that we could use https://dxr.mozilla.org/mozilla-central/source/js/src/jsfriendapi.h#182 instead (since it handles proxies properly), but it doesn't appear to be exported publically. We probably want to do the equivalent of |
|
Nevermind, it's actually there but github won't show me all 12,000 lines that make up the file. Go ahead and use pub unsafe extern "C" fn ObjectClassName(cx: *mut JSContext,
obj: HandleObject) -> *const i8 { |
|
Awesome!
I sort of skipped over the last point of what you said about the // console.rs
let actor_name = match registry.contains(&uuid) {
true => registry.find::<ObjectActor>(&uuid).name(),
false => ObjectActor::create(registry, uuid),
};// object.rs
impl ObjectActor {
pub fn create(registry: &ActorRegistry, new_uuid: String) -> String {
let actor_name = registry.new_name("object");
let actor = ObjectActor {
name: actor_name.clone(),
uuid: new_uuid,
};
registry.register_later(box actor);
actor_name
}
}// actor.rs
pub fn contains(&self, name: &str) -> bool {
self.actors.contains_key(&name.to_string())
}I figured we'd want to use |
|
The difficulty is that we use names like "object5" for actors in the devtools task, but script tells us about JS objects using names like "34B7E350-A535-426E-BB0E-535AFBAA65AE". The hashmap gives us a mapping between those two. This will allow us to use more informative names in the future (like "tab2/object5", for example), which yields easier debugging. |
|
So then when we create an
so we can map between them like you said? Similar to this piece of |
|
Correct! I forgot about that prior art. |
Allows object evaluation in devtools -- Closes #6724 The purpose of this is to fix how objects were previously evaluated in the developer tools. - Before this, evaluating an object such as the `window` would `panic!` - After this, evaluating an object such as the `window` outputs `[object Window]` A few things to note: - This commit contains `unsafe` code. - This does not contain a test because the developer tools cannot be properly tested until #5971 lands. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7101) <!-- Reviewable:end -->
Allows object evaluation in devtools -- Closes #6724 The purpose of this is to fix how objects were previously evaluated in the developer tools. - Before this, evaluating an object such as the `window` would `panic!` - After this, evaluating an object such as the `window` outputs `[object Window]` A few things to note: - This commit contains `unsafe` code. - This does not contain a test because the developer tools cannot be properly tested until #5971 lands. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7101) <!-- Reviewable:end -->
The purpose of this is to fix how objects were previously evaluated in the developer tools. - Before this, evaluating an object such as the `window` would `panic!` - After this, evaluating an object such as the `window` outputs `[object Window]` A few things to note: - This commit contains `unsafe` code. - This does not contain a test because the developer tools cannot be properly tested until servo#5971 lands.
The purpose of this is to fix how objects were previously evaluated in the developer tools. - Before this, evaluating an object such as the `window` would `panic!` - After this, evaluating an object such as the `window` outputs `[object Window]` A few things to note: - This commit contains `unsafe` code. - This does not contain a test because the developer tools cannot be properly tested until servo#5971 lands.
./mach run http://google.com --devtoolsConnecting with Firefox devtools and then evaluating any object (e.g.
documentorwindow) panics the devtools task.