Implement Debugger.Object.prototype.hostAnnotations

NEW
Unassigned

Status

()

Firefox
Developer Tools: Debugger
P2
normal
6 years ago
a month ago

People

(Reporter: jimb, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [chrome-debug][after-landing: see comment #2][leave open])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
The Debugger documentation drafts here:
https://github.com/jimblandy/DebuggerDocs/compare/master...onNewGlobal

describe the Debugger.Object.prototype.hostAttributes accessor:

"A typical JavaScript embedding provides "host objects" to expose application-specific functionality to scripts. The <code>hostAnnotations</code> accessor consults the embedding for additional information about the referent that might be of interest to the debugger. The returned object's properties' meanings are up to the embedding. For example, a web browser might provide host annotations for global objects to distinguish top-level windows, iframes, and internal JavaScript scopes."
Whiteboard: [chrome-debug]
Summary: [jsdbg2] Implement Debugger.Object.prototype.hostAttributes → [jsdbg2] Implement Debugger.Object.prototype.hostAnnotations
FILTER ON PUMPKINS.
Priority: -- → P2
check to see if bug 818134 has landed when landing patches for this.

dbg-script-actors.js :: onNewGlobal() may need some JSONification.
Whiteboard: [chrome-debug] → [chrome-debug][after-landing: see comment #2]
(Reporter)

Comment 3

5 years ago
It looks like bug 946813 would also benefit from having this implemented. jryans has found a workaround based on JSCompartment::invisibleToDebugger, but the underlying problem is really that we just don't have enough metadata about globals when we add them as debuggees.

I tried implementing hostAnnotations a while back, and was daunted by the prospect of adding a JSClass hook that could be managed with nsIXPCScriptable. However, all the use cases mentioned in the draft documentation involve global objects; and this seems like yet another. It would be a lot easier to implement hostAnnotations if they only applied to global objects: globals already have lots of slots for miscellaneous purposes, so it wouldn't be hard to add another slot holding annotations.

So I think we should restrict hostAnnotations to global objects, just to help things get moving here.
Sounds like a great plan to me!
(Reporter)

Comment 5

4 years ago
Coming back to this after a three-month hiatus.

Comment copy-and-pasted from bug 966472:

In the proposals I've written in the past, the object's js::Class could provide a hook that produces an object retrieved by a Debugger.Object.prototype.hostAnnotations getter; said object could provide whatever introspection properties make sense for the given object class.

Jason points out that many of our extant D.O accessors are actually class-specific already. The following apply only to Function instances: name, displayName, parameterNames, script, and environment. The following apply only to proxies: proxyHandler, proxyCallTrap, proxyConstructTrap. And so on. Some D.O methods are also class-specific: decompile, call, apply (for Function); evalInGlobal and evalInGlobalWithBindings (for global objects). He then argues that Function and JSCLASS_IS_GLOBAL should not be special, and that every class should be able to define its own accessors to appear on Debugger.Object instances. There's a risk of name collisions between generic Debugger.Object accessors, but I think we can manage that. As laid out in bug 961325 comment 43, we should aggressively specify well-defined common meanings for property names to help different classes avoid giving conflicting meanings to a given name, but we'll always have D.O.p.class as a discriminant if things get ugly.
(Reporter)

Comment 6

4 years ago
Okay, having spoken with Jason, here is how I think the ideal design would work:

Debugger.Object instances should have an immediate prototype object that carries class-specific accessors; that prototype object's prototype, in turn, would be the general D.O.p. For example, all D.O instances referring to functions would have a prototype holding 'name', 'displayName', 'script', etc. accessor properties; and that object's prototype would have the generic 'getOwnPropertyNames' methods.

These prototype objects would be generated on demand by consulting data on js::Class, and cached on the Debugger constructor. js::Class would own a table of accessor names and functions, which Debugger would use to produce these prototype objects.

Rather than making these accessor implementations deal with Debugger and Debugger.Object instances directly, they could be passed, in addition to the actual object to be inspected, a builder object whose methods construct the value to be returned, wrapping debuggee values as appropriate.
(Reporter)

Comment 7

4 years ago
Created attachment 8439719 [details] [diff] [review]
Remove documentation for the unimplemented (and certain to be redesigned) Debugger.Object.prototype.hostAnnotations accessor.

For now, we should delete the published documentation. Draft features should have their docs in patches in bugs, not on MDN.
Attachment #8439719 - Flags: review?(jorendorff)
Comment on attachment 8439719 [details] [diff] [review]
Remove documentation for the unimplemented (and certain to be redesigned) Debugger.Object.prototype.hostAnnotations accessor.

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

Great.
Attachment #8439719 - Flags: review?(jorendorff) → review+
(Reporter)

Comment 9

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/741953db7a6d
Whiteboard: [chrome-debug][after-landing: see comment #2] → [chrome-debug][after-landing: see comment #2][leave open]
(Reporter)

Updated

4 years ago
Attachment #8439719 - Flags: checkin+

Updated

4 years ago
Summary: [jsdbg2] Implement Debugger.Object.prototype.hostAnnotations → Implement Debugger.Object.prototype.hostAnnotations

Comment 11

3 years ago
AIUI this is necessary for multiple-window chrome debugging. Are there any plans to work on this, and if not, how do we petition the devtools team to make some of these plans? The current situation is painful for add-on and browser devs. :-(
Flags: needinfo?(jimb)
(Reporter)

Comment 12

3 years ago
I'm not going to be able to put in any time on this until after March. I'll bring this topic up with Dave and Joe.
Flags: needinfo?(jimb)
(Reporter)

Comment 13

3 years ago
Gijs, are there bugs filed for the user-visible consequences of not having this implemented? Is there a bug that says "multi-window chrome debugging doesn't work"?
Flags: needinfo?(gijskruitbosch+bugs)

Comment 14

3 years ago
(In reply to Jim Blandy :jimb from comment #13)
> Gijs, are there bugs filed for the user-visible consequences of not having
> this implemented? Is there a bug that says "multi-window chrome debugging
> doesn't work"?

bug 928018, but that *just* got duped. I'm not sure that's correct, poking :ochameau on bug 1059308 to find out more.
Flags: needinfo?(gijskruitbosch+bugs)
(Reporter)

Comment 15

3 years ago
Okay, if it's all right, I would rather have discussions about prioritization and allocating engineering resources on the bugs concerned with developer-visible features, and then mark bugs like this as blockers as appropriate.
(In reply to Jim Blandy :jimb from comment #15)
> Okay, if it's all right, I would rather have discussions about
> prioritization and allocating engineering resources on the bugs concerned
> with developer-visible features, and then mark bugs like this as blockers as
> appropriate.

FWIW, during the London workweek last year, we agreed that improving the chrome debugging experience is something that should be on our list after we improve our breakpoint experience.

How about having the discussion about prioritisation and allocating engineering resources for user-visible features related to chrome debugging, and for this bug in particular, during the weekly scripting tools meeting? I'm not currently attending that meeting, but it sounds like this would be the appropriate venue to have these discussions.
You need to log in before you can comment on or make changes to this bug.